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

Allow KeyValue processor to write parsed entries to event root #3378

Closed
oeyh opened this issue Sep 21, 2023 · 3 comments · Fixed by #3380
Closed

Allow KeyValue processor to write parsed entries to event root #3378

oeyh opened this issue Sep 21, 2023 · 3 comments · Fixed by #3380
Labels
enhancement New feature or request

Comments

@oeyh
Copy link
Collaborator

oeyh commented Sep 21, 2023

Is your feature request related to a problem? Please describe.
KeyValue processor currently writes parsed entries to a user specified destination (default to "parsed_message") but doesn't support writing those entries directly to event root.

Describe the solution you'd like
When destination is set to null, we write parsed entries to root. The config would look like this:

...
processor:
  key_value:
    destination:

or

...
processor:
  key_value:
    destination: null

Describe alternatives you've considered (Optional)
Unless user specifies a destination, writing to root as default behavior. But this is going to be a breaking change.

Additional context
parse_json has similar configuration that when target is null, the parsed results will be written to root.

@dlvenable
Copy link
Member

@oeyh , Can we support destination: / instead of using null? The null approach is not intuitive to me.

Also, we can make null an allowed approach by writing to the root by default. I don't think we want users writing destination: null as this indicates something completely different.

@oeyh
Copy link
Collaborator Author

oeyh commented Sep 25, 2023

@dlvenable The null approach is consistent with what we have in other processors (target_key in grok, destination in parse_json). I thought about writing to root by default and put it in the alternative approach above, but it's going to be a breaking change because destination in this processor has a default value of "parsed_message".

Supporting "/" makes a lot of sense, and is more intuitive. But I think we should support that in the event level (i.e. the JacksonEvent.put() method). We can make it support full json pointer syntax, including root "/", using something like JsonNode.at() instead of using a custom parser. This will potentially also help resolve other issues like this one: #2416. I can make an issue for it.

@dlvenable
Copy link
Member

The consistency is good. But, at the same time, I think we should try to find a better way going forward.

I think using / is the clearest approach.

I thought about writing to root by default and put it in the alternative approach above, but it's going to be a breaking change because destination in this processor has a default value of "parsed_message".

Good point. We should not change this.

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

Successfully merging a pull request may close this issue.

2 participants