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

[bugFix] Meter tags are now recognized as identifiers #5982

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Notes](../../RELEASENOTES.md).

## Unreleased

* Updated `OpenTelemetry.Metrics.MetricStreamIdentity` Metric tags are now considered identifiers,
contributing to metric scope uniqueness.
([#5982](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5982))

## 1.10.0

Released 2024-Nov-12
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