-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[WIP] Event object accessors and field reference API #5035
Comments
We need to bear this in mind #4743 |
@guyboertje can you please provide a tl;dr here for what we should bear in mind in #4743 ? |
OK This can be made to happen in the #4743 - Discussed various mitigation approaches. |
In the same vein, should we forbid/allow the replacement of a Map with a scalar? |
@guyboertje these are very valid concerns and should be properly defined in the API. maybe we can start with the legacy behaviour and see what makes sense moving forward? |
@guyboertje I am copying your accessors path invalidation bug sequence from #5212 : > event = LogStash::Event.new
=> #<LogStash::Event:0x25ce435 @metadata_accessors=#<LogStash::Util::Accessors:0x7ea71fc2 @store={}, @lut={}>, @cancelled=false, @data={"@version"=>"1", "@timestamp"=>"2016-04-15T14:19:31.561Z"}, @metadata={}, @accessors=#<LogStash::Util::Accessors:0x7cd5fcf4 @store={"@version"=>"1", "@timestamp"=>"2016-04-15T14:19:31.561Z"}, @lut={}>>
> event["[a][0]"]
=> nil
> event["[a][0]"] = 42
=> 42
> event["[a]"]
=> {"0"=>42}
> event["[a]"] = [42,24]
=> [42, 24]
> event["[a]"]
=> [42, 24]
> event["[a][0]"]
=> 42
> event["[a]"] = [24,42]
=> [24, 42]
> event["[a][0]"]
=> 42
> event["[a][0]"] = {"a"=> 99, "b" => 98}
=> {"a"=>99, "b"=>98}
> event["[a][0]"]
=> {"a"=>99, "b"=>98}
> event["[a]"]
=> [24, 42]
> event["[a][0]"]
=> {"a"=>99, "b"=>98}
> event["[a][0][b]"]
TypeError: can't convert String into Integer
from org/jruby/RubyFixnum.java:1143:in `[]' I wrote a spec with this sequence and confirm the problem on both the Ruby and Java Event implementations. |
Actually ^^ I will move that into a separate issue - #5127 |
This ruby hash-like Event API will be dropped in 5.0, see #5140 - closing |
The new Java Event implementation changes the way plugins or Ruby filter scripts can interact with the Event object, in particular for setting fields values in nested objects. Some implementation specific assumptions cannot be made anymore as we move forward with new implementation strategies and eventually offer new alternate languages APIs.
This brings forward the need to clearly define the Event object API from a plugin or Ruby filter script developer. I believe this API was never formally defined and the 5.0 release cycle would be a good time to correct this.
To help with the evolution of the implementation, we should avoid relying on implementation and language specific idioms and provide a clear API and guidelines to our core objects from a plugin development context.
In that context and precisely for the Event accessors and field reference case, we should adopt the field reference syntax as the only way to manipulate Event inner values and we should document that the returned values when looking up a field reference are copies of the Event values and modifying these values is not guaranteed to automatically update the Event. An updated value must by explicitly written back into the event using the field reference setter syntax.
For example, imagine an Event which has this nested structure:
Moving forward the following construct to update the value in the
[foo][bar]
path would not be supported:Note in the example above, it is assumed that
event["foo"]
returns a reference to a Ruby hash that contains thebar
key which can be in-place updated.Likewise with this same assumption but written a bit differently:
The proper way is to explicitly set an updated value using the complete field reference like this:
Or alternatively:
This relates to #5028 #4834 #4999
The text was updated successfully, but these errors were encountered: