chenBright opened a new pull request, #3549:
URL: https://github.com/apache/brpc/pull/3549
### What problem does this PR solve?
Issue Number: resolve #2012, resolve #2116
Problem Summary:
`LatencyRecorder` is exported to Prometheus by two independent emitters that
disagree on the latency format, and both emit a `quantile` label that is not
spec compliant:
- `PrometheusMetricsDumper::DumpLatencyRecorderSuffix` (single dimension)
dumps
the average as `quantile="avg"`. Per the Prometheus exposition format the
`quantile`
label value must be parsable as a float, so `avg` is illegal and makes
the whole summary
unparsable for strict clients.
- The `LatencyRecorder` specialization of `MultiDimension::dump_impl`
(mbvar) dumps
the average as a bare `N_latency{labels}` series, sharing the metric name
with the
`N_latency{labels,quantile=...}` percentile series. Since the average
carries no `quantile`
label, there is no way to filter it out, so any aggregation over
`N_latency` silently mixes
the average into the percentiles. For example a Grafana panel querying
`sum by (label1) (N_latency)` or `avg(N_latency)` adds the average on top
of `0.8` / `0.9` /
`0.99` / `0.999` / `0.9999`, and a legend grouped by `quantile` gets an
extra unlabeled entry.
- On top of that, the mbvar quantiles are integers (`quantile="80"`,
`"999"`, `"9999"`) instead
of fractions, which is also not spec compliant.
### What is changed and the side effects?
Changed:
- The average latency is dumped as a separate `N_latency_average` gauge on
both paths,
instead of `quantile="avg"` (single dimension) or a bare `N_latency`
series (mbvar). This
makes `N_latency` contain nothing but quantile series, so aggregating
over it no longer
picks up the average. On the single dimension path `N_latency_average` is
dumped before
the summary it belongs to, so the summary block stays contiguous.
- mbvar quantile labels are now fractions (`0.8` / `0.9` / `0.99` / `0.999`
/ `0.9999`), matching
the single dimension output. The `quantile` parameter of
`make_dump_key()` / `make_labels_kvpair_string()` is widened from `int`
to `double`, and a
non-positive value means "not a quantile series".
Side effects:
- Performance effects:
- Breaking backward compatibility:
---
### Check List:
- Please make sure your changes are compilable.
- When providing us with a new feature, it is best to add related tests.
- Please follow [Contributor Covenant Code of
Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
