From 67b7c7a64cf8a426486ba26aac9e6c3f8246bb6f Mon Sep 17 00:00:00 2001 From: temppus Date: Sat, 26 Nov 2022 13:16:00 +0100 Subject: [PATCH] Add support for some AspNetCoreMetricsInstrumentationOptions --- .../.publicApi/net7.0/PublicAPI.Unshipped.txt | 3 + .../AspNetCoreMetrics.cs | 10 +- ...AspNetCoreMetricsInstrumentationOptions.cs | 45 +++++++ .../Implementation/HttpInMetricsListener.cs | 21 +++- .../MeterProviderBuilderExtensions.cs | 57 +++++++-- .../DependencyInjectionConfigTests.cs | 33 ++++- .../MetricTests.cs | 115 +++++++++++++++++- 7 files changed, 266 insertions(+), 18 deletions(-) create mode 100644 src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetricsInstrumentationOptions.cs diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net7.0/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net7.0/PublicAPI.Unshipped.txt index b3b24c45be9..4c066a56238 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net7.0/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/.publicApi/net7.0/PublicAPI.Unshipped.txt @@ -12,9 +12,12 @@ OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.Filter.set -> void OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.RecordException.get -> bool OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreInstrumentationOptions.RecordException.set -> void +OpenTelemetry.Instrumentation.AspNetCore.AspNetCoreMetricsInstrumentationOptions OpenTelemetry.Metrics.MeterProviderBuilderExtensions OpenTelemetry.Trace.TracerProviderBuilderExtensions static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder) -> OpenTelemetry.Metrics.MeterProviderBuilder +static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder, string name, System.Action configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Metrics.MeterProviderBuilder +static OpenTelemetry.Metrics.MeterProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder builder, System.Action configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Metrics.MeterProviderBuilder static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, string name, System.Action configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAspNetCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action configureAspNetCoreInstrumentationOptions) -> OpenTelemetry.Trace.TracerProviderBuilder diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs index fa992ea7ad8..eb947b9f7db 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetrics.cs @@ -19,6 +19,7 @@ using System.Diagnostics.Metrics; using System.Reflection; using OpenTelemetry.Instrumentation.AspNetCore.Implementation; +using OpenTelemetry.Internal; namespace OpenTelemetry.Instrumentation.AspNetCore { @@ -44,13 +45,12 @@ internal sealed class AspNetCoreMetrics : IDisposable private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; private readonly Meter meter; - /// - /// Initializes a new instance of the class. - /// - public AspNetCoreMetrics() + internal AspNetCoreMetrics(AspNetCoreMetricsInstrumentationOptions options) { + Guard.ThrowIfNull(options); this.meter = new Meter(InstrumentationName, InstrumentationVersion); - this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpInMetricsListener("Microsoft.AspNetCore", this.meter), this.isEnabled); + var metricsListener = new HttpInMetricsListener("Microsoft.AspNetCore", this.meter, options); + this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(metricsListener, this.isEnabled); this.diagnosticSourceSubscriber.Subscribe(); } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetricsInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetricsInstrumentationOptions.cs new file mode 100644 index 00000000000..3e7b6bd8f96 --- /dev/null +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/AspNetCoreMetricsInstrumentationOptions.cs @@ -0,0 +1,45 @@ +// +// 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; +using Microsoft.AspNetCore.Http; + +namespace OpenTelemetry.Instrumentation.AspNetCore +{ + /// + /// Options for metrics requests instrumentation. + /// + public class AspNetCoreMetricsInstrumentationOptions + { + /// + /// Gets or sets a Filter function that determines whether or not to collect telemetry about requests on a per request basis. + /// The Filter gets the HttpContext, and should return a boolean. + /// If Filter returns true, the request is collected. + /// If Filter returns false or throw exception, the request is filtered out. + /// + public Func Filter { get; set; } + + /// + /// Gets or sets an function to enrich an recorded metric with additional custom tags. + /// + /// + /// : the HttpContext object. + /// + public Func>> EnrichWithCustomTags { get; set; } + } +} diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs index ea025fb5004..669a2dfd657 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInMetricsListener.cs @@ -14,12 +14,14 @@ // limitations under the License. // +using System; using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.Metrics; using Microsoft.AspNetCore.Http; #if NET6_0_OR_GREATER using Microsoft.AspNetCore.Routing; +using OpenTelemetry.Internal; #endif using OpenTelemetry.Trace; @@ -30,12 +32,14 @@ internal sealed class HttpInMetricsListener : ListenerHandler private const string OnStopEvent = "Microsoft.AspNetCore.Hosting.HttpRequestIn.Stop"; private readonly Meter meter; + private readonly AspNetCoreMetricsInstrumentationOptions options; private readonly Histogram httpServerDuration; - public HttpInMetricsListener(string name, Meter meter) + internal HttpInMetricsListener(string name, Meter meter, AspNetCoreMetricsInstrumentationOptions options) : base(name) { this.meter = meter; + this.options = options; this.httpServerDuration = meter.CreateHistogram("http.server.duration", "ms", "measures the duration of the inbound HTTP request"); } @@ -50,6 +54,12 @@ public override void OnEventWritten(string name, object payload) return; } + if (this.options.Filter?.Invoke(context) == false) + { + AspNetCoreInstrumentationEventSource.Log.RequestIsFilteredOut(Activity.Current.OperationName); + return; + } + // TODO: Prometheus pulls metrics by invoking the /metrics endpoint. Decide if it makes sense to suppress this. // Below is just a temporary way of achieving this suppression for metrics (we should consider suppressing traces too). // If we want to suppress activity from Prometheus then we should use SuppressInstrumentationScope. @@ -81,6 +91,15 @@ public override void OnEventWritten(string name, object payload) tags.Add(new KeyValuePair(SemanticConventions.AttributeHttpRoute, route)); } #endif + if (this.options.EnrichWithCustomTags != null) + { + var enrichedTags = this.options.EnrichWithCustomTags(context); + + foreach (var keyValuePair in enrichedTags) + { + tags.Add(keyValuePair); + } + } this.httpServerDuration.Record(Activity.Current.Duration.TotalMilliseconds, tags); } diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs index 8679638c559..edf88b085a0 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/MeterProviderBuilderExtensions.cs @@ -14,6 +14,9 @@ // limitations under the License. // +using System; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using OpenTelemetry.Instrumentation.AspNetCore; using OpenTelemetry.Internal; @@ -31,20 +34,56 @@ public static class MeterProviderBuilderExtensions /// The instance of to chain the calls. public static MeterProviderBuilder AddAspNetCoreInstrumentation( this MeterProviderBuilder builder) + => AddAspNetCoreInstrumentation(builder, name: null, configureAspNetCoreInstrumentationOptions: null); + + /// + /// Enables the incoming requests automatic data collection for ASP.NET Core. + /// + /// being configured. + /// Callback action for configuring . + /// The instance of to chain the calls. + public static MeterProviderBuilder AddAspNetCoreInstrumentation( + this MeterProviderBuilder builder, + Action configureAspNetCoreInstrumentationOptions) + => AddAspNetCoreInstrumentation(builder, name: null, configureAspNetCoreInstrumentationOptions); + + /// + /// Enables the incoming requests automatic data collection for ASP.NET Core. + /// + /// being configured. + /// Name which is used when retrieving options. + /// Callback action for configuring . + /// The instance of to chain the calls. + public static MeterProviderBuilder AddAspNetCoreInstrumentation( + this MeterProviderBuilder builder, + string name, + Action configureAspNetCoreInstrumentationOptions) { Guard.ThrowIfNull(builder); - // TODO: Implement an IDeferredMeterProviderBuilder + name ??= Options.DefaultName; + + if (configureAspNetCoreInstrumentationOptions != null) + { + builder.ConfigureServices(services => services.Configure(name, configureAspNetCoreInstrumentationOptions)); + } + + builder.ConfigureBuilder((sp, builder) => + { + var options = sp.GetRequiredService>().Get(name); + + // TODO: Implement an IDeferredMeterProviderBuilder + + // TODO: Add AspNetCoreMetricsInstrumentationOptions ? + // RecordException - probably doesn't make sense for metric instrumentation + // EnableGrpcAspNetCoreSupport - this instrumentation will also need to also handle gRPC requests - // TODO: Handle AspNetCoreInstrumentationOptions - // Filter - makes sense for metric instrumentation - // Enrich - do we want a similar kind of functionality for metrics? - // RecordException - probably doesn't make sense for metric instrumentation - // EnableGrpcAspNetCoreSupport - this instrumentation will also need to also handle gRPC requests + var instrumentation = new AspNetCoreMetrics(options); + builder.AddMeter(AspNetCoreMetrics.InstrumentationName); + builder.AddInstrumentation(() => instrumentation); + }); - var instrumentation = new AspNetCoreMetrics(); - builder.AddMeter(AspNetCoreMetrics.InstrumentationName); - return builder.AddInstrumentation(() => instrumentation); + return builder; } } } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/DependencyInjectionConfigTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/DependencyInjectionConfigTests.cs index 32fa500d0a4..447a54902ac 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/DependencyInjectionConfigTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/DependencyInjectionConfigTests.cs @@ -18,6 +18,7 @@ using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using OpenTelemetry.Metrics; using OpenTelemetry.Trace; using Xunit; @@ -36,7 +37,7 @@ public DependencyInjectionConfigTests(WebApplicationFactory factory) [Theory] [InlineData(null)] [InlineData("CustomName")] - public void TestDIConfig(string name) + public void TestTracingOptionsDIConfig(string name) { name ??= Options.DefaultName; @@ -62,5 +63,35 @@ void ConfigureTestServices(IServiceCollection services) Assert.True(optionsPickedFromDI); } + + [Theory] + [InlineData(null)] + [InlineData("CustomName")] + public void TestMetricsOptionsDIConfig(string name) + { + name ??= Options.DefaultName; + + bool optionsPickedFromDI = false; + void ConfigureTestServices(IServiceCollection services) + { + services.AddOpenTelemetryMetrics( + builder => builder.AddAspNetCoreInstrumentation(name, configureAspNetCoreInstrumentationOptions: null)); + + services.Configure(name, options => + { + optionsPickedFromDI = true; + }); + } + + // Arrange + using (var client = this.factory + .WithWebHostBuilder(builder => + builder.ConfigureTestServices(ConfigureTestServices)) + .CreateClient()) + { + } + + Assert.True(optionsPickedFromDI); + } } } diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index 9ed66c4220e..72f88e68e1c 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -20,6 +20,8 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; @@ -33,6 +35,8 @@ public class MetricTests private readonly WebApplicationFactory factory; private MeterProvider meterProvider = null; + private const int StandardTagsCount = 6; + public MetricTests(WebApplicationFactory factory) { this.factory = factory; @@ -61,6 +65,102 @@ public async Task RequestMetricIsCaptured() builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); }) .CreateClient()) + { + var response1 = await client.GetAsync("/api/values"); + var response2 = await client.GetAsync("/api/values2"); + + response1.EnsureSuccessStatusCode(); + response2.EnsureSuccessStatusCode(); + } + + // We need to let End callback execute as it is executed AFTER response was returned. + // In unit tests environment there may be a lot of parallel unit tests executed, so + // giving some breezing room for the End callback to complete + await Task.Delay(TimeSpan.FromSeconds(1)); + + this.meterProvider.Dispose(); + + var requestMetrics = metricItems + .Where(item => item.Name == "http.server.duration") + .ToArray(); + + Assert.True(requestMetrics.Length == 2); + + AssertMetric(requestMetrics[0]); + AssertMetric(requestMetrics[1]); + } + + [Fact] + public async Task MetricNotCollectedWhenFilterIsApplied() + { + var metricItems = new List(); + + void ConfigureTestServices(IServiceCollection services) + { + this.meterProvider = Sdk.CreateMeterProviderBuilder() + .AddAspNetCoreInstrumentation(opt => opt.Filter = (ctx) => ctx.Request.Path != "/api/values/2") + .AddInMemoryExporter(metricItems) + .Build(); + } + + using (var client = this.factory + .WithWebHostBuilder(builder => + { + builder.ConfigureTestServices(ConfigureTestServices); + builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); + }) + .CreateClient()) + { + var response1 = await client.GetAsync("/api/values"); + var response2 = await client.GetAsync("/api/values2"); + + response1.EnsureSuccessStatusCode(); + response2.EnsureSuccessStatusCode(); + } + + // We need to let End callback execute as it is executed AFTER response was returned. + // In unit tests environment there may be a lot of parallel unit tests executed, so + // giving some breezing room for the End callback to complete + await Task.Delay(TimeSpan.FromSeconds(1)); + + this.meterProvider.Dispose(); + + var requestMetrics = metricItems + .Where(item => item.Name == "http.server.duration") + .ToArray(); + + // Made two request to separate endpoints but one should be filtered out + Assert.True(requestMetrics.Length == 1); + + AssertMetric(requestMetrics[0]); + } + + [Fact] + public async Task MetricEnrichedWithCustomTags() + { + var tagsToAdd = new List>() + { + new("custom_tag_1", 1), + new("custom_tag_2", "one") + }; + + var metricItems = new List(); + + void ConfigureTestServices(IServiceCollection services) + { + this.meterProvider = Sdk.CreateMeterProviderBuilder() + .AddAspNetCoreInstrumentation(opt => opt.EnrichWithCustomTags = (ctx) => tagsToAdd) + .AddInMemoryExporter(metricItems) + .Build(); + } + + using (var client = this.factory + .WithWebHostBuilder(builder => + { + builder.ConfigureTestServices(ConfigureTestServices); + builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); + }) + .CreateClient()) { var response = await client.GetAsync("/api/values"); response.EnsureSuccessStatusCode(); @@ -79,7 +179,16 @@ public async Task RequestMetricIsCaptured() Assert.True(requestMetrics.Length == 1); - var metric = requestMetrics[0]; + var expectedTagsCount = StandardTagsCount + tagsToAdd.Count; + + var tags = AssertMetric(requestMetrics[0], expectedTagsCount); + + Assert.Contains(tagsToAdd[0], tags); + Assert.Contains(tagsToAdd[1], tags); + } + + private static KeyValuePair[] AssertMetric(Metric metric, int expectedTagsCount = StandardTagsCount) + { Assert.NotNull(metric); Assert.True(metric.MetricType == MetricType.Histogram); var metricPoints = new List(); @@ -127,7 +236,9 @@ public async Task RequestMetricIsCaptured() Assert.Contains(flavor, attributes); Assert.Contains(host, attributes); Assert.Contains(route, attributes); - Assert.Equal(6, attributes.Length); + Assert.Equal(expectedTagsCount, attributes.Length); + + return attributes; } public void Dispose()