From e6efd4d23d42074b2fb13871801aedf1c396bf94 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 17 Feb 2023 20:33:42 +0530 Subject: [PATCH] [To metric branch] Exemplar - add filtered tags (#4202) --- .../ConsoleMetricExporter.cs | 15 +++++++ .../Implementation/MetricItemExtensions.cs | 17 ++++++- src/OpenTelemetry/Metrics/AggregatorStore.cs | 8 ++-- ...AlignedHistogramBucketExemplarReservoir.cs | 44 +++++++++++++++++-- .../Metrics/Exemplar/Exemplar.cs | 5 +++ src/OpenTelemetry/Metrics/MetricPoint.cs | 24 +++++----- src/OpenTelemetry/ReadOnlyTagCollection.cs | 8 ++-- 7 files changed, 95 insertions(+), 26 deletions(-) diff --git a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs index 6388dc2dbb6..f66a34ba756 100644 --- a/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs +++ b/src/OpenTelemetry.Exporter.Console/ConsoleMetricExporter.cs @@ -180,6 +180,21 @@ public override ExportResult Export(in Batch batch) exemplarString.Append(exemplar.TraceId); exemplarString.Append(" SpanId: "); exemplarString.Append(exemplar.SpanId); + + if (exemplar.FilteredTags != null && exemplar.FilteredTags.Count > 0) + { + exemplarString.Append(" Filtered Tags : "); + + foreach (var tag in exemplar.FilteredTags) + { + if (ConsoleTagTransformer.Instance.TryTransformTag(tag, out var result)) + { + exemplarString.Append(result); + exemplarString.Append(' '); + } + } + } + exemplarString.AppendLine(); } } diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index 84c6c81de41..75296c96332 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -281,13 +281,26 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) byte[] spanIdBytes = new byte[8]; examplar.SpanId?.CopyTo(spanIdBytes); - dataPoint.Exemplars.Add(new OtlpMetrics.Exemplar() + var otlpExemplar = new OtlpMetrics.Exemplar { TimeUnixNano = (ulong)examplar.Timestamp.ToUnixTimeNanoseconds(), TraceId = UnsafeByteOperations.UnsafeWrap(traceIdBytes), SpanId = UnsafeByteOperations.UnsafeWrap(spanIdBytes), AsDouble = examplar.DoubleValue, - }); + }; + + if (examplar.FilteredTags != null) + { + foreach (var tag in examplar.FilteredTags) + { + if (OtlpKeyValueTransformer.Instance.TryTransformTag(tag, out var result)) + { + otlpExemplar.FilteredAttributes.Add(result); + } + } + } + + dataPoint.Exemplars.Add(otlpExemplar); } } diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index e90093522c7..558691fa6c8 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -323,7 +323,7 @@ private void UpdateLong(long value, ReadOnlySpan> t // TODO: can special case built-in filters to be bit faster. if (this.exemplarFilter.ShouldSample(value, tags)) { - this.metricPoints[index].UpdateWithExemplar(value); + this.metricPoints[index].UpdateWithExemplar(value, tags: default); } else { @@ -354,7 +354,7 @@ private void UpdateLongCustomTags(long value, ReadOnlySpan> tags) { - var exemplar = default(Exemplar); + ref var exemplar = ref this.runningExemplars[index]; exemplar.Timestamp = DateTime.UtcNow; exemplar.DoubleValue = value; exemplar.TraceId = Activity.Current?.TraceId; exemplar.SpanId = Activity.Current?.SpanId; - this.runningExemplars[index] = exemplar; + + if (tags == default) + { + // default tag is used to indicate + // the special case where all tags provided at measurement + // recording time are stored. + // In this case, Exemplars does not have to store any tags. + // In other words, FilteredTags will be empty. + return; + } + + if (exemplar.FilteredTags == null) + { + exemplar.FilteredTags = new List>(tags.Length); + } + else + { + // Keep the list, but clear contents. + exemplar.FilteredTags.Clear(); + } + + // Though only those tags that are filtered need to be + // stored, finding filtered list from the full tag list + // is expensive. So all the tags are stored in hot path (this). + // During snapshot, the filtered list is calculated. + // TODO: Evaluate alternative approaches based on perf. + foreach (var tag in tags) + { + exemplar.FilteredTags.Add(tag); + } } public Exemplar[] Collect() @@ -47,11 +76,18 @@ public Exemplar[] Collect() return this.snapshotExemplars; } - public void SnapShot(bool reset) + public void SnapShot(ReadOnlyTagCollection actualTags, bool reset) { for (int i = 0; i < this.runningExemplars.Length; i++) { this.snapshotExemplars[i] = this.runningExemplars[i]; + if (this.runningExemplars[i].FilteredTags != null) + { + // TODO: Better data structure to avoid this Linq. + // This is doing filtered = alltags - storedtags. + this.snapshotExemplars[i].FilteredTags = this.runningExemplars[i].FilteredTags.Except(actualTags.KeyAndValues.ToList()).ToList(); + } + if (reset) { this.runningExemplars[i].Timestamp = default; diff --git a/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs b/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs index a8d82bce651..aa9fb938e48 100644 --- a/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs +++ b/src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs @@ -45,5 +45,10 @@ public struct Exemplar /// Gets the double value. /// public double DoubleValue { get; internal set; } + + /// + /// Gets the FilteredTags (i.e any tags that were dropped during aggregation). + /// + public List> FilteredTags { get; internal set; } } } diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index 77092e14455..89e60abedc2 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -348,7 +348,7 @@ internal void Update(long number) this.MetricPointStatus = MetricPointStatus.CollectPending; } - internal void UpdateWithExemplar(long number) + internal void UpdateWithExemplar(long number, ReadOnlySpan> tags) { switch (this.aggType) { @@ -384,13 +384,13 @@ internal void UpdateWithExemplar(long number) case AggregationType.HistogramWithBuckets: { - this.UpdateHistogramWithBuckets((double)number, true); + this.UpdateHistogramWithBuckets((double)number, tags, true); break; } case AggregationType.HistogramWithMinMaxBuckets: { - this.UpdateHistogramWithBucketsAndMinMax((double)number, true); + this.UpdateHistogramWithBucketsAndMinMax((double)number, tags, true); break; } } @@ -488,7 +488,7 @@ internal void Update(double number) this.MetricPointStatus = MetricPointStatus.CollectPending; } - internal void UpdateWithExemplar(double number) + internal void UpdateWithExemplar(double number, ReadOnlySpan> tags) { switch (this.aggType) { @@ -542,13 +542,13 @@ internal void UpdateWithExemplar(double number) case AggregationType.HistogramWithBuckets: { - this.UpdateHistogramWithBuckets(number, true); + this.UpdateHistogramWithBuckets(number, tags, true); break; } case AggregationType.HistogramWithMinMaxBuckets: { - this.UpdateHistogramWithBucketsAndMinMax(number, true); + this.UpdateHistogramWithBucketsAndMinMax(number, tags, true); break; } } @@ -692,7 +692,7 @@ internal void TakeSnapshot(bool outputDelta) } } - this.histogramBuckets.ExemplarReservoir?.SnapShot(outputDelta); + this.histogramBuckets.ExemplarReservoir?.SnapShot(this.Tags, outputDelta); this.MetricPointStatus = MetricPointStatus.NoCollectPending; @@ -767,7 +767,7 @@ internal void TakeSnapshot(bool outputDelta) } } - this.histogramBuckets.ExemplarReservoir?.SnapShot(outputDelta); + this.histogramBuckets.ExemplarReservoir?.SnapShot(this.Tags, outputDelta); this.MetricPointStatus = MetricPointStatus.NoCollectPending; // Release lock @@ -865,7 +865,7 @@ private void UpdateHistogramWithMinMax(double number) } } - private void UpdateHistogramWithBuckets(double number, bool reportExemplar = false) + private void UpdateHistogramWithBuckets(double number, ReadOnlySpan> tags = default, bool reportExemplar = false) { int i = this.histogramBuckets.FindBucketIndex(number); @@ -882,7 +882,7 @@ private void UpdateHistogramWithBuckets(double number, bool reportExemplar = fal this.histogramBuckets.RunningBucketCounts[i]++; if (reportExemplar) { - this.histogramBuckets.ExemplarReservoir.OfferAtBoundary(i, number); + this.histogramBuckets.ExemplarReservoir.OfferAtBoundary(i, number, tags); } } @@ -895,7 +895,7 @@ private void UpdateHistogramWithBuckets(double number, bool reportExemplar = fal } } - private void UpdateHistogramWithBucketsAndMinMax(double number, bool reportExemplar = false) + private void UpdateHistogramWithBucketsAndMinMax(double number, ReadOnlySpan> tags = default, bool reportExemplar = false) { int i = this.histogramBuckets.FindBucketIndex(number); @@ -912,7 +912,7 @@ private void UpdateHistogramWithBucketsAndMinMax(double number, bool reportExemp this.histogramBuckets.RunningBucketCounts[i]++; if (reportExemplar) { - this.histogramBuckets.ExemplarReservoir.OfferAtBoundary(i, number); + this.histogramBuckets.ExemplarReservoir.OfferAtBoundary(i, number, tags); } this.histogramBuckets.RunningMin = Math.Min(this.histogramBuckets.RunningMin, number); diff --git a/src/OpenTelemetry/ReadOnlyTagCollection.cs b/src/OpenTelemetry/ReadOnlyTagCollection.cs index ab451626e1d..1ff8bbfd4c7 100644 --- a/src/OpenTelemetry/ReadOnlyTagCollection.cs +++ b/src/OpenTelemetry/ReadOnlyTagCollection.cs @@ -25,17 +25,17 @@ namespace OpenTelemetry // prevent accidental boxing. public readonly struct ReadOnlyTagCollection { - private readonly KeyValuePair[] keyAndValues; + internal readonly KeyValuePair[] KeyAndValues; internal ReadOnlyTagCollection(KeyValuePair[]? keyAndValues) { - this.keyAndValues = keyAndValues ?? Array.Empty>(); + this.KeyAndValues = keyAndValues ?? Array.Empty>(); } /// /// Gets the number of tags in the collection. /// - public int Count => this.keyAndValues.Length; + public int Count => this.KeyAndValues.Length; /// /// Returns an enumerator that iterates through the tags. @@ -78,7 +78,7 @@ public bool MoveNext() if (index < this.source.Count) { - this.Current = this.source.keyAndValues[index]; + this.Current = this.source.KeyAndValues[index]; this.index++; return true;