Make Steeltoe Metrics extensions play nice with Otel (#848)

* Adds checks and warnings when extension methods are used incorrectly with Opentelemetry * Fix Threaddump extension methods * Apply suggestions from code review Co-authored-by: Tim Hess <[email protected]> * Easy configuration for actuators & Otel. Fix thread dump extension for non-windows * Fix failing testS * CR Fixes Co-authored-by: Tim Hess <[email protected]>
SteeltoeOSS · May 25, 2022 · 44a87f2 · 44a87f2
1 parent bc62967
commit 44a87f2
Show file tree

Hide file tree

Showing 11 changed files with 251 additions and 73 deletions.
diff --git a/src/Management/src/EndpointBase/Metrics/MetricsEndpoint.cs b/src/Management/src/EndpointBase/Metrics/MetricsEndpoint.cs
@@ -101,9 +101,21 @@ protected internal MetricsResponse GetMetric(MetricsRequest request, List<Metric
 
         protected internal void GetMetricsCollection(out MetricsCollection<List<MetricSample>> metricSamples, out MetricsCollection<List<MetricTag>> availTags)
         {
-            var collectionResponse = (SteeltoeCollectionResponse)_exporter.CollectionManager.EnterCollect().Result;
-            metricSamples = collectionResponse.MetricSamples;
-            availTags = collectionResponse.AvailableTags;
+            var response = _exporter.CollectionManager.EnterCollect().Result;
+
+            if (response is SteeltoeCollectionResponse collectionResponse)
+            {
+                metricSamples = collectionResponse.MetricSamples;
+                availTags = collectionResponse.AvailableTags;
+                return;
+            }
+            else
+            {
+                _logger?.LogWarning("Please ensure OpenTelemetry is configured via Steeltoe extension methods.");
+            }
+
+            metricSamples = new MetricsCollection<List<MetricSample>>();
+            availTags = new MetricsCollection<List<MetricTag>>();
 
             // TODO: update the response header with actual updatetime
         }

diff --git a/src/Management/src/EndpointBase/Metrics/Prometheus/PrometheusScraperEndpoint.cs b/src/Management/src/EndpointBase/Metrics/Prometheus/PrometheusScraperEndpoint.cs
@@ -28,21 +28,28 @@ public override string Invoke()
             var result = string.Empty;
             try
             {
-                var collectionResponse = (PrometheusCollectionResponse)_exporter.CollectionManager.EnterCollect().Result;
-                try
+                var response = _exporter.CollectionManager.EnterCollect().Result;
+                if (response is PrometheusCollectionResponse collectionResponse)
                 {
-                    if (collectionResponse.View.Count > 0)
+                    try
                     {
-                        result = Encoding.UTF8.GetString(collectionResponse.View.Array, 0, collectionResponse.View.Count);
+                        if (collectionResponse.View.Count > 0)
+                        {
+                            result = Encoding.UTF8.GetString(collectionResponse.View.Array, 0, collectionResponse.View.Count);
+                        }
+                        else
+                        {
+                            throw new InvalidOperationException("Collection failure.");
+                        }
                     }
-                    else
+                    finally
                     {
-                        throw new InvalidOperationException("Collection failure.");
+                        _exporter.CollectionManager.ExitCollect();
                     }
                 }
-                finally
+                else
                 {
-                    _exporter.CollectionManager.ExitCollect();
+                    _logger?.LogWarning("Please ensure OpenTelemetry is configured via Steeltoe extension methods.");
                 }
             }
             catch (Exception ex)

diff --git a/src/Management/src/EndpointBase/Metrics/ServiceCollectionExtensions.cs b/src/Management/src/EndpointBase/Metrics/ServiceCollectionExtensions.cs
@@ -5,6 +5,8 @@
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+using OpenTelemetry;
 using OpenTelemetry.Metrics;
 using Steeltoe.Common.Diagnostics;
 using Steeltoe.Management;
@@ -15,6 +17,8 @@
 using Steeltoe.Management.OpenTelemetry.Metrics;
 using System;
 using System.Diagnostics;
+using System.Diagnostics.Metrics;
+using System.Linq;
 
 namespace Microsoft.Extensions.DependencyInjection
 {
@@ -54,8 +58,8 @@ public static IServiceCollection AddMetricsActuatorServices(this IServiceCollect
                 var exporterOptions = new PullmetricsExporterOptions() { ScrapeResponseCacheDurationMilliseconds = options.ScrapeResponseCacheDurationMilliseconds };
                 return new SteeltoeExporter(exporterOptions);
             }));
-
             services.AddOpenTelemetryMetricsForSteeltoe();
+
             return services;
         }
 
@@ -87,31 +91,69 @@ public static IServiceCollection AddPrometheusActuatorServices(this IServiceColl
                 var exporterOptions = new PullmetricsExporterOptions() { ScrapeResponseCacheDurationMilliseconds = options.ScrapeResponseCacheDurationMilliseconds };
                 return new SteeltoePrometheusExporter(exporterOptions);
             }));
+
             services.AddOpenTelemetryMetricsForSteeltoe();
 
             return services;
         }
 
-        public static IServiceCollection AddOpenTelemetryMetricsForSteeltoe(this IServiceCollection services, string name = null, string version = null)
+        /// <summary>
+        /// Helper method to configure opentelemetry metrics. Do not use in conjuction with Extension methods provided by OpenTelemetry.
+        /// </summary>
+        /// <param name="services">Reference to the service collection</param>
+        /// <param name="configure">The Action to configure OpenTelemetry</param>
+        /// <param name="name">Instrumentation Name </param>
+        /// <param name="version">Instrumentation Version</param>
+        /// <returns>A reference to the service collection </returns>
+        public static IServiceCollection AddOpenTelemetryMetricsForSteeltoe(this IServiceCollection services, Action<IServiceProvider, MeterProviderBuilder> configure = null, string name = null, string version = null)
+        {
+            if (services.Any(sd => sd.ServiceType == typeof(MeterProvider)))
+            {
+                if (!services.Any(sd => sd.ImplementationInstance?.ToString() == "{ ConfiguredSteeltoeMetrics = True }"))
+                {
+                    Console.WriteLine("Warning!! Make sure one of the extension methods that calls ConfigureSteeltoeMetrics is used to correctly configure metrics using OpenTelemetry for Steeltoe.");
+                }
+
+                return services; // Already Configured, get out of here
+            }
+
+            services.AddSingleton(new { ConfiguredSteeltoeMetrics = true });
+            return services.AddOpenTelemetryMetrics(builder => builder.ConfigureSteeltoeMetrics());
+        }
+
+        /// <summary>
+        /// Configures the <see cref="MeterProviderBuilder"></see> as an underlying Metrics processor and exporter for Steeltoe in actuators and exporters. />
+        /// </summary>
+        /// <param name="builder">MeterProviderBuilder </param>
+        /// <param name="configure"> Configuration callback</param>
+        /// <param name="name">Instrumentation Name</param>
+        /// <param name="version">Instrumentation Version</param>
+        /// <returns>Configured MeterProviderBuilder</returns>
+        public static MeterProviderBuilder ConfigureSteeltoeMetrics(this MeterProviderBuilder builder, Action<IServiceProvider, MeterProviderBuilder> configure = null, string name = null, string version = null)
         {
-            return services.AddOpenTelemetryMetrics(builder =>
+            if (configure != null)
             {
-                builder.Configure((provider, deferredBuilder) =>
+                builder.Configure(configure);
+            }
+
+            builder.Configure((provider, deferredBuilder) =>
+            {
+                var views = provider.GetService<IViewRegistry>();
+                var exporters = provider.GetServices(typeof(IMetricsExporter)) as System.Collections.Generic.IEnumerable<IMetricsExporter>;
+
+                deferredBuilder
+                    .AddMeter(name ?? OpenTelemetryMetrics.InstrumentationName, version ?? OpenTelemetryMetrics.InstrumentationVersion)
+                    .AddRegisteredViews(views)
+                    .AddExporters(exporters);
+
+                var wavefrontExporter = provider.GetService<WavefrontMetricsExporter>(); // Not an IMetricsExporter
+
+                if (wavefrontExporter != null)
                 {
-                    var views = provider.GetService<IViewRegistry>();
-                    var exporters = provider.GetServices(typeof(IMetricsExporter)) as System.Collections.Generic.IEnumerable<IMetricsExporter>;
-                    deferredBuilder
-                        .AddMeter(name ?? OpenTelemetryMetrics.InstrumentationName, version ?? OpenTelemetryMetrics.InstrumentationVersion)
-                        .AddRegisteredViews(views)
-                        .AddExporters(exporters);
-
-                    var wavefrontExporter = provider.GetService<WavefrontMetricsExporter>(); // Not an IMetricsExporter
-                    if (wavefrontExporter != null)
-                    {
-                        deferredBuilder.AddWavefrontExporter(wavefrontExporter);
-                    }
-                });
+                    deferredBuilder.AddWavefrontExporter(wavefrontExporter);
+                }
             });
+            return builder;
         }
     }
 }
diff --git a/src/Management/src/EndpointCore/ActuatorServiceCollectionExtensions.cs b/src/Management/src/EndpointCore/ActuatorServiceCollectionExtensions.cs
@@ -57,10 +57,8 @@ public static IServiceCollection AddAllActuators(this IServiceCollection service
             }
 
             services.AddHypermediaActuator(config);
-            if (Platform.IsWindows)
-            {
-                services.AddThreadDumpActuator(config, version);
-            }
+
+            services.AddThreadDumpActuator(config, version);
 
             services.AddHeapDumpActuator(config);
 

diff --git a/src/Management/src/OpenTelemetryBase/Exporters/Prometheus/PrometheusSerializer.cs b/src/Management/src/OpenTelemetryBase/Exporters/Prometheus/PrometheusSerializer.cs
@@ -13,7 +13,7 @@ namespace Steeltoe.Management.OpenTelemetry.Exporters.Prometheus
 {
     /// <summary>
     /// Basic PrometheusSerializer which has no OpenTelemetry dependency.
-    /// Copied from Opentelemetry.Net project
+    /// Copied from OpenTelemetry.Net project
     /// </summary>
     internal static partial class PrometheusSerializer
     {

diff --git a/src/Management/src/OpenTelemetryBase/Exporters/PullmetricsCollectionManager.cs b/src/Management/src/OpenTelemetryBase/Exporters/PullmetricsCollectionManager.cs
@@ -13,7 +13,7 @@ namespace Steeltoe.Management.OpenTelemetry.Exporters
 {
 #pragma warning disable SX1309 // Field names should begin with underscore
 
-    // Adapted from Opentelemetry.Net project
+    // Adapted from OpenTelemetry.Net project
     internal sealed partial class PullmetricsCollectionManager
     {
         private readonly IMetricsExporter exporter;
@@ -165,7 +165,7 @@ private bool ExecuteCollect()
             this.exporter.OnExport = this.onCollectRef;
             var result = this.exporter.Collect?.Invoke(Timeout.Infinite);
             this.exporter.OnExport = null;
-            return result.HasValue ? result.Value : false;
+            return result.GetValueOrDefault();
         }
 
         private ExportResult OnCollect(Batch<Metric> metrics)

diff --git a/src/Management/src/OpenTelemetryBase/Exporters/Wavefront/WavefrontExporterOptions.cs b/src/Management/src/OpenTelemetryBase/Exporters/Wavefront/WavefrontExporterOptions.cs
@@ -24,11 +24,11 @@ public WavefrontExporterOptions(IConfiguration config)
 
         public string ApiToken { get; set; }
 
-        public int Step { get; set; } = 30000; // milliseconds
+        public int Step { get; set; } = 30_000; // milliseconds
 
-        public int BatchSize { get; set; } = 10000;
+        public int BatchSize { get; set; } = 10_000;
 
-        public int MaxQueueSize { get; set; } = 1000;
+        public int MaxQueueSize { get; set; } = 500_000;
 
         public WavefrontApplicationOptions ApplicationOptions { get; }
 

diff --git a/src/Management/test/CloudFoundryCore.Test/CloudFoundryHostBuilderExtensionsTest.cs b/src/Management/test/CloudFoundryCore.Test/CloudFoundryHostBuilderExtensionsTest.cs
@@ -45,14 +45,7 @@ public void AddCloudFoundryActuators_IWebHostBuilder()
 
             Assert.Contains(managementOptions, t => t.GetType() == typeof(CloudFoundryManagementOptions));
 
-            if (Platform.IsWindows)
-            {
-                Assert.Single(host.Services.GetServices<ThreadDumpEndpoint_v2>());
-            }
-            else
-            {
-                Assert.Empty(host.Services.GetServices<ThreadDumpEndpoint_v2>());
-            }
+            Assert.Single(host.Services.GetServices<ThreadDumpEndpoint_v2>());
 
             Assert.NotNull(filters);
             Assert.Single(filters.OfType<CloudFoundryActuatorsStartupFilter>());
@@ -72,15 +65,7 @@ public void AddCloudFoundryActuators_IWebHostBuilder_Serilog()
             var filters = host.Services.GetServices<IStartupFilter>();
 
             Assert.Contains(managementOptions, t => t.GetType() == typeof(CloudFoundryManagementOptions));
-
-            if (Platform.IsWindows)
-            {
-                Assert.Single(host.Services.GetServices<ThreadDumpEndpoint_v2>());
-            }
-            else
-            {
-                Assert.Empty(host.Services.GetServices<ThreadDumpEndpoint_v2>());
-            }
+            Assert.Single(host.Services.GetServices<ThreadDumpEndpoint_v2>());
 
             Assert.Single(host.Services.GetServices<HeapDumpEndpoint>());
 
@@ -99,15 +84,7 @@ public void AddCloudFoundryActuators_IHostBuilder()
             var filter = host.Services.GetServices<IStartupFilter>().FirstOrDefault();
 
             Assert.Contains(managementOptions, t => t.GetType() == typeof(CloudFoundryManagementOptions));
-
-            if (Platform.IsWindows)
-            {
-                Assert.Single(host.Services.GetServices<ThreadDumpEndpoint>());
-            }
-            else
-            {
-                Assert.Empty(host.Services.GetServices<ThreadDumpEndpoint>());
-            }
+            Assert.Single(host.Services.GetServices<ThreadDumpEndpoint>());
 
             Assert.Single(host.Services.GetServices<HeapDumpEndpoint>());
 
@@ -164,15 +141,7 @@ public void AddCloudFoundryActuators_IHostBuilder_Serilog()
             var filters = host.Services.GetServices<IStartupFilter>();
 
             Assert.Contains(managementOptions, t => t.GetType() == typeof(CloudFoundryManagementOptions));
-
-            if (Platform.IsWindows)
-            {
-                Assert.Single(host.Services.GetServices<ThreadDumpEndpoint_v2>());
-            }
-            else
-            {
-                Assert.Empty(host.Services.GetServices<ThreadDumpEndpoint_v2>());
-            }
+            Assert.Single(host.Services.GetServices<ThreadDumpEndpoint_v2>());
 
             Assert.Single(host.Services.GetServices<HeapDumpEndpoint>());
 

diff --git a/src/Management/test/EndpointCore.Test/ConsoleOutputBorrower.cs b/src/Management/test/EndpointCore.Test/ConsoleOutputBorrower.cs
@@ -0,0 +1,35 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the Apache 2.0 License.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Text;
+
+namespace Steeltoe.Management.Endpoint.Test
+{
+    internal class ConsoleOutputBorrower : IDisposable
+    {
+        private readonly StringWriter _borrowedOutput;
+        private readonly TextWriter _originalOutput;
+
+        public ConsoleOutputBorrower()
+        {
+            _borrowedOutput = new StringWriter();
+            _originalOutput = Console.Out;
+            Console.SetOut(_borrowedOutput);
+        }
+
+        public override string ToString()
+        {
+            return _borrowedOutput.ToString();
+        }
+
+        public void Dispose()
+        {
+            Console.SetOut(_originalOutput);
+            _borrowedOutput.Dispose();
+        }
+    }
+}