-
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
Make elasticsearch/enrich metricset work for Stack Monitoring without xpack.enabled flag #20821
Make elasticsearch/enrich metricset work for Stack Monitoring without xpack.enabled flag #20821
Conversation
Pinging @elastic/integrations-services (Team:Services) |
Pinging @elastic/stack-monitoring (Stack monitoring) |
I added some fields into |
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.
We do not currently use this data in the Stack Monitoring UI. I'll approve the PR but I'm not sure how we want to handle this. I'm also fine with not doing this work since we don't use the data.
I would say that given that we have already done it, let's merge it and we can decide what to do as part of the migration. Thanks, @ycombinator I think this is ready for your final review, thanks! |
76a10b6
to
6cbc265
Compare
# Conflicts: # metricbeat/docs/fields.asciidoc # metricbeat/module/elasticsearch/fields.go # metricbeat/module/elasticsearch/node_stats/_meta/fields.yml
6cbc265
to
9e195af
Compare
🐛 Flaky test report❕ There are test failures but not known flaky tests. Expand to view the summary
Test stats 🧪
Genuine test errors
💔 There are test failures but not known flaky tests, most likely a genuine test failure.
|
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.
Left a couple of comments.
…without xpack.enabled flag (elastic#20615)
9e195af
to
a55741a
Compare
a55741a
to
001c44c
Compare
"name": "docker-cluster" | ||
}, | ||
"enrich": { | ||
"executed_searches": { |
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.
If I'm following this correctly, we are introducing a breaking change here, as we're going from elasticsearch.enrich.executed_searches
to elasticsearch.enrich.coordinator_stats.executed_searches
. Same applies to other fields as well. We should try to prevent this, maybe by introducing field aliases from the old to the new fields?
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.
Bump
Does there need to be a |
Yes. I have been struggling to run the "fields are documented" test in local until today and now I could manage to add all the missing fields in the mapping |
58e2a7d
to
9de5959
Compare
|
…csearch/enrich_xpack_flag # Conflicts: # metricbeat/docs/fields.asciidoc # metricbeat/module/elasticsearch/fields.go # metricbeat/module/elasticsearch/node_stats/_meta/fields.yml
… xpack.enabled flag (elastic#20821)
Ready to test in Kibana. data.json is included to help troubleshooting and it has been generating testing the Metricbeat binary directly with Elasticsearch