fix(kafkaexporter): Make Sarama's ConfigurationError as permanent to prevent retries #38608
Conversation
an-mmx
force-pushed
the
feat/kafkaexporter_permanent_config_error
branch
from
3e2ada1 to
1577beb
Compare
|
If there a configuration error, should it not fail to start up? |
|
Reading the issue, it sounds like this should be part of the configuration validation step, not part of the execution. |
|
Unfortunately, it's impossible to validate size of a message upon configuration validation. The fact is that Sarama client produce such kind of error and this error should be processed by otelcol as a permanent as this is some kind idempotent state. |
| if !errors.As(err, &producerErrs) || len(producerErrs) == 0 { | ||
| return err | ||
| } |
There was a problem hiding this comment.
My understanding of errors.As is that will check that it was cast into the expected type, and work with the interface.
It shouldn't mutate the passed value.
There was a problem hiding this comment.
The errors.As writes result to the second argument and returns boolean result of type casting. Thus it works with variable, not interface.
https://pkg.go.dev/errors#As
There was a problem hiding this comment.
Does my previous response clarify your concern?
|
Please address review and mark the PR as ready for review afterwards. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
an-mmx
force-pushed
the
feat/kafkaexporter_permanent_config_error
branch
from
1577beb to
d9c998a
Compare
| return consumererror.NewPermanent(confErr) | ||
| } | ||
|
|
||
| return kafkaErrors{len(prodErr), prodErr[0].Err.Error()} |
There was a problem hiding this comment.
This feels like we are loosing some information performing this. I might follow up in a seperate PR.
…prevent retries (open-telemetry#38608) #### Description This fix unifies message send error handling for all types of telemetry. It is designed to identify whether the error was caused by a ConfigurationError and then reclassify it as a permanent consumer error to prevent further retries. #### Link to tracking issue open-telemetry#38604 #### Testing Unit test coverage added #### Documentation No changes Co-authored-by: Antoine Toulme <[email protected]> Co-authored-by: Sean Marciniak <[email protected]>
…prevent retries (open-telemetry#38608) #### Description This fix unifies message send error handling for all types of telemetry. It is designed to identify whether the error was caused by a ConfigurationError and then reclassify it as a permanent consumer error to prevent further retries. #### Link to tracking issue open-telemetry#38604 #### Testing Unit test coverage added #### Documentation No changes Co-authored-by: Antoine Toulme <[email protected]> Co-authored-by: Sean Marciniak <[email protected]>
Description
This fix unifies message send error handling for all types of telemetry. It is designed to identify whether the error was caused by a ConfigurationError and then reclassify it as a permanent consumer error to prevent further retries.
Link to tracking issue
#38604
Testing
Unit test coverage added
Documentation
No changes