MetricPoint.UpdateWithExemplar method honors isSampled parameter only for DoubleSumIncomingDelta case

[ImoutoChan]

Bug Report

Latest master:

opentelemetry-dotnet/src/OpenTelemetry/Metrics/MetricPoint.cs

Line 444 in b870ed9

internal void UpdateWithExemplar(long number, ReadOnlySpan<KeyValuePair<string, object?>> tags, bool isSampled)

net7.0

Symptom

isSampled parameter is only checked in AggregationType.DoubleSumIncomingDelta case. That's why some exemplars are reported even when exemplar filter returns false.

What is the expected behavior?

isSampled should be checked before each this.mpComponents.ExemplarReservoir!.Offer call.

What is the actual behavior?

isSampled is only checked once in the first case.

Additional Context

It would be great to have the ability to overwrite ExemplarReservoir. In this case, I could easily fix this bug in my codebase without waiting for bug fixes. Unfortunately, everything is internal in this library. Please consider providing an extension point to change ExemplarReservoir.

Another bug (maybe not in this library) is that SimpleExemplarReservoir can report exemplars with null trace id and trace span:
SimpleExemplarReservoir.cs#L89)

This breaks open-collector's open metric output and Prometheus with empty exemplars. Again, because there is no extension point I can't fix it myself, so providing it would be a great help for the dotnet community.

[cijothomas]

It would be great to have the ability to overwrite ExemplarReservoir. In this case, I could easily fix this bug in my codebase without waiting for bug fixes. Unfortunately, everything is internal in this library. Please consider providing an extension point to change ExemplarReservoir.

Yes, this will eventually be offered. Reservoir and Filter are the 2 extensibility points that'll be available in the future.

[cijothomas]

Another bug (maybe not in this library) is that SimpleExemplarReservoir can report exemplars with null trace id and trace span:

TraceId and SpanId in exemplar are optional. So why is this a bug?

[cijothomas]

@ImoutoChan Would you be willing to send a PR to fix this?

[ImoutoChan]

Yes, I'll try