search

siimon / prom-client

Prometheus client for node.js
Apache License 2.0
3.09k stars 372 forks source link

fix: avoid updating exemplar values during subsequent metric changes #620

Closed psimk closed 5 months ago

psimk commented 5 months ago

Problem

The current exemplars implementation expects that every time we update a metric, we will have fresh exemplar labels to set together with the metric. However, setups that have a low sampling rate for their exemplar labels (e.g. trace ids), this means that this implementation would be reseting exemplar labels.

This also creates a lot of noise in graphing solutions like Grafana as there are lots of exemplars displayed without any contextual data attached to them.

Solution

Before setting an exemplar, or even creating a brand new one, we first will check whether the the exemplar labels were configured with anything other that isn't the default empty object. If there are no exemplar labels, then we simply short circuit the function.

Note, this likely means that we could get rid off the defaultExemplarLabelSet field and just handle the undefined value, but I wanted to have the least amount of changes in the PR.

fixes https://github.com/siimon/prom-client/issues/616

@SimenB

SimenB commented 5 months ago

Sweet, thanks! Gauge and Histograms are not affected?

psimk commented 5 months ago

Sweet, thanks! Gauge and Histograms are not affected?

Gauge's don't seem to have exemplars and I made the change for histograms in lib/histogram.js.

SimenB commented 5 months ago

👍

(I meant summary, but I see there's no exemplar support there either).

Could you add a changelog entry? (also, CI is failing)

psimk commented 5 months ago

Could you add a changelog entry? (also, CI is failing)

Added.