Skip to content
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

azure-event hub: improve error handling and stop input if the event has not been processed correctly #16215

Merged
merged 20 commits into from
Mar 24, 2020

Conversation

narph
Copy link
Contributor

@narph narph commented Feb 10, 2020

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#L406
so 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.

@narph narph requested a review from exekias February 10, 2020 16:42
@narph narph self-assigned this Feb 10, 2020
@narph narph added Filebeat Filebeat Team:Platforms Label for the Integrations - Platforms team labels Feb 10, 2020
@exekias
Copy link
Contributor

exekias commented Feb 11, 2020

This is great! thank you for looking into it. ey @narph, I'm missing where we return the error that goes up in the chain so EPH detects it. Can you point to it in the code?

@exekias
Copy link
Contributor

exekias commented Mar 3, 2020

This is not implementing e2e ACK, yet it handles the stopping better, as it won't checkpoint documents that haven't been sent. Could you please update changelog & description to describe what the change is doing?

e2e is something we still want to implement in the future, let's make sure people don't get confused by this change

@narph narph changed the title azure-event hub: implement-e2e ack azure-event hub: improve error handling and stop input if the event has not been processed correctly Mar 3, 2020
@narph narph added the needs_backport PR is waiting to be backported to other branches. label Mar 24, 2020
@narph narph merged commit 1d3bc8c into elastic:master Mar 24, 2020
@narph narph deleted the azure-input-ga branch March 24, 2020 08:47
narph added a commit to narph/beats that referenced this pull request Mar 24, 2020
…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)
narph added a commit that referenced this pull request Mar 24, 2020
…d stop input if the event has not been processed correctly (#17195)

* azure-event hub: improve error handling and stop input if the event has not been processed correctly (#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)

* ran mage vendor
@narph narph added test-plan Add this PR to be manual test plan v7.7.0 and removed needs_backport PR is waiting to be backported to other branches. test-plan Add this PR to be manual test plan labels Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Filebeat Filebeat Team:Platforms Label for the Integrations - Platforms team v7.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants