Skip to content
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

@opentelemetry/metrics: The option "boundaries" for metrics is not propagated to the HistogramAggregator #1626

Closed
jhbabon opened this issue Oct 27, 2020 · 6 comments · Fixed by #1628
Assignees
Labels
bug Something isn't working

Comments

@jhbabon
Copy link

jhbabon commented Oct 27, 2020

Please answer these questions before submitting a bug report.

What version of OpenTelemetry are you using?

Version v0.12.

What version of Node are you using?

Node 14.10.1

Please provide the code you used to setup the OpenTelemetry SDK

package.json

{
  "name": "otel-bug-repro",
  "version": "1.0.0",
  "description": "OTel Metrics bug repro",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "UNLICENSED",
  "dependencies": {
    "@opentelemetry/core": "^0.12.0",
    "@opentelemetry/exporter-prometheus": "^0.12.0",
    "@opentelemetry/metrics": "^0.12.0",
    "node-fetch": "^2.6.1"
  }
}

index.js

'use strict';

const { MeterProvider } = require('@opentelemetry/metrics');
const { ConsoleLogger, LogLevel } = require('@opentelemetry/core');
const { PrometheusExporter } = require('@opentelemetry/exporter-prometheus');
const fetch = require('node-fetch');

function main() {
  const scrapeUrl = `http://localhost:${PrometheusExporter.DEFAULT_OPTIONS.port}${PrometheusExporter.DEFAULT_OPTIONS.endpoint}`
  const logger = new ConsoleLogger(LogLevel.DEBUG)
  const exporter = new PrometheusExporter(
    {
      preventServerStart: false,
      logger,
    },
    () => {
      console.log(`prometheus scrape endpoint: ${scrapeUrl}`);
    },
  );

  const meter = new MeterProvider({
    exporter,
    interval: 1000,
    logger,
  }).getMeter('example-observer');

  const tester = meter.createValueRecorder('tester', {
    description: 'test boundaries propagation',
    logger,
    boundaries: [0.01, 5, 10], // these are not propagated to Prometheus' Histogram
  });

  tester.record(8);

  setTimeout(async () => {
    const res = await fetch(scrapeUrl);
    const metrics = await res.text();

    console.log(metrics);

    process.exit(1);
  }, 1500);
}

main();

What did you do?

I'm trying to create a Promtetheus Histogram using a ValueRecorder metric, but when setting the boundaries option this same option is not translated to the buckets list that the Histogram should be using. Somehow the value is lost in the conversion.

What did you expect to see?

I would expect to see a list of *_bucket counters with the le tag with values 0.01, 5 and 10. Something similar to this (based on the example):

# HELP tester tester
# TYPE tester histogram
tester_count 1 1603784561929
tester_sum 8 1603784561929
tester_bucket{le="0.01"} 0 0
tester_bucket{le="5"} 0 0
tester_bucket{le="10"} 1 1603784561929
tester_bucket{le="+Inf"} 1 1603784561929

What did you see instead?

If a run the example you can see the buckets are not set up:

$ node index.js
node index.js
Prometheus exporter started on port 9464 at endpoint /metrics
prometheus scrape endpoint: http://localhost:9464/metrics
Prometheus exporter export
# HELP tester tester
# TYPE tester histogram
tester_count 1 1603784561929
tester_sum 8 1603784561929
tester_bucket{le="Infinity"} 1 1603784561929
tester_bucket{le="+Inf"} 1 1603784561929
@jhbabon jhbabon added the bug Something isn't working label Oct 27, 2020
@AndrewGrachov
Copy link
Contributor

will take it

@vmarchaud
Copy link
Member

@AndrewGrachov Feel free to reach out if you have any questions

@AndrewGrachov
Copy link
Contributor

AndrewGrachov commented Oct 27, 2020

OK, I see this is my "favorite" thing with otel :)

#1477

Guess we should remove histogram option from createValueRecorder metric options

@AndrewGrachov
Copy link
Contributor

@jhbabon pls see #1465

@AndrewGrachov
Copy link
Contributor

AndrewGrachov commented Oct 28, 2020

The PR above should fix the issue, but I'm not sure if its logically correct to specify histogram-aggregator specific parameters within Base Metric class (which could be possibly aggregated many ways)

@jhbabon
Copy link
Author

jhbabon commented Oct 28, 2020

The PR above should fix the issue, but I'm not sure if its logically correct to specify histogram-aggregator specific parameters within Base Metric class (which could be possibly aggregated many ways)

Many thanks!

pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
…packages (open-telemetry#1626)

Co-authored-by: Amir Blum <amirgiraffe@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants