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

[Proposal] Make event.kind a list #2270

Open
andrewkroh opened this issue Sep 11, 2023 · 3 comments
Open

[Proposal] Make event.kind a list #2270

andrewkroh opened this issue Sep 11, 2023 · 3 comments
Labels
discuss enhancement New feature or request

Comments

@andrewkroh
Copy link
Member

Summary

event.kind is currently a string. I propose that its type be changed to a list of strings ([]string).

Motivation

The event.kind values are not mutually exclusive. Specifically, data that experiences a pipeline failure does not suddenly cease being of its original kind. For example, an alert that experiences a pipeline failure is still an alert, and it has value in being treated as such despite the pipeline failure.

Data processing pipelines that want to comply with ECS effectively must overwrite the original event.kind value on a failure, and this leads to a signal loss. As a user, I need to retain the original event.kind value for my queries to continue to function. It's OK for the pipeline to annotate the event in some way on failure, but it cannot remove or replace existing information.

Detailed Design

Assumptions

  • Changing the value from string to list of strings does not affect any existing Elasticsearch queries or dashboards.
  • If projects don't immediately migrate to using a list then nothing breaks.

Affected projects

This change would affect most projects generating ECS data because event.kind is a core field. The most common changes would be to switch Elasticsearch Ingest Node pipelines from using a set processor to an append processor anytime manipulations to event.kind are made. The most important change would be to on_failure processors where pipelines currently discard the existing event.kind value. For example:

diff --git a/packages/1password/data_stream/audit_events/elasticsearch/ingest_pipeline/default.yml b/packages/1password/data_stream/audit_events/elasticsearch/ingest_pipeline/default.yml
index 160a8c386..ab931a55d 100644
--- a/packages/1password/data_stream/audit_events/elasticsearch/ingest_pipeline/default.yml
+++ b/packages/1password/data_stream/audit_events/elasticsearch/ingest_pipeline/default.yml
@@ -135,9 +135,10 @@ processors:
       ignore_failure: true
       ignore_missing: true
 on_failure:
-  - set:
+  - append:
       field: event.kind
       value: pipeline_error
+      allow_duplicates: false
   - append:
       field: error.message
       value: '{{{ _ingest.on_failure_message }}}'
@andrewkroh andrewkroh added enhancement New feature or request discuss labels Sep 11, 2023
@Samrose-Ahmed
Copy link

Samrose-Ahmed commented Sep 11, 2023

I am not strictly against the reasoning but from the perspective of a breaking change this is quite unreasonable and strongly breaking, esp for a core field. ECS is used as a format outside of Elasticsearch and changing a string to a list is not a compatible change in other data systems.

@andrewkroh
Copy link
Member Author

@Samrose-Ahmed That's a good point about backends other than Elasticsearch.

Another idea I had been thinking about which is probably worth further exploration is to deprecate the pipeline_error value of event.kind (I view it as broken), and use some other field to annotate that a pipeline error occurred. Today we use error.message to give context about the pipeline_error, so it might make sense to do something like error.type: pipeline_error.

@Samrose-Ahmed
Copy link

I think that makes sense, the event kind never really changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants