feat: Add validators for pipeline on_failure handler #1038
Conversation
- Add validation rule SVR00007 to verify all ingest pipelines set event.kind to pipeline_error in the global on_failure handler. - Add validation rule SVR00008 to verify all ingest pipelines set error.message in the global on_failure handler and that the failing processor type, tag, message, and pipeline are included in the error message.
|
/test |
code/go/internal/validator/semantic/validate_pipeline_on_failure.go
Outdated
Show resolved
Hide resolved
mrodm
left a comment
mrodm
left a comment
There was a problem hiding this comment.
Thanks for adding the support to validate ingest pipelines defined in the package root. Added a couple of minor comments.
| pipelineFileMetadatas = append(pipelineFileMetadatas, pipelineFileMetadata{ | ||
| filePath: file, | ||
| fullFilePath: fsys.Path(file), | ||
| dataStream: d.dataStream, |
There was a problem hiding this comment.
Is this dataStream field form pipelineFileMetadata struct used ?
Taking a look at #1010 and this PR, it looks like it is not. Could you check it? If not, should it be kept or used somewhere?
There was a problem hiding this comment.
While I haven't used the dataStream field yet, I've added it to keep it consistent with listFieldsFiles and fieldFileMetadata, which is what this function and struct were based on.
| return nil, fmt.Errorf("cannot read pipeline files from integration package: %w", err) | ||
| type pipelineDirMetadata struct { | ||
| dir string | ||
| dataStream string |
There was a problem hiding this comment.
Maybe if dataStream field from pipelineFileMetadata can be removed, this one too could be removed.
|
@taylor-swanson i merged #1042 and used some codes as yours. please update the pr to resolve conflict. thnks! |
Not a problem, will get that fixed up! |
💚 Build Succeeded
History
|
teresaromero
left a comment
teresaromero
left a comment
There was a problem hiding this comment.
LGTM, i guess this comment https://github.com/elastic/package-spec/pull/1038/changes#r2626165855 is already resolved by this one too https://github.com/elastic/package-spec/pull/1038/changes#r2627155051 ?? cc @mrodm
4 participants
What does this PR do?
Checklist
test/packagesthat prove my change is effective.spec/changelog.yml.Related issues