-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cherry-pick #16215 to 7.x: azure-event hub: improve error handling and stop input if the event has not been processed correctly #17195
Conversation
…as not been processed correctly (elastic#16215) * work on GA * update changelog * go vet * work on error * error handling * error message * temp * temp * undo tests * update * integration * fmt update * upgrade * upgrade * mage vendor * notice (cherry picked from commit 1d3bc8c)
Pinging @elastic/integrations (Team:Integrations) |
Pinging @elastic/integrations-platforms (Team:Platforms) |
go.sum
Outdated
github.com/antihax/optional v0.0.0-20180407024304-ca021399b1a6/go.mod h1:V8iCPQYkqmusNa815XgQio277wI47sdRh1dUOLdyC6Q= | ||
github.com/antlr/antlr4 v0.0.0-20200225173536-225249fdaef5 h1:nkZ9axP+MvUFCu8JRN/MCY+DmTfs6lY7hE0QnJbxSdI= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should not be here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ran mage vendor
, updated go.sum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm mostly concerned about https://github.com/elastic/beats/pull/16215/files#diff-f949e2d81c8076ebbf8af38fcbb72c1fR111 and https://github.com/elastic/beats/pull/16215/files#diff-f949e2d81c8076ebbf8af38fcbb72c1fR734 from the original PR which are absent now. Is this normal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@narph I think that dependencies would need a revisit. Also the link in the original PR is wrong.
Cherry-pick of PR #16215 to 7.x branch. Original message:
Should help with issue #15671
Previously, if the event was not successfully processed, the checkpoint would have still been updated and the input would move to the next set of messages.
compositeHandlers
method returns nil either way https://github.com/elastic/beats/blob/master/vendor/github.com/Azure/azure-event-hubs-go/v3/eph/eph.go#L406so the checkpoint is updated https://github.com/elastic/beats/blob/master/vendor/github.com/Azure/azure-event-hubs-go/v3/receiver.go#L242.
In the new release 3.1.2 for the azure-event-hub sdk, the error is returned https://github.com/Azure/azure-event-hubs-go/blob/v3.1.2/eph/eph.go#L423 so the checkpoint is not updated anymore.
This is still not e2e ack as this does not stop if the events were not sent to ES.