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

Use _doc_count data type when indexing histograms #4474

Closed
dgieselaar opened this issue Dec 1, 2020 · 2 comments · Fixed by #4647
Closed

Use _doc_count data type when indexing histograms #4474

dgieselaar opened this issue Dec 1, 2020 · 2 comments · Fixed by #4647

Comments

@dgieselaar
Copy link
Member

dgieselaar commented Dec 1, 2020

In the APM app, we are currently using value_count aggregations for transactions, as transaction metric documents use histogram fields, meaning doc_count is no longer representative. ES now also has a _doc_count data type that allows us to specify how many documents a transaction metric document represents. ES will then use this field in its bucketing aggregations, removing the need for an additional value_count aggregation, which should speed up some of our queries and remove some complexity from our queries.

https://www.elastic.co/guide/en/elasticsearch/reference/master/mapping-doc-count-field.html

@axw
Copy link
Member

axw commented Jan 21, 2021

I think this should be straightforward. I thought we would need to update betas and fields.yml; that is not the case.

To whomever implements this: I think we'll just need to add a field describing the doc count to model.Metricset, and use that to add a _doc_count field to the metric event docs that we index.

@jalvz
Copy link
Contributor

jalvz commented Feb 23, 2021

Checked _doc_count field in metrics matched the number of docs running the aggregation:

GET apm-7.12.0-transaction/_search
{
  "aggs": {
    "txnames": {
      "terms": { "field": "transaction.name" } 
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants