Skip to content

Commit

Permalink
Handle meter disposal cleanly
Browse files Browse the repository at this point in the history
  • Loading branch information
@stonkie
stonkie committed Nov 21, 2024
1 parent e3665c9 commit 2823545
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 28 deletions.
69 changes: 69 additions & 0 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/MetricItemExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -424,6 +454,45 @@ internal static OtlpMetrics.Exemplar ToOtlpExemplar<T>(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<OtlpCommon.KeyValue> attributes)
{
foreach (var tag in tags)
Expand Down
2 changes: 2 additions & 0 deletions src/OpenTelemetry/Metrics/Metric.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ internal Metric(

internal bool Active { get; set; } = true;

internal bool NoRecordedValueNeeded { get; set; }

/// <summary>
/// Get the metric points for the metric stream.
/// </summary>
Expand Down
55 changes: 27 additions & 28 deletions src/OpenTelemetry/Metrics/Reader/MetricReaderExt.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -231,7 +229,33 @@ private Batch<Metric> 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--;
}
}
}
}
Expand All @@ -244,29 +268,4 @@ private Batch<Metric> 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;
}
}

0 comments on commit 2823545

Please sign in to comment.