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

Expose failed processor using exception metadata #27724

Open
DaveCTurner opened this issue Dec 8, 2017 · 4 comments
Open

Expose failed processor using exception metadata #27724

DaveCTurner opened this issue Dec 8, 2017 · 4 comments
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >enhancement help wanted adoptme Team:Data Management Meta label for data/management team

Comments

@DaveCTurner
Copy link
Contributor

... instead of headers.

#18342 introduced some extra information on exceptions thrown by a compound processor, using headers, which is one of the many metadata mechanisms we currently have. As part of #27672 we'd like to reduce the variety of metadata mechanisms for the benefit of clients. Could this information be passed through using addMetadata() instead?

@talevy do you have any opinions on this?

@DaveCTurner DaveCTurner added the :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP label Dec 8, 2017
@DaveCTurner
Copy link
Contributor Author

Also @simonw said this: #16276 (comment) - do you (Simon) have opinions about preferring metadata over headers?

@DaveCTurner
Copy link
Contributor Author

The code in question is

if (processorType != null) {
exception.addHeader("processor_type", processorType);
}
if (processorTag != null) {
exception.addHeader("processor_tag", processorTag);
}

@talevy
Copy link
Contributor

talevy commented Dec 8, 2017

Thanks for bringing this up @DaveCTurner. At the time, headers were the only way to inject custom context without creating a new custom Exception (something that is frowned upon). I was not aware that metadata was added in 5.3 to clean up the semantics of these additional fields (#22703). It seems like metadata is the more appropriate place for this information given the direction of the code.

I will take a look at this and see if it is as I thought. We can update this code for 7.0.

@javanna
Copy link
Member

javanna commented Dec 8, 2017

Wasn't also not having to parse response body a reason for using headers over metadata? Do we know if any of our product reads this stuff returned as part of response headers?

@talevy talevy removed their assignment Jul 19, 2018
@talevy talevy added the help wanted adoptme label Jul 19, 2018
@rjernst rjernst added the Team:Data Management Meta label for data/management team label May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >enhancement help wanted adoptme Team:Data Management Meta label for data/management team
Projects
None yet
Development

No branches or pull requests

5 participants