siimon / prom-client

Prometheus client for node.js
Apache License 2.0
3.15k stars 378 forks source link

Metrics rendering error with version 15.0.0-0 #554

Closed vothanhbinhlt closed 1 year ago

vothanhbinhlt commented 1 year ago

Hello everyone. First, I would like to thank you for merging PR to support exemplars in this package. I am very excited. I am trying version 15.0 in my current project and fact with an issuer of metrics rendering here is an example:

We are using: typescript version 4.8.4 and nestjs framework

# metrics.ts
import * as promClient from 'prom-client';

export const register = promClient.register;

const fakeRegistry: any = promClient.Registry; // I have to cheat here because It seems type of the library have not supported
register.setContentType(fakeRegistry.OPENMETRICS_CONTENT_TYPE);

// promClient.collectDefaultMetrics({ register });
export const histogram = new promClient.Histogram({
  name: 'http_request_duration_seconds',
  help: 'Duration of HTTP requests in seconds',
  labelNames: ['method', 'path', 'status', 'traceID', 'spanID'],
  buckets: [0.5, 1, 2, 5, 10, 60],
  enableExemplars: true,
});

// Register the histogram
register.registerMetric(histogram);
# httpmiddleware.ts
...
    histogram.observe({
      labels: {
        method,
        path,
        status: statusCode,
      },
      value: time / 1000,
      exemplarLabels: {
        traceID: spanContext.traceId,
        spanID: spanContext.spanId,
      },
    });
...

Results

# HELP http_request_duration_seconds Duration of HTTP requests in seconds
# TYPE http_request_duration_seconds histogram
http_request_duration_seconds_bucket{le="0.5",method="GET",path="/metrics",status="200"} 1
 # {traceID="659eb21633214be74be9b03afa6f7105",spanID="cbb23fa4a5669144"} 0.04501697799999999 1678595397.754
http_request_duration_seconds_bucket{le="1",method="GET",path="/metrics",status="200"} 1
http_request_duration_seconds_bucket{le="2",method="GET",path="/metrics",status="200"} 1
http_request_duration_seconds_bucket{le="5",method="GET",path="/metrics",status="200"} 1
http_request_duration_seconds_bucket{le="10",method="GET",path="/metrics",status="200"} 1
http_request_duration_seconds_bucket{le="60",method="GET",path="/metrics",status="200"} 1
http_request_duration_seconds_bucket{le="+Inf",method="GET",path="/metrics",status="200"} 1
http_request_duration_seconds_sum{method="GET",path="/metrics",status="200"} 0.04501697799999999
http_request_duration_seconds_count{method="GET",path="/metrics",status="200"} 1
# EOF

you can see the result has some unexpected new-lines with Exemplars. Therefore, Prometheus said "INVALID" " " is not a valid start token

Do I miss anything? please help me, I am very waiting for this feature

SimenB commented 1 year ago

I haven't gotten around to using this myself yet, so I'm not sure.

/cc @karlodwyer @voltbit

vothanhbinhlt commented 1 year ago

Hello, I love to see the name @voltbit here. I watched the seminar of Grafana where they use his fork repo of this repo in it. I downloaded the lab of the seminar and try to follow the same which @voltbit did. Unfortunately, It has not worked on my project with nestjs and typescript

voltbit commented 1 year ago

I ran the example/exemplars.js code with master and with the commit that introduced the exemplars 9e49ee6378e8e1632321ef401db827df24291c2b and the code on the commit outputs the exemplars correctly.

As far as I can tell the regression was introduced with a19a1ad653a5c4f286ce21cc25e198dcebdd03df.

The test for exemplars should have caught that tho 😬.

SimenB commented 1 year ago

Huh, sounds like something tests should have caught

EDIT: heh, like your edit says

On 12 Mar 2023, 14:07 +0100, Andrei Dobre @.***>, wrote:

I ran the example/exemplars.js code with master and with the commit that introduced the exemplars 9e49ee6378e8e1632321ef401db827df24291c2b and the code on the commit outputs the exemplars correctly. As far as I can tell the regression was introduced with a19a1ad653a5c4f286ce21cc25e198dcebdd03df. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

ngavalas commented 1 year ago

The exemplar is supposed to be on the same line as the metric, right? Looks like a pretty easy fix at least, just concat the strings before pushing into values.

voltbit commented 1 year ago

L.E. I added a fix and a test in the PR above. cc @ngavalas

For the moment I have just opened a PR with a proper snapshot test for the exemplars, it successfully fails on master and passes on the openmetrics commit (just for reference, the test should be added on the PR that brings the fix, I'll work on it tomorrow evening if nobody picks it up).

vothanhbinhlt commented 1 year ago

Hello guys, I tried the patch "prom-client": "github:voltbit/prom-client#improve-exemplars-tests",, and It worked perfectly. I think you guys should merged to the main branch

SimenB commented 1 year ago

https://github.com/siimon/prom-client/releases/tag/v15.0.0-1