Skip to content

Exemplar still gets updated(to one with empty labels) when providing no exemplar labels with the sample #616

Closed

Description

@zkx5xkt

I have a setup where I only upload a small percentage of my traces.
This means that for majority of my samples in a histogram there won't be a corresponding trace.

Current library implementation:

When presented with a new sample with no exemplar labels the library overrides the bucket's exemplar labels with an empty object essentially purging the existing exemplar

Example of metric after sample is published with an exemplar labels: example_duration_bucket{le="5"} 1 # {traceID="9899898"} 3.0005617 1708951967.457
Example of metric after sample is published with no exemplar labels afterwards: example_duration_bucket{le="5"} 1 # {} 3.0000431 1708975949.123

Desired library implementation:

When presented with a new sample with no exemplar labels the library should retain the existing exemplar so it can be scraped with the metric.

Example of metric after sample is published with an exemplar labels: example_duration_bucket{le="5"} 1 # {traceID="9899898"} 3.0005617 1708951967.457
Example of metric after sample is published with no exemplar labels afterwards: example_duration_bucket{le="5"} 1 # {traceID="9899898"} 3.0005617 1708951967.457

Example

import { Span, TraceFlags } from "@opentelemetry/api";
import { setTimeout } from "node:timers/promises";
import { Histogram } from "prom-client";

const histogram = new Histogram({
    name: "example_duration",
    help: "example duration",
    buckets: [0.5, 5],
    labelNames: [],
    enableExemplars: true,
});

async function example(span: Span) {
    const spanContext = span.spanContext();

    const exemplarLabels = spanContext.traceFlags & TraceFlags.SAMPLED ? {
        traceID: spanContext.traceId,
        spanID: spanContext.spanId
    } : undefined;

    const histogramTimer = histogram.startTimer(undefined, undefined);

    await setTimeout(3_000);

    histogramTimer(undefined, exemplarLabels);
}

currently for prom-client@15.1.0 i'm applying the following hotpatch for Histogram (do not overwrite exemplar values when no exemplar labels given):

--- node_modules/prom-client/lib/histogram.js	2024-02-26 06:50:35.132071300 +0200
+++ node_modules/prom-client/lib/histogram.js	2024-02-26 07:09:28.088071300 +0200
@@ -89,14 +89,16 @@
 		const bound = findBound(this.upperBounds, value);
 		const { bucketExemplars } = this.hashMap[hash];
 		let exemplar = bucketExemplars[bound];
-		if (!isObject(exemplar)) {
-			exemplar = new Exemplar();
-			bucketExemplars[bound] = exemplar;
+		if (Object.keys(exemplarLabels).length) {
+			if (!isObject(exemplar)) {
+				exemplar = new Exemplar();
+				bucketExemplars[bound] = exemplar;
+			}
+			exemplar.validateExemplarLabelSet(exemplarLabels);
+			exemplar.labelSet = exemplarLabels;
+			exemplar.value = value;
+			exemplar.timestamp = nowTimestamp();
 		}
-		exemplar.validateExemplarLabelSet(exemplarLabels);
-		exemplar.labelSet = exemplarLabels;
-		exemplar.value = value;
-		exemplar.timestamp = nowTimestamp();
 	}
 
 	async get() {

This can probably be done in a backwards compatible manner with an additional flag or something to specifically disable the overriding of exemplars when no exemplar is provided.

prom-client version: 15.1.0

Activity

  1. psimk commented on Mar 26, 2024

    @psimk
    Contributor

    same issue is present for counters as well, I created a similar patch for anyone wanting this feature:

    diff --git a/lib/counter.js b/lib/counter.js
    index 22a440eca40ed13d088789903eb4c04cc70e8128..8c5c1b0d9f2c2134460c248fbc181881cbaef063 100644
    --- a/lib/counter.js
    +++ b/lib/counter.js
    @@ -85,9 +85,12 @@ class Counter extends Metric {
     	}
     
     	updateExemplar(exemplarLabels, value, hash) {
    +		if (Object.keys(exemplarLabels).length === 0) return;
    +
     		if (!isObject(this.hashMap[hash].exemplar)) {
     			this.hashMap[hash].exemplar = new Exemplar();
     		}
    +
     		this.hashMap[hash].exemplar.validateExemplarLabelSet(exemplarLabels);
     		this.hashMap[hash].exemplar.labelSet = exemplarLabels;
     		this.hashMap[hash].exemplar.value = value ? value : 1;
    
  2. psimk commented on Mar 26, 2024

    @psimk
    Contributor
  3. SimenB commented on Mar 26, 2024

    @SimenB
    Contributor

    Happy to take a PR fixing this 🙂

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions