Skip to content

Commit

Permalink
[bugFix] Meter tags are now recognized as identified when reviewing m…
Browse files Browse the repository at this point in the history
…eter identity uniqueness.
  • Loading branch information
dulikvor committed Nov 16, 2024
1 parent 2ae01a7 commit b5d1011
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 47 deletions.
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)
{
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
}
}
34 changes: 23 additions & 11 deletions test/OpenTelemetry.Tests/Metrics/MetricApiTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Diagnostics;
using System.Diagnostics.Metrics;
using System.Threading.Tasks;
using OpenTelemetry.Exporter;
using OpenTelemetry.Internal;
using OpenTelemetry.Tests;
Expand Down Expand Up @@ -192,11 +193,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 @@ -228,16 +228,13 @@ public void MetricInstrumentationScopeAttributesAreNotTreatedAsIdentifyingProper
// 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 +243,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

0 comments on commit b5d1011

Please sign in to comment.