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

Histogram metric support #3195

Closed
1 task
axw opened this issue Jan 16, 2020 · 9 comments · Fixed by #5360
Closed
1 task

Histogram metric support #3195

axw opened this issue Jan 16, 2020 · 9 comments · Fixed by #5360

Comments

@axw
Copy link
Member

axw commented Jan 16, 2020

Currently, the metrics API only supports recording single value metrics: counters and gauges. We would like to extend the API to also support ingesting pre-aggregated histograms, storing as the histogram field type.

For dynamically mapping histograms, we are currently waiting for elastic/elasticsearch#61939.

@axw axw added this to the 7.7 milestone Jan 16, 2020
@axw
Copy link
Member Author

axw commented Jan 16, 2020

For dynamic field mapping, we could use [match_mapping_type](https://www.elastic.co/guide/en/elasticsearch/reference/master/dynamic-templates.html#match-mapping-type): object.

This might be a bit too lax, as it would assume all objects we index are histograms, and might prevent adding support for aggregate_metric later. An extension of Elasticsearch's match_mapping_type: object to also match on the types of object properties might be helpful here. i.e. also require "values" and "counts" fields, with the appropriate types.

Alternatively we could enforce a specific field naming, e.g. prefix all histograms with "histogram.". That feels like a cop out to me though, forcing implementation details onto the user.

@simitt
Copy link
Contributor

simitt commented Jan 17, 2020

Alternatively we could enforce a specific field naming, e.g. prefix all histograms with "histogram.". That feels like a cop out to me though, forcing implementation details onto the user.

Based on the fixed format for histogram fields, we might also be able to detect histograms when processing the input in the apm-server and then add a specific prefix, so the convention is not on the user but auto-applied. In case users search for their specific metric name this might still be an issue. Maybe nesting all of those metrics under a histogram key might be an option.

@axw
Copy link
Member Author

axw commented Jan 18, 2020

I wasn't very clear in my comment, but that's what I meant by that bit you quoted. i.e. store everything with "histogram" as the root key. I think this would be the simplest option working with what we've got, but it means that users need to prefix everything with "histogram." in their queries.

Again I wasn't clear, but that's what I meant by "forcing implementation details onto the user". Application developers would record a metric with one name, but then it ends up getting this "histogram." prefix because of implementation details/constraints. Not the end of the world, but I think it would be nice if we could avoid it.

@axw
Copy link
Member Author

axw commented Jan 23, 2020

Opened an ES feature request: elastic/elasticsearch#51341

@axw
Copy link
Member Author

axw commented Feb 5, 2020

Ran into this odd behaviour while doing some experimentation yesterday: elastic/elasticsearch#51847

Also I created a branch with support for histogram metrics: https://github.com/axw/apm-server/tree/histogram-metrics. Very hacky, and requires explicitly defining field mappings in the index template.

@cyrille-leclerc
Copy link
Contributor

Can we align the histogram support through APM Server with the Histogram support via Metricbeat, for example for Prometheus, as documented on https://www.elastic.co/guide/en/beats/metricbeat/current/metricbeat-metricset-prometheus-collector.html#_histograms_and_types

@axw
Copy link
Member Author

axw commented Dec 3, 2020

Can we align the histogram support through APM Server with the Histogram support via Metricbeat

We can, but I'm not convinced we should. I would prefer that we improve the foundation (Elasticsearch) and provide a single, zero-configuration approach.

@axw
Copy link
Member Author

axw commented Apr 23, 2021

https://github.com/axw/apm-server/tree/histogram-metrics-ingest has a basic working prototype. Summary:

  • adds a dynamic template called _metric_histogram with "mapping": {"type": "histogram"}
  • adds a apm_metrics_dynamic_template ingest pipeline, which sets the _dynamic_templates meta field from a _metric_descriptions field that may be added to metric docs; this field will be removed during ingest
  • updates the metric event doc transformer to set _metric_descriptions for histogram metrics
  • updates the OpenTelemetry metrics consumer to record histograms

No support for summaries yet. There are some complications around optionally supporting min+max sub-fields.

@axw
Copy link
Member Author

axw commented May 6, 2021

Part one: #5239

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.

4 participants