-
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
[discuss] accept @timestamp numeric value as epoch in event initialization #10369
base: main
Are you sure you want to change the base?
Conversation
In fact there was a single spec in |
I found some history around this and the seconds vs milliseconds epoch values, see #2922 I think we could go one step further here and add logic to see if the numeric value is |
Historically, I agree that we should make a best effort to interpret an inbound field called I would prefer a slightly more-conservative magic number of
SEE: There are a couple of things we should also consider if we are opening up the acceptable input values for
|
@yaauie The string case is a bit trickier to handle since we first need to assess that the string in question is a numeric representation possibly using a regex. I am not against the idea but not sure if we should necessarily have to explore this case if we think handling the simple numeric case is a step forward. Same for float representations. I came up with this because we had a recent concrete use-case. If this proposal which seems to me like a more common case makes sense we can move forward with it and create a new issues for the string handling proposals? WDYT? |
I added a commit for the magic number change and some specs. LMKWYT. |
Circling back - any concerns for not moving forward with this? /cc @jsvd |
@colinsurprenant since |
Found some more history in #2929 and #2998 - seems at that time we dropped that idea. In any case, this PR is the result of investigating an issue where the input data had an epoch Since we already try to some extend to coerce a source |
@jsvd We have a use case where this PR would be helpful. We are currently using Elastic Rally to benchmark dev environments and having the results stored in an elasticsearch instance in those environments. We then use logstash to process and ship those indices to our production ES instance. Rally stores the timestamp field as an epoch time and when using the Elasticsearch Input Plugin, it is unable to parse the timestamp resulting in us using the following workaround.
It took me a while to find this discussion to help explain why my config was throwing this timestamp parse failure. This PR would be a great solution and allow us to stop using this workaround. Is there anything else blocking this PR from being applied? Thanks |
Jenkins test this |
The build errors look unrelated. |
While looking at a
@timestamp
issue inEvent
initialization I ran into this problem; numeric values in a@timestamp
field atEvent
initialization are not supported.It seems fair to assume that a numeric value is in fact an epoch value. After this patch this now works:
I haven't found much specs around the accepted initialization values for the Event
@timestamp
field - unless there is objection to this patch I will add a better spec suite for that.