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
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
6 changes: 4 additions & 2 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 @@ -91,7 +92,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 +142,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
66 changes: 33 additions & 33 deletions src/OpenTelemetry/Metrics/Tags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,39 @@ public Tags(KeyValuePair<string, object?>[] keyValuePairs)

public static bool operator !=(Tags tag1, Tags tag2) => !tag1.Equals(tag2);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int ComputeHashCode(KeyValuePair<string, object?>[] keyValuePairs)
dulikvor marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(keyValuePairs != null, "keyValuePairs was null");

#if NET
HashCode hashCode = default;

for (int i = 0; i < keyValuePairs.Length; i++)
{
ref var item = ref keyValuePairs[i];
hashCode.Add(item.Key.GetHashCode());
hashCode.Add(item.Value);
}

return hashCode.ToHashCode();
#else
var hash = 17;

for (int i = 0; i < keyValuePairs!.Length; i++)
{
ref var item = ref keyValuePairs[i];
unchecked
{
hash = (hash * 31) + item.Key.GetHashCode();
hash = (hash * 31) + (item.Value?.GetHashCode() ?? 0);
}
}

return hash;
#endif
}

public override readonly bool Equals(object? obj)
{
return obj is Tags other && this.Equals(other);
Expand Down Expand Up @@ -98,37 +131,4 @@ public readonly bool Equals(Tags other)
}

public override readonly int GetHashCode() => this.hashCode;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static int ComputeHashCode(KeyValuePair<string, object?>[] keyValuePairs)
{
Debug.Assert(keyValuePairs != null, "keyValuePairs was null");

#if NET
HashCode hashCode = default;

for (int i = 0; i < keyValuePairs.Length; i++)
{
ref var item = ref keyValuePairs[i];
hashCode.Add(item.Key.GetHashCode());
hashCode.Add(item.Value);
}

return hashCode.ToHashCode();
#else
var hash = 17;

for (int i = 0; i < keyValuePairs!.Length; i++)
{
ref var item = ref keyValuePairs[i];
unchecked
{
hash = (hash * 31) + item.Key.GetHashCode();
hash = (hash * 31) + (item.Value?.GetHashCode() ?? 0);
}
}

return hash;
#endif
}
}
37 changes: 22 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,20 +223,13 @@ 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(2, exportedItems.Count);

var meterTagHashed = Tags.ComputeHashCode(meterTags1.ToArray());
var metric = exportedItems.First(m => Tags.ComputeHashCode(m.MeterTags?.ToArray() ?? Array.Empty<KeyValuePair<string, object?>>()) == meterTagHashed);
Assert.Equal(meterName, metric.MeterName);
Assert.Equal(meterVersion, metric.MeterVersion);

Assert.NotNull(metric.MeterTags);

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);

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

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

meterTagHashed = Tags.ComputeHashCode(meterTags2.ToArray());
metric = exportedItems.First(m => Tags.ComputeHashCode(m.MeterTags?.ToArray() ?? Array.Empty<KeyValuePair<string, object?>>()) == meterTagHashed);
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