-
Notifications
You must be signed in to change notification settings - Fork 524
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
Add support for histograms to metrics intake #5360
Conversation
e2b6d5c
to
2fc5e46
Compare
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
@beniwohli @felixbarny FYI. Would be good to get your eyes at least on the changes in docs/schema and testdata: https://github.com/elastic/apm-server/pull/5360/files?file-filters%5B%5D=.json&file-filters%5B%5D=.ndjson |
2fc5e46
to
89ee1e2
Compare
89ee1e2
to
1dca4c8
Compare
jenkins run the tests please |
* model/modeldecoder: add metric type and unit * systemtest: test histogram metrics * Update changelog * systemtest: fix min docs expectation in test (cherry picked from commit 0744428) # Conflicts: # changelogs/head.asciidoc
…5381) * Add support for histograms to metrics intake (#5360) * model/modeldecoder: add metric type and unit * systemtest: test histogram metrics * Update changelog * systemtest: fix min docs expectation in test (cherry picked from commit 0744428) # Conflicts: # changelogs/head.asciidoc * Delete head.asciidoc Co-authored-by: Andrew Wilkins <[email protected]>
The spec for histograms has been merged in elastic/apm-server#5360
Tested with BC2 with an example request received from @beniwohli (testing with opbeans app did not yet fully work): histogram data are indexed in ES. |
* catch and log exceptions in the interval timer function Instead of letting the thread die, we log the exception with a stack trace to ease debugging. * implement histogram metrics and a prometheus histogram bridge The spec for histograms has been merged in elastic/apm-server#5360 * trying to debug failure that only happens on CI... * adapt prometheus histograms to use centroids instead of upper limit buckets * move midpoint calculation into base metrics
* catch and log exceptions in the interval timer function Instead of letting the thread die, we log the exception with a stack trace to ease debugging. * implement histogram metrics and a prometheus histogram bridge The spec for histograms has been merged in elastic/apm-server#5360 * trying to debug failure that only happens on CI... * adapt prometheus histograms to use centroids instead of upper limit buckets * move midpoint calculation into base metrics
Motivation/summary
Extend the metricset schema with additional fields on "samples": type, unit, values, and counts. For now, the only type that is honoured is "histogram", and units are not honoured at all. We need a resolution on elastic/elasticsearch#72536 to handle gauge/counter and units.
If type is "histogram", then you are expected send "counts" and "values". Counts must be a list of non-negative integers. Values must be a list of (any) numbers; the values must be provided in ascending order.
With this change, one of "value" or "values" must be supplied. Previously "value" was required, so sending a histogram to an older server will result in a validation error. If "values" is supplied, "counts" must also be supplied.
Checklist
- [ ] Documentation has been updatedHow to test these changes
Once one of our agents has implemented support for sending histogram metrics (probably via the Prometheus API?), use one to send to APM Server and ensure the metric is stored in a histogram field with expected counts/values.
Related issues
Closes #4884
Closes #3195