feat(contrib/IBM/sarama): add WithErrorCheck option - #5244
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4b4034576
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| err := p.SyncProducer.SendMessages(msgs) | ||
| for i, span := range spans { | ||
| finishProducerSpan(span, msgs[i].Partition, msgs[i].Offset, err) | ||
| finishProducerSpan(span, msgs[i].Partition, msgs[i].Offset, err, p.cfg) |
There was a problem hiding this comment.
When SendMessages returns sarama.ProducerErrors, err aggregates failures from the entire batch, but this passes that same aggregate to every message's checker. For a batch containing both an ignorable error and a real failure, a typical checker such as !errors.Is(err, ignoredErr) suppresses the aggregate—and therefore marks every span successful, including the span for the real failure. Match each ProducerError.Msg to its input message and finish each span with only that message's error.
Useful? React with 👍 / 👎.
| // span right away because there's no way to know when it will | ||
| // be done | ||
| span.Finish() | ||
| finishSpan(span, nil, cfg) |
There was a problem hiding this comment.
When Producer.Return.Successes is false—the default async-producer mode—this finishes the span immediately and never inserts it into spans. If the broker later emits a ProducerError, the error path cannot find the span, so WithErrorCheck is never invoked and even a checker returning true leaves the failed produce span marked successful. The new test masks this case by always enabling successes; the option must either support error outcomes in the default mode or explicitly reject/document that configuration.
Useful? React with 👍 / 👎.
| // WithErrorCheck specifies a function fn which determines whether the passed | ||
| // error should be marked as an error. The fn is called whenever a Kafka producer | ||
| // request finishes with an error. | ||
| func WithErrorCheck(fn func(err error) bool) OptionFn { |
There was a problem hiding this comment.
This public option introduces a new way for users to customize which producer failures mark spans as errors, but the commit does not update CONTRIBUTING.md to describe the feature or its usage constraints. Add the required documentation for this significant tracer customization, including the async-producer limitation and batch-error behavior.
AGENTS.md reference: AGENTS.md:L21-L24
Useful? React with 👍 / 👎.
|
/gardener shadow |
2 participants
What does this PR do?
Adds a
WithErrorCheckoption to the IBM/sarama wrapper which allows library users to drop specific errors from being sent to DataDog.Motivation
Implements the feature described in discussion #5243. Similar features have been merged for other wrappers such as in #773, #806, #1315.
Reviewer's Checklist
make lintlocally.make testlocally.make generatelocally.make fix-moduleslocally.Unsure? Have a question? Request a review!