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

[sdk-metrics] Include Meter.Tags in metric identity resolution #5982

Merged
6 changes: 6 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ Notes](../../RELEASENOTES.md).

## Unreleased

* [Meter.Tags](https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.metrics.meter.tags?view=net-9.0)
will now be considered when resolving the SDK metric to update when
measurements are recorded. Meters with the same name and different tags will
now lead to unique metrics.
([#5982](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5982))

## 1.11.0-rc.1

Released 2024-Dec-11
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Metrics/Metric.cs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ internal Metric(
/// <summary>
/// Gets the attributes (tags) for the metric stream.
/// </summary>
public IEnumerable<KeyValuePair<string, object?>>? MeterTags => this.InstrumentIdentity.MeterTags;
public IEnumerable<KeyValuePair<string, object?>>? MeterTags => this.InstrumentIdentity.MeterTags?.KeyValuePairs;

/// <summary>
/// Gets the <see cref="MetricStreamIdentity"/> for the metric stream.
Expand Down
9 changes: 5 additions & 4 deletions src/OpenTelemetry/Metrics/MetricStreamIdentity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration? me
{
this.MeterName = instrument.Meter.Name;
this.MeterVersion = instrument.Meter.Version ?? string.Empty;
this.MeterTags = instrument.Meter.Tags;
this.MeterTags = instrument.Meter.Tags != null ? new Tags(instrument.Meter.Tags.ToArray()) : null;
this.InstrumentName = metricStreamConfiguration?.Name ?? instrument.Name;
this.Unit = instrument.Unit ?? string.Empty;
this.Description = metricStreamConfiguration?.Description ?? instrument.Description ?? string.Empty;
Expand All @@ -32,6 +32,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration? me
hashCode.Add(this.InstrumentType);
hashCode.Add(this.MeterName);
hashCode.Add(this.MeterVersion);
hashCode.Add(this.MeterTags);
hashCode.Add(this.InstrumentName);
hashCode.Add(this.HistogramRecordMinMax);
hashCode.Add(this.Unit);
Expand Down Expand Up @@ -63,8 +64,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration? me
hash = (hash * 31) + this.InstrumentType.GetHashCode();
hash = (hash * 31) + this.MeterName.GetHashCode();
hash = (hash * 31) + this.MeterVersion.GetHashCode();

// MeterTags is not part of identity, so not included here.
hash = (hash * 31) + this.MeterTags?.GetHashCode() ?? 0;
hash = (hash * 31) + this.InstrumentName.GetHashCode();
hash = (hash * 31) + this.HistogramRecordMinMax.GetHashCode();
hash = (hash * 31) + this.ExponentialHistogramMaxSize.GetHashCode();
Expand All @@ -91,7 +91,7 @@ public MetricStreamIdentity(Instrument instrument, MetricStreamConfiguration? me

public string MeterVersion { get; }

public IEnumerable<KeyValuePair<string, object?>>? MeterTags { get; }
public Tags? MeterTags { get; }

public string InstrumentName { get; }

Expand Down Expand Up @@ -141,6 +141,7 @@ public bool Equals(MetricStreamIdentity other)
&& this.Unit == other.Unit
&& this.Description == other.Description
&& this.ViewId == other.ViewId
&& this.MeterTags == other.MeterTags
&& this.HistogramRecordMinMax == other.HistogramRecordMinMax
&& this.ExponentialHistogramMaxSize == other.ExponentialHistogramMaxSize
&& this.ExponentialHistogramMaxScale == other.ExponentialHistogramMaxScale
Expand Down
40 changes: 25 additions & 15 deletions test/OpenTelemetry.Tests/Metrics/MetricApiTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,10 @@ public void MetricInstrumentationScopeIsExportedCorrectly()
}

[Fact]
public void MetricInstrumentationScopeAttributesAreNotTreatedAsIdentifyingProperty()
public void MetricInstrumentationScopeAttributesAreTreatedAsIdentifyingProperty()
{
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#get-a-meter
// Meters are identified by name, version, and schema_url fields
// and not with tags.
// Meters are identified by name, version, meter tags and schema_url fields.
var exportedItems = new List<Metric>();
var meterName = "MyMeter";
var meterVersion = "1.0";
Expand Down Expand Up @@ -224,19 +223,16 @@ public void MetricInstrumentationScopeAttributesAreNotTreatedAsIdentifyingProper
counter2.Add(15);
meterProvider.ForceFlush(MaxTimeToAllowForFlush);

// The instruments differ only in the Meter.Tags, which is not an identifying property.
// The first instrument's Meter.Tags is exported.
// It is considered a user-error to create Meters with same name,version but with
// different tags. TODO: See if we can emit an internal log about this.
Assert.Single(exportedItems);
var metric = exportedItems[0];
Assert.Equal(meterName, metric.MeterName);
Assert.Equal(meterVersion, metric.MeterVersion);
Assert.Equal(2, exportedItems.Count);

Assert.NotNull(metric.MeterTags);
bool TagComparator(KeyValuePair<string, object?> lhs, KeyValuePair<string, object?> rhs)
{
return lhs.Key.Equals(rhs.Key) && lhs.Value!.GetHashCode().Equals(rhs.Value!.GetHashCode());
}

Assert.Single(metric.MeterTags.Where(kvp => kvp.Key == meterTags1[0].Key && kvp.Value == meterTags1[0].Value));
Assert.DoesNotContain(metric.MeterTags, kvp => kvp.Key == meterTags2[0].Key && kvp.Value == meterTags2[0].Value);
var metric = exportedItems.First(m => TagComparator(m.MeterTags!.First(), meterTags1!.First()));
Assert.Equal(meterName, metric.MeterName);
Assert.Equal(meterVersion, metric.MeterVersion);

List<MetricPoint> metricPoints = new List<MetricPoint>();
foreach (ref readonly var mp in metric.GetMetricPoints())
Expand All @@ -246,7 +242,21 @@ public void MetricInstrumentationScopeAttributesAreNotTreatedAsIdentifyingProper

Assert.Single(metricPoints);
var metricPoint1 = metricPoints[0];
Assert.Equal(25, metricPoint1.GetSumLong());
Assert.Equal(10, metricPoint1.GetSumLong());

metric = exportedItems.First(m => TagComparator(m.MeterTags!.First(), meterTags2!.First()));
Assert.Equal(meterName, metric.MeterName);
Assert.Equal(meterVersion, metric.MeterVersion);

metricPoints = new List<MetricPoint>();
foreach (ref readonly var mp in metric.GetMetricPoints())
{
metricPoints.Add(mp);
}

Assert.Single(metricPoints);
metricPoint1 = metricPoints[0];
Assert.Equal(15, metricPoint1.GetSumLong());
}

[Fact]
Expand Down
Loading