From 15981dfd7f177579b137b9f2ab96b7761743d6b5 Mon Sep 17 00:00:00 2001 From: Reiley Yang Date: Wed, 22 Sep 2021 10:14:58 -0700 Subject: [PATCH] Improve Metrics pull export mode (#2389) --- docs/metrics/extending-the-sdk/MyExporter.cs | 1 - .../MeterProviderBuilderExtensions.cs | 6 +- .../PrometheusExporter.cs | 9 +- .../PrometheusExporterMetricsHttpServer.cs | 2 +- .../Metrics/BaseExportingMetricReader.cs | 40 +++++++ .../Metrics/IPullMetricExporter.cs | 28 +++++ src/OpenTelemetry/Metrics/PullMetricScope.cs | 52 +++++++++ .../Metrics/MetricExporterTests.cs | 105 ++++++++++++++++++ 8 files changed, 235 insertions(+), 8 deletions(-) create mode 100644 src/OpenTelemetry/Metrics/IPullMetricExporter.cs create mode 100644 src/OpenTelemetry/Metrics/PullMetricScope.cs create mode 100644 test/OpenTelemetry.Tests/Metrics/MetricExporterTests.cs diff --git a/docs/metrics/extending-the-sdk/MyExporter.cs b/docs/metrics/extending-the-sdk/MyExporter.cs index bb3bc53dc1b..bb8fd545595 100644 --- a/docs/metrics/extending-the-sdk/MyExporter.cs +++ b/docs/metrics/extending-the-sdk/MyExporter.cs @@ -43,7 +43,6 @@ public override ExportResult Export(in Batch batch) } sb.Append($"{record}"); - sb.Append(')'); } Console.WriteLine($"{this.name}.Export([{sb.ToString()}])"); diff --git a/src/OpenTelemetry.Exporter.Prometheus/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Exporter.Prometheus/MeterProviderBuilderExtensions.cs index b30f7b3c97a..443d0968768 100644 --- a/src/OpenTelemetry.Exporter.Prometheus/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Exporter.Prometheus/MeterProviderBuilderExtensions.cs @@ -39,13 +39,11 @@ public static MeterProviderBuilder AddPrometheusExporter(this MeterProviderBuild configure?.Invoke(options); var exporter = new PrometheusExporter(options); - - var metricReader = new BaseExportingMetricReader(exporter); - exporter.CollectMetric = metricReader.Collect; + var reader = new BaseExportingMetricReader(exporter); var metricsHttpServer = new PrometheusExporterMetricsHttpServer(exporter); metricsHttpServer.Start(); - return builder.AddMetricReader(metricReader); + return builder.AddMetricReader(reader); } } } diff --git a/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporter.cs b/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporter.cs index 672d9b675ac..4a21d78bc67 100644 --- a/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporter.cs +++ b/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporter.cs @@ -25,10 +25,11 @@ namespace OpenTelemetry.Exporter /// [AggregationTemporality(AggregationTemporality.Cumulative)] [ExportModes(ExportModes.Pull)] - public class PrometheusExporter : BaseExporter + public class PrometheusExporter : BaseExporter, IPullMetricExporter { internal readonly PrometheusExporterOptions Options; internal Batch Metrics; + private Func funcCollect; /// /// Initializes a new instance of the class. @@ -39,7 +40,11 @@ public PrometheusExporter(PrometheusExporterOptions options) this.Options = options; } - internal Func CollectMetric { get; set; } + public Func Collect + { + get => this.funcCollect; + set { this.funcCollect = value; } + } public override ExportResult Export(in Batch metrics) { diff --git a/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporterMetricsHttpServer.cs b/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporterMetricsHttpServer.cs index 3d24c703b3e..330764363ad 100644 --- a/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporterMetricsHttpServer.cs +++ b/src/OpenTelemetry.Exporter.Prometheus/PrometheusExporterMetricsHttpServer.cs @@ -124,7 +124,7 @@ private void WorkerThread() using var output = ctx.Response.OutputStream; using var writer = new StreamWriter(output); - this.exporter.CollectMetric(Timeout.Infinite); + this.exporter.Collect(Timeout.Infinite); this.exporter.WriteMetricsCollection(writer); } } diff --git a/src/OpenTelemetry/Metrics/BaseExportingMetricReader.cs b/src/OpenTelemetry/Metrics/BaseExportingMetricReader.cs index 31e6f61016c..06409b924a5 100644 --- a/src/OpenTelemetry/Metrics/BaseExportingMetricReader.cs +++ b/src/OpenTelemetry/Metrics/BaseExportingMetricReader.cs @@ -43,6 +43,24 @@ public BaseExportingMetricReader(BaseExporter exporter) var attr = (ExportModesAttribute)attributes[attributes.Length - 1]; this.supportedExportModes = attr.Supported; } + + if (exporter is IPullMetricExporter pullExporter) + { + if (this.supportedExportModes.HasFlag(ExportModes.Push)) + { + pullExporter.Collect = this.Collect; + } + else + { + pullExporter.Collect = (timeoutMilliseconds) => + { + using (PullMetricScope.Begin()) + { + return this.Collect(timeoutMilliseconds); + } + }; + } + } } protected ExportModes SupportedExportModes => this.supportedExportModes; @@ -59,6 +77,23 @@ protected override bool ProcessMetrics(Batch metrics, int timeoutMillise return this.exporter.Export(metrics) == ExportResult.Success; } + /// + protected override bool OnCollect(int timeoutMilliseconds) + { + if (this.supportedExportModes.HasFlag(ExportModes.Push)) + { + return base.OnCollect(timeoutMilliseconds); + } + + if (this.supportedExportModes.HasFlag(ExportModes.Pull) && PullMetricScope.IsPullAllowed) + { + return base.OnCollect(timeoutMilliseconds); + } + + // TODO: add some error log + return false; + } + /// protected override bool OnShutdown(int timeoutMilliseconds) { @@ -77,6 +112,11 @@ protected override void Dispose(bool disposing) { try { + if (this.exporter is IPullMetricExporter pullExporter) + { + pullExporter.Collect = null; + } + this.exporter.Dispose(); } catch (Exception) diff --git a/src/OpenTelemetry/Metrics/IPullMetricExporter.cs b/src/OpenTelemetry/Metrics/IPullMetricExporter.cs new file mode 100644 index 00000000000..aac55247fdf --- /dev/null +++ b/src/OpenTelemetry/Metrics/IPullMetricExporter.cs @@ -0,0 +1,28 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; + +namespace OpenTelemetry.Metrics +{ + /// + /// Describes a type of which supports . + /// + public interface IPullMetricExporter + { + Func Collect { get; set; } + } +} diff --git a/src/OpenTelemetry/Metrics/PullMetricScope.cs b/src/OpenTelemetry/Metrics/PullMetricScope.cs new file mode 100644 index 00000000000..dd00e844bad --- /dev/null +++ b/src/OpenTelemetry/Metrics/PullMetricScope.cs @@ -0,0 +1,52 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using OpenTelemetry.Context; + +namespace OpenTelemetry.Metrics +{ + internal sealed class PullMetricScope : IDisposable + { + private static readonly RuntimeContextSlot Slot = RuntimeContext.RegisterSlot("otel.pull_metric"); + + private readonly bool previousValue; + private bool disposed; + + internal PullMetricScope(bool value = true) + { + this.previousValue = Slot.Get(); + Slot.Set(value); + } + + internal static bool IsPullAllowed => Slot.Get(); + + public static IDisposable Begin(bool value = true) + { + return new PullMetricScope(value); + } + + /// + public void Dispose() + { + if (!this.disposed) + { + Slot.Set(this.previousValue); + this.disposed = true; + } + } + } +} diff --git a/test/OpenTelemetry.Tests/Metrics/MetricExporterTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricExporterTests.cs new file mode 100644 index 00000000000..8047b0acb73 --- /dev/null +++ b/test/OpenTelemetry.Tests/Metrics/MetricExporterTests.cs @@ -0,0 +1,105 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using System.Diagnostics.Metrics; +using Xunit; + +namespace OpenTelemetry.Metrics.Tests +{ + public class MetricExporterTests + { + [Theory] + [InlineData(ExportModes.Push)] + [InlineData(ExportModes.Pull)] + [InlineData(ExportModes.Pull | ExportModes.Push)] + public void FlushMetricExporterTest(ExportModes mode) + { + BaseExporter exporter = null; + + switch (mode) + { + case ExportModes.Push: + exporter = new PushOnlyMetricExporter(); + break; + case ExportModes.Pull: + exporter = new PullOnlyMetricExporter(); + break; + case ExportModes.Pull | ExportModes.Push: + exporter = new PushPullMetricExporter(); + break; + } + + var reader = new BaseExportingMetricReader(exporter); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMetricReader(reader) + .Build(); + + switch (mode) + { + case ExportModes.Push: + Assert.True(reader.Collect()); + Assert.True(meterProvider.ForceFlush()); + break; + case ExportModes.Pull: + Assert.False(reader.Collect()); + Assert.False(meterProvider.ForceFlush()); + Assert.True((exporter as IPullMetricExporter).Collect(-1)); + break; + case ExportModes.Pull | ExportModes.Push: + Assert.True(reader.Collect()); + Assert.True(meterProvider.ForceFlush()); + break; + } + } + + [ExportModes(ExportModes.Push)] + private class PushOnlyMetricExporter : BaseExporter + { + public override ExportResult Export(in Batch batch) + { + return ExportResult.Success; + } + } + + [ExportModes(ExportModes.Pull)] + private class PullOnlyMetricExporter : BaseExporter, IPullMetricExporter + { + private Func funcCollect; + + public Func Collect + { + get => this.funcCollect; + set { this.funcCollect = value; } + } + + public override ExportResult Export(in Batch batch) + { + return ExportResult.Success; + } + } + + [ExportModes(ExportModes.Pull | ExportModes.Push)] + private class PushPullMetricExporter : BaseExporter + { + public override ExportResult Export(in Batch batch) + { + return ExportResult.Success; + } + } + } +}