Skip to content

Commit

Permalink
[sdk-metrics] Include Meter.Tags in metric identity resolution (#5982)
Browse files Browse the repository at this point in the history
Co-authored-by: Cijo Thomas <cijo.thomas@gmail.com>
Co-authored-by: Piotr Kiełkowicz <pkiekowicz@splunk.com>
Co-authored-by: Mikel Blanchard <mblanchard@macrosssoftware.com>
  • Loading branch information
4 people authored Dec 26, 2024
1 parent 408a52d commit 7ae7b13
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 20 deletions.
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

0 comments on commit 7ae7b13

Please sign in to comment.