Support dynamic target field names#30
Support dynamic target field names#30magnusbaeck wants to merge 1 commit intologstash-plugins:mainfrom
Conversation
|
|
||
| if @target | ||
| event.set(@target, parsed) | ||
| event.set(event.sprintf(@target), parsed) |
There was a problem hiding this comment.
There's room for confusion here that a user may want a value from the parsed object to dictate what field is set. If we're going to allow this behavior, I think we need to document what does and doesn't work for this plugin. Thoughts?
There was a problem hiding this comment.
Good point @jordansissel! I think it'd be nice to support pulling a field from the parsed object, but how would that addressing even work since we also need to support indirect addressing based on existing fields?
I'm leaning towards keeping the existing behavior but documenting it (I forgot about that in this first version of the patch). Selecting the target field based on fields from the parsed object is possible via an extra mutate operation afterwards so we won't be blocking anything with this limitation. Sounds good to you?
|
@magnusbaeck Are you still planning on moving ahead with this change? It appears that #29 has been merged and this would be a useful addition, even with the (to be documented) limitation. |
Yes, I've just been too lazy to complete it. I'll try to get to it during the next couple of days. |
It's conceivable that one would want the target field name to be chosen dynamically using the contents of another field, so pass the target option value through sprintf.
magnusbaeck
force-pushed
the
dynamic-target
branch
from
5e3553e to
4fb72d8
Compare
|
@jordansissel, I've updated this PR and documented the behavior that you had a question about. Please have another look. |
4 participants
It's conceivable that one would want the target field name to be chosen dynamically using the contents of another field, so pass the target option value through sprintf. Fixes #28.
Tests will fail until #29 is merged.