diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs index ea014ec5b29..0a58c103544 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs @@ -169,6 +169,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } sum.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + sum.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Sum = sum; @@ -206,6 +211,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } sum.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + sum.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Sum = sum; @@ -237,6 +247,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } gauge.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + gauge.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Gauge = gauge; @@ -268,6 +283,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } gauge.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + gauge.DataPoints.Add(CreateNoRecordedValueNumberDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Gauge = gauge; @@ -318,6 +338,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } histogram.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + histogram.DataPoints.Add(CreateNoRecordedValueHistogramDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.Histogram = histogram; @@ -370,6 +395,11 @@ internal static OtlpMetrics.Metric ToOtlpMetric(this Metric metric) } histogram.DataPoints.Add(dataPoint); + + if (metric.NoRecordedValueNeeded) + { + histogram.DataPoints.Add(CreateNoRecordedValueExponentialHistogramDataPoint(dataPoint.TimeUnixNano, metricPoint.Tags)); + } } otlpMetric.ExponentialHistogram = histogram; @@ -424,6 +454,45 @@ internal static OtlpMetrics.Exemplar ToOtlpExemplar(T value, in Metrics.Exemp return otlpExemplar; } + private static NumberDataPoint CreateNoRecordedValueNumberDataPoint(ulong timestamp, ReadOnlyTagCollection tags) + { + var lastDataPoint = new NumberDataPoint + { + StartTimeUnixNano = timestamp, + TimeUnixNano = timestamp, + Flags = (uint)DataPointFlags.NoRecordedValueMask, + }; + + AddAttributes(tags, lastDataPoint.Attributes); + return lastDataPoint; + } + + private static HistogramDataPoint CreateNoRecordedValueHistogramDataPoint(ulong timestamp, ReadOnlyTagCollection tags) + { + var lastDataPoint = new HistogramDataPoint + { + StartTimeUnixNano = timestamp, + TimeUnixNano = timestamp, + Flags = (uint)DataPointFlags.NoRecordedValueMask, + }; + + AddAttributes(tags, lastDataPoint.Attributes); + return lastDataPoint; + } + + private static ExponentialHistogramDataPoint CreateNoRecordedValueExponentialHistogramDataPoint(ulong timestamp, ReadOnlyTagCollection tags) + { + var lastDataPoint = new ExponentialHistogramDataPoint() + { + StartTimeUnixNano = timestamp, + TimeUnixNano = timestamp, + Flags = (uint)DataPointFlags.NoRecordedValueMask, + }; + + AddAttributes(tags, lastDataPoint.Attributes); + return lastDataPoint; + } + private static void AddAttributes(ReadOnlyTagCollection tags, RepeatedField attributes) { foreach (var tag in tags) diff --git a/src/OpenTelemetry/Metrics/Metric.cs b/src/OpenTelemetry/Metrics/Metric.cs index ac73955dda6..c4e6fe30fdd 100644 --- a/src/OpenTelemetry/Metrics/Metric.cs +++ b/src/OpenTelemetry/Metrics/Metric.cs @@ -243,6 +243,8 @@ internal Metric( internal bool Active { get; set; } = true; + internal bool NoRecordedValueNeeded { get; set; } + /// /// Get the metric points for the metric stream. /// diff --git a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs index 6074dd072f6..dc457839b29 100644 --- a/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs +++ b/src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs @@ -197,8 +197,6 @@ private void CreateOrUpdateMetricStreamRegistration(in MetricStreamIdentity metr { if (!this.metricStreamNames.Add(metricStreamIdentity.MetricStreamName)) { - // TODO: If a metric is deactivated and then reactivated we log the - // same warning as if it was a duplicate. OpenTelemetrySdkEventSource.Log.DuplicateMetricInstrument( metricStreamIdentity.InstrumentName, metricStreamIdentity.MeterName, @@ -231,7 +229,33 @@ private Batch GetMetricsBatch() if (!metric.Active) { - this.RemoveMetric(ref metric); + // Inactive metrics are sent one last time so the remaining data points and + // NoRecordedValue data points can be sent. The Active property might be + // set to false between collection cycles, so a separate property must be + // used to avoid duplicate staleness markers. + metric.NoRecordedValueNeeded = true; + + lock (this.instrumentCreationLock) + { + OpenTelemetrySdkEventSource.Log.MetricInstrumentRemoved(metric.Name, metric.MeterName); + + // Note: This is using TryUpdate and NOT TryRemove because there is a + // race condition. If a metric is deactivated and then reactivated in + // the same collection cycle + // instrumentIdentityToMetric[metric.InstrumentIdentity] may already + // point to the new activated metric and not the old deactivated one. + this.instrumentIdentityToMetric.TryUpdate(metric.InstrumentIdentity, null, metric); + + this.metricStreamNames.Remove(metric.InstrumentIdentity.MetricStreamName); + + // Defragment metrics list so storage can be reused on future metrics. + for (int j = i + 1; j < target; j++) + { + this.metrics[j - 1] = this.metrics[j]; + } + + this.metricIndex--; + } } } } @@ -244,29 +268,4 @@ private Batch GetMetricsBatch() return default; } } - - private void RemoveMetric(ref Metric? metric) - { - Debug.Assert(metric != null, "metric was null"); - - // TODO: This logic removes the metric. If the same - // metric is published again we will create a new metric - // for it. If this happens often we will run out of - // storage. Instead, should we keep the metric around - // and set a new start time + reset its data if it comes - // back? - - OpenTelemetrySdkEventSource.Log.MetricInstrumentRemoved(metric!.Name, metric.MeterName); - - // Note: This is using TryUpdate and NOT TryRemove because there is a - // race condition. If a metric is deactivated and then reactivated in - // the same collection cycle - // instrumentIdentityToMetric[metric.InstrumentIdentity] may already - // point to the new activated metric and not the old deactivated one. - this.instrumentIdentityToMetric.TryUpdate(metric.InstrumentIdentity, null, metric); - - // Note: metric is a reference to the array storage so - // this clears the metric out of the array. - metric = null; - } }