-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Filebeat] Changes to text fields in elasticsearch module #10414
Conversation
Pinging @elastic/stack-monitoring |
jenkins, test this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog?
@@ -9,7 +9,6 @@ | |||
"elasticsearch.slowlog.logger": "index.indexing.slowlog.index", | |||
"elasticsearch.slowlog.routing": "", | |||
"elasticsearch.slowlog.source_query": "{\"@timestamp\":\"2018-07-04T21:50:40.799Z\",\"metricset\":{\"module\":\"system\",\"rtt\":9610,\"name\":\"network\"},\"system\":{\"network\":{\"name\":\"bridg\",\"in\":{\"packets\":0,\"errors\":0,\"dropped\":0,\"bytes\":0},\"out\":{\"errors\":0,\"dropped\":0,\"packets\":1,\"bytes\":342}}},\"beat\":{\"name\":\"Rados-MacBook-Pro.local\",\"hostname\":\"Rados-MacBook-Pro.local\",\"version\":\"6.3.0\"},\"host\":{\"name\":\"Rados-MacBook-Pro.local\"}}", | |||
"elasticsearch.slowlog.took": "221micros", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I spot now: Below the message fields seems to contain everything including timestamp. Is that expected?
For the took
. The plan is to extract the unit later or just remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't follow your question about the message
field?
For the took
, I think we should just the unit as we already have the actual numeric value in ms
, which seems more useful? BTW, this is something we're doing in the case of the elasticsearch/server
fileset as well:
"elasticsearch.server.gc.collection_duration.unit", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point about message is that once the header has been parsed and moved to other fields, only the rest of the message should be left in message
.
Right now, the message
fields looks like an event.original
, which is meant to contain the pristine initial copy of the event (if the user wants this).
For the took
field, I would keep it until we're ready to interpret it, no? Right now event.duration is based on the took_millis (removed by the pipeline), for simplicity, which is often rounded to 0ms. Elasticsearch is too fast. But I think it's worth keeping the custom field with the more precise timing until we can leverage it better (or until the ES log format gives us nanoseconds).
da19fbc
to
46e5803
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with the text field changes.
However I'd keep the took
custom field.
46e5803
to
22b9b6f
Compare
22b9b6f
to
f0ae6fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM
You just need to update fields.go accordingly :-)
jenkins, test this |
8606c1c
to
1c9c74f
Compare
jenkins, test this |
This PR is an offshoot of conversations and decisions made in #10372 w.r.t
text
fields, but scoped to theelasticsearch
module.