Introduce on_failure_pipeline ingest metadata inside on_failure block #49076
Conversation
In case an exception occurs inside a pipeline processor, the pipeline stack is kept around as header in the exception. Then in the on_failure processor the id of the pipeline the exception occurred is made accessible via the `on_failure_pipeline` ingest metadata. Closes elastic#44920
martijnvg
added
>enhancement
:Data Management/Ingest Node
|
Pinging @elastic/es-core-features (:Core/Features/Ingest) |
There was a problem hiding this comment.
This generally looks good. I've got a few minor suggestions and a question. Thanks @martijnvg
| @@ -279,6 +282,41 @@ public void testBreakOnFailure() throws Exception { | |||
| assertStats(pipeline, 1, 1, 0); | |||
| } | |||
|
|
|||
| public void testFailurePipelineField() { | |||
There was a problem hiding this comment.
nitpick would it make sense to rename this to what it tests? eg testFailureProcessorIsInvokedOnFailure
| } | ||
|
|
||
| private ElasticsearchException newCompoundProcessorException(Exception e, String processorType, String processorTag) { | ||
| private ElasticsearchException newCompoundProcessorException(Exception e, Processor processor, IngestDocument document) { |
There was a problem hiding this comment.
should we make this package accessible and add some tests?
There was a problem hiding this comment.
I've added test for this method: 0772a99
| if (processorTag != null) { | ||
| exception.addHeader("processor_tag", processorTag); | ||
| } | ||
| List<String> pipelineStack = document.getPipelineStack(); | ||
| if (pipelineStack.size() > 1) { | ||
| exception.addHeader("pipeline_origin", pipelineStack); |
There was a problem hiding this comment.
is the reason we are providing a stack here because we can't pin point which pipeline in the stack failed? I might be off here but would it make sense to pass the pipeline id to the handlers and report the exact pipeline that failed? https://github.com/elastic/elasticsearch/pull/49076/files#diff-197c33567939d6a2a4f637dda45b72dcR652
There was a problem hiding this comment.
The reason is because there may be several levels of nested pipeline processors and then it is not clear if we just report the pipeline name that has failed. A pipeline may be used multiple times from the same initial pipeline. Having a stack here, really pin points to the pipeline and how it executed to that point.
|
@elasticmachine update branch |
|
@elasticmachine update branch |
|
Thanks for reviewing @andreidan. |
…elastic#49076) In case an exception occurs inside a pipeline processor, the pipeline stack is kept around as header in the exception. Then in the on_failure processor the id of the pipeline the exception occurred is made accessible via the `on_failure_pipeline` ingest metadata. Closes elastic#44920
…lure block (#49596) Backport of #49076 In case an exception occurs inside a pipeline processor, the pipeline stack is kept around as header in the exception. Then in the on_failure processor the id of the pipeline the exception occurred is made accessible via the `on_failure_pipeline` ingest metadata. Closes #44920
In case an exception occurs inside a pipeline processor,
the pipeline stack is kept around as header in the exception.
Then in the on_failure processor the id of the pipeline the
exception occurred is made accessible via the
on_failure_pipelineingest metadata.
Closes #44920