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

Histogram<decimal> does not report values #5615

Open
joegoldman2 opened this issue May 14, 2024 · 7 comments
Open

Histogram<decimal> does not report values #5615

joegoldman2 opened this issue May 14, 2024 · 7 comments
Labels
metrics pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Comments

@joegoldman2
Copy link
Contributor

Area

area:api

Package Version

Package Name Version
OpenTelemetry.Api 1.8.1
OpenTelemetry 1.8.1

Runtime Version

net8.0

Description

When using a Histogram<decimal>, values are not reported.

Based on the .NET documentation, Histogram<T> should work with decimal.

Steps to Reproduce

using System.Diagnostics.Metrics;
using OpenTelemetry;
using OpenTelemetry.Metrics;

using var meterProvider = Sdk.CreateMeterProviderBuilder()
    .AddMeter("TestMeter")
    .AddConsoleExporter()
    .Build();

var meter = new Meter("TestMeter");

var histogram = meter.CreateHistogram<decimal>("test-histogram");
histogram.Record(10);

This produces the following output:

Resource associated with Metric:
    telemetry.sdk.name: opentelemetry
    telemetry.sdk.language: dotnet
    telemetry.sdk.version: 1.8.1
    service.name: unknown_service:ConsoleApp1

But when using a Histogram<double> for example, the output is:

Resource associated with Metric:
    telemetry.sdk.name: opentelemetry
    telemetry.sdk.language: dotnet
    telemetry.sdk.version: 1.8.1
    service.name: unknown_service:ConsoleApp1

Metric Name: test-histogram, Meter: TestMeter
(2024-05-14T16:27:14.4076497Z, 2024-05-14T16:27:14.4159669Z] Histogram
Value: Sum: 10 Count: 1 Min: 10 Max: 10
(-Infinity,0]:0
(0,5]:0
(5,10]:1
(10,25]:0
(25,50]:0
(50,75]:0
(75,100]:0
(100,250]:0
(250,500]:0
(500,750]:0
(750,1000]:0
(1000,2500]:0
(2500,5000]:0
(5000,7500]:0
(7500,10000]:0
(10000,+Infinity]:0

Expected Result

Values should be reported when using Histogram<decimal>.

Actual Result

Values are not reported when using Histogram<decimal>.

Additional Context

No response

@joegoldman2 joegoldman2 added the bug Something isn't working label May 14, 2024
@github-actions github-actions bot added the pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package label May 14, 2024
@joegoldman2
Copy link
Contributor Author

It's working when using a MeterListener, so I don't think it's a issue with .NET:

var meter = new Meter("TestMeter");
var histogram = meter.CreateHistogram<decimal>("test-histogram");

using var listener = new MeterListener();
listener.EnableMeasurementEvents(histogram);
listener.SetMeasurementEventCallback<decimal>((instrument, measurement, tags, state) =>
{
    Console.WriteLine(measurement);
});

histogram.Record(10);

This produces the following output:

10

@joegoldman2
Copy link
Contributor Author

joegoldman2 commented May 14, 2024

I think the issue is here:

// Everything double
this.listener.SetMeasurementEventCallback<double>(MeasurementRecordedDouble);
this.listener.SetMeasurementEventCallback<float>(static (instrument, value, tags, state) => MeasurementRecordedDouble(instrument, value, tags, state));
// Everything long
this.listener.SetMeasurementEventCallback<long>(MeasurementRecordedLong);
this.listener.SetMeasurementEventCallback<int>(static (instrument, value, tags, state) => MeasurementRecordedLong(instrument, value, tags, state));
this.listener.SetMeasurementEventCallback<short>(static (instrument, value, tags, state) => MeasurementRecordedLong(instrument, value, tags, state));
this.listener.SetMeasurementEventCallback<byte>(static (instrument, value, tags, state) => MeasurementRecordedLong(instrument, value, tags, state));

where SetMeasurementEventCallback<decimal> is missing. And this issue is not specific to Histogram<T> but all instruments that use decimal.

The fix can be a bit complicated (or deep let's say) as MetricPoint itself doesn't support decimal 😞.

@reyang reyang added pkg:OpenTelemetry.Api pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package and removed pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package pkg:OpenTelemetry.Api labels May 14, 2024
@cijothomas cijothomas added pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package metrics and removed bug Something isn't working labels May 15, 2024
@cijothomas
Copy link
Member

It is intentional to only support what can be reasonably represented/transported in OTLP format, which do not support decimal, without precision loss.

@joegoldman2
Copy link
Contributor Author

I understand and yes I had to cast to double (especially in the OpenTelemetry exporter) which means a loss of precision. For financial or scientific applications it's quite common and recommended to use decimal. Is the recommended solution to keep decimal but use metrics that use double in this case?

@cijothomas
Copy link
Member

Is the recommended solution

I am sorry, I do not have a recommendation for this yet. I was just pointing out that it was intentional to keep the supported types limited (and remove the bug tag!)

If we are anyway going to lose precision in export side, is it feasible to do the decimal->double by the user when calling the API itself?

@joegoldman2
Copy link
Contributor Author

I think it could be something feasible and could be the solution for now. As OTLP doesn't support more precision for the moment, I let you decide if you want to close this issue and the associated PR. Thank you for your feedback!

@cijothomas
Copy link
Member

I'd close the PR, but keep this issue open, to see if this is a common ask. Fixing this would be likely involve working with OpenTelemetry spec/proto repo, but first step is to gauge how common is this ask.
Thanks for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metrics pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants