[refactor] - Adjust File Handling Errors #3519
Conversation
| defer func() { | ||
| h.measureLatencyAndHandleErrors(start, err) | ||
| h.metrics.incFilesProcessed() | ||
| }() |
There was a problem hiding this comment.
Does this mean that archives that generate errors during processing are no longer counted by the metric and no longer have their latency measured?
There was a problem hiding this comment.
This change increments incFilesProcessed only for Non-Fatal errors, but latency is recorded regardless of success or failure. For Fatal errors, we increment the incErrors metric within measureLatencyAndHandleErrors instead.
I was concerned when I saw that we’re incrementing the file processed metric even in cases of errors. The metric name may be unclear, as I assumed "processed" meant successfully processed, not just acknowledged. With the current approach, even files we can’t open are counted as processed, which feels somewhat misleading. I’m open to changing it back, though.
There was a problem hiding this comment.
Food for thought, the status could be represented with a metric tag and then we could use the same metric visualizations 'by status', which could be "success", "failure", "skipped" etc.
There was a problem hiding this comment.
That's an interesting idea. I'll put together a separate PR with that. Thanks for the suggestion.
Description:
This PR address the first point in this comment.
Avoid shadowing
errby handling it immediately upon encountering an error.Checklist:
make test-community)?make lintthis requires golangci-lint)?