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

ObservableCounter should report absolute value #3073

Closed
pfdgithub opened this issue Jul 1, 2022 · 3 comments · Fixed by #3065
Closed

ObservableCounter should report absolute value #3073

pfdgithub opened this issue Jul 1, 2022 · 3 comments · Fixed by #3065
Assignees
Labels
bug Something isn't working priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect

Comments

@pfdgithub
Copy link

pfdgithub commented Jul 1, 2022

What happened?

Steps to Reproduce

In api-metrics@0.29.2, there are synchronous api Counter and asynchronous api ObservableCounter.
But ObservableCounter does not report absolute value, this is not conforming to specification.

Note: Unlike Counter.Add() which takes the increment/delta value, the callback function reports the absolute value of the counter. To determine the reported rate the counter is changing, the difference between successive measurements is used.

According to my colleague, in the Java implementation, ObservableCounter report absolute value.
If that's true, this will cause cross-language developers to misunderstand the API.

Expected Result

The same value is reported each time.
1 - 1 - 1 - 1 - 1 - ...

Actual Result

The value is incremented with each report.
1 - 2 - 3 - 4 - 5 -...

Additional Details

refer:
#1564

OpenTelemetry Setup Code

import { metrics } from "@opentelemetry/api-metrics";

let counterInstrument;
let observableCounter = 0;

const meter = metrics.getMeter("nodejs-metrics");

counterInstrument = meter.createCounter("nodejs-counter");

meter
  .createObservableCounter("nodejs-observable-counter")
  .addCallback((observable) => {
    observable.observe(observableCounter);
  });

counterInstrument.add(1);

observableCounter = 1;

package.json

@opentelemetry/api-metrics@0.29.2

Relevant log output

// ---------- first report ----------

[
  {
    scope: { name: 'nodejs-metrics', version: '', schemaUrl: undefined },
    metrics: [
      {
        descriptor: {
          name: 'nodejs-counter',
          description: '',
          type: 'COUNTER',
          unit: '',
          valueType: 1
        },
        aggregationTemporality: 1,
        dataPointType: 0,
        dataPoints: []
      },
      {
        descriptor: {
          name: 'nodejs-observable-counter',
          description: '',
          type: 'OBSERVABLE_COUNTER',
          unit: '',
          valueType: 1
        },
        aggregationTemporality: 1,
        dataPointType: 0,
        dataPoints: [
          {
            attributes: {},
            startTime: [ 1656687460, 406996931 ],
            endTime: [ 1656687520, 409942931 ],
            value: 0 // <---------- look here
          }
        ]
      }
    ]
  }
]

// ---------- second report ----------

[
  {
    scope: { name: 'nodejs-metrics', version: '', schemaUrl: undefined },
    metrics: [
      {
        descriptor: {
          name: 'nodejs-counter',
          description: '',
          type: 'COUNTER',
          unit: '',
          valueType: 1
        },
        aggregationTemporality: 1,
        dataPointType: 0,
        dataPoints: []
      },
      {
        descriptor: {
          name: 'nodejs-observable-counter',
          description: '',
          type: 'OBSERVABLE_COUNTER',
          unit: '',
          valueType: 1
        },
        aggregationTemporality: 1,
        dataPointType: 0,
        dataPoints: [
          {
            attributes: {},
            startTime: [ 1656687460, 406996931 ],
            endTime: [ 1656687580, 410089014 ],
            value: 1 // <---------- look here
          }
        ]
      }
    ]
  }
]

// ---------- thirdly report ----------

[
  {
    scope: { name: 'nodejs-metrics', version: '', schemaUrl: undefined },
    metrics: [
      {
        descriptor: {
          name: 'nodejs-counter',
          description: '',
          type: 'COUNTER',
          unit: '',
          valueType: 1
        },
        aggregationTemporality: 1,
        dataPointType: 0,
        dataPoints: []
      },
      {
        descriptor: {
          name: 'nodejs-observable-counter',
          description: '',
          type: 'OBSERVABLE_COUNTER',
          unit: '',
          valueType: 1
        },
        aggregationTemporality: 1,
        dataPointType: 0,
        dataPoints: [
          {
            attributes: {},
            startTime: [ 1656687460, 406996931 ],
            endTime: [ 1656687640, 415848556 ],
            value: 2 // <---------- look here
          }
        ]
      }
    ]
  }
]
@pfdgithub pfdgithub added bug Something isn't working triage labels Jul 1, 2022
@legendecas
Copy link
Member

legendecas commented Jul 1, 2022

Thank you for reporting. This is a problem related to DeltaMetricProcessor incorrectly saves the delta value as the cumulative value history. The problem should have been fixed in #2990.

@legendecas legendecas removed the triage label Jul 1, 2022
@dyladan dyladan added the priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect label Jul 6, 2022
@dyladan dyladan linked a pull request Jul 6, 2022 that will close this issue
@dyladan
Copy link
Member

dyladan commented Jul 6, 2022

Assigning @legendecas since he wrote the PR that fixes this.

@legendecas
Copy link
Member

The fix will be released in #3065

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants