Skip to content
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

Fix default_field for metricbeat #7015

Merged
merged 1 commit into from
May 22, 2018
Merged

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented May 4, 2018

In Elasticsearch 7.0 there is a limit of 1024 fields (see #5275). As Metricbeat exceeds the limit of 1024 fields per index it has to be defined which the default fields are as otherwise Elasticsearch returns an error.

This change set the index config option index.query.default_field. To fields chosen to be searchable are all string and keyword values as these are the ones that I would expect users to put directly in the search without specifying a field. For all numbers I expect that the exact field is specified.

This PR also removes all presets from dashboards which did set default_fields as they were *. In Kibana the default * was removed also in master: elastic/kibana#16232

@ruflin ruflin added in progress Pull request is currently in progress. discuss Issue needs further discussion. libbeat :Dashboards labels May 4, 2018
@ruflin
Copy link
Contributor Author

ruflin commented May 4, 2018

@Bargs Few questions to default_field in Kibana related to elastic/kibana#16232

  • What are the priorites of default_field in Kibana. If a visualization has it, will it overwrite the global setting?
  • And will the global setting overwrite the Elasticsearch setting?
  • Is there a way to overwrite the global default_field setting per index pattern?

@Bargs
Copy link

Bargs commented May 4, 2018

What are the priorites of default_field in Kibana. If a visualization has it, will it overwrite the global setting?

I believe the global option will take precedence for the query from the query bar. However, in 7.0 query_string filters will no longer get the default options, so one option might be to add a query_string filter to the vis instead of using the query bar.

And will the global setting overwrite the Elasticsearch setting?

Yes, it is sent as a param on the query_string query in the search request body, so it takes precedence over the index settings in ES.

Is there a way to overwrite the global default_field setting per index pattern?

Nope

@ruflin
Copy link
Contributor Author

ruflin commented May 8, 2018

@Bargs Thanks for the details. Do you have a time frame when elastic/kibana#16232 could happen? Because I think it is also prerequisit for this PR to fully fix the problem without having to overwrite it in each visualisation.

@Bargs
Copy link

Bargs commented May 8, 2018

It's easy to do, I've just been putting it off. If it's blocking you guys I'll try to get it done in the next few days.

@Bargs
Copy link

Bargs commented May 9, 2018

Here we go elastic/kibana#18966

@ruflin
Copy link
Contributor Author

ruflin commented May 11, 2018

@Bargs Thanks a lot for this. This will make it much easier for us / me to test if my above solution to the problem actually works.

@ruflin
Copy link
Contributor Author

ruflin commented May 15, 2018

I tested this again with the most recent snapshot builds from Kibana and for the system dashboards this seems to fix the problem.

An other thing I stumbled over partially related is that the searchSourceJSON is always a bit different for the visualisations. I don't think it depends on the order how the visualisations were built. The outcome is normally the same. A few example:

"searchSourceJSON": "{\"query\":{\"query\":{\"query_string\":{\"analyze_wildcard\":true,\"query\":\"*\"}},\"language\":\"lucene\"},\"filter\":[]}"

"searchSourceJSON": "{\"index\":\"metricbeat-*\",\"query\":{\"query\":{\"query_string\":{\"query\":\"*\",\"analyze_wildcard\":true,\"default_field\":\"*\"}},\"language\":\"lucene\"},\"filter\":[]}"

"searchSourceJSON": "{\"query\":{\"query\":{\"query_string\":{\"query\":\"*\"}},\"language\":\"lucene\"},\"filter\":[]}"

All similar but not 100% identical. Should we standardise these after the export to make sure they are all the same?

@exekias
Copy link
Contributor

exekias commented May 15, 2018

💯 we had issues in the past with wrong values here, AFAIK index must be set to metricbeat-*, for instance

@ruflin ruflin added review and removed discuss Issue needs further discussion. in progress Pull request is currently in progress. labels May 17, 2018
@ruflin
Copy link
Contributor Author

ruflin commented May 17, 2018

I changed the PR that for now default_fields only apply when running against ES 7. This makes sure it's not a breaking change in case it has side effects. It also gives us time to test it.

@exekias Lets sync up separately on how we should "clean" our dashboards. Ready for review.

Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, should this have a changelog entry?

In Elasticsearch 7.0 there is a limit of 1024 fields (see elastic#5275). As Metricbeat exceeds the limit of 1024 fields per index it has to be defined which the default fields are as otherwise Elasticsearch returns an error.

This change set the index config option `index.query.default_field`. To fields chosen to be searchable are all string and keyword values as these are the ones that I would expect users to put directly in the search without specifying a field. For all numbers I expect that the exact field is specified.

This PR also removes all presets from dashboards which did set default_fields as they were `*`. In Kibana the default `*` was removed also in master: elastic/kibana#16232
@ruflin
Copy link
Contributor Author

ruflin commented May 22, 2018

@exekias Yes, added one and rebase on master.

@exekias
Copy link
Contributor

exekias commented May 22, 2018

WFG

@exekias exekias merged commit 539c07e into elastic:master May 22, 2018
@ruflin ruflin deleted the default-fields branch March 12, 2019 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants