diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index 17a4145542b..a138be0d012 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -23,6 +23,7 @@ using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Moq; using OpenTelemetry.Context.Propagation; @@ -840,45 +841,42 @@ public async Task DiagnosticSourceCallbacksAreReceivedOnlyForSubscribedEvents() { int numberOfUnSubscribedEvents = 0; int numberofSubscribedEvents = 0; - void ConfigureTestServices(IServiceCollection services) - { - this.tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddAspNetCoreInstrumentation( - new TestHttpInListener(new AspNetCoreInstrumentationOptions()) + + this.tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddAspNetCoreInstrumentation( + new TestHttpInListener(new AspNetCoreInstrumentationOptions()) + { + OnEventWrittenCallback = (name, payload) => { - OnEventWrittenCallback = (name, payload) => + switch (name) { - switch (name) - { - case HttpInListener.OnStartEvent: - { - numberofSubscribedEvents++; - } + case HttpInListener.OnStartEvent: + { + numberofSubscribedEvents++; + } - break; - case HttpInListener.OnStopEvent: - { - numberofSubscribedEvents++; - } + break; + case HttpInListener.OnStopEvent: + { + numberofSubscribedEvents++; + } - break; - default: - { - numberOfUnSubscribedEvents++; - } + break; + default: + { + numberOfUnSubscribedEvents++; + } - break; - } - }, - }) - .Build(); - } + break; + } + }, + }) + .Build(); // Arrange using (var client = this.factory .WithWebHostBuilder(builder => { - builder.ConfigureTestServices(ConfigureTestServices); builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); }) .CreateClient()) @@ -899,56 +897,53 @@ public async Task DiagnosticSourceExceptionCallbackIsReceivedForUnHandledExcepti int numberOfUnSubscribedEvents = 0; int numberofSubscribedEvents = 0; int numberOfExceptionCallbacks = 0; - void ConfigureTestServices(IServiceCollection services) - { - this.tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddAspNetCoreInstrumentation( - new TestHttpInListener(new AspNetCoreInstrumentationOptions()) + + this.tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddAspNetCoreInstrumentation( + new TestHttpInListener(new AspNetCoreInstrumentationOptions()) + { + OnEventWrittenCallback = (name, payload) => { - OnEventWrittenCallback = (name, payload) => + switch (name) { - switch (name) - { - case HttpInListener.OnStartEvent: - { - numberofSubscribedEvents++; - } + case HttpInListener.OnStartEvent: + { + numberofSubscribedEvents++; + } - break; - case HttpInListener.OnStopEvent: - { - numberofSubscribedEvents++; - } + break; + case HttpInListener.OnStopEvent: + { + numberofSubscribedEvents++; + } - break; + break; - // TODO: Add test case for validating name for both the types - // of exception event. - case HttpInListener.OnUnhandledHostingExceptionEvent: - case HttpInListener.OnUnHandledDiagnosticsExceptionEvent: - { - numberofSubscribedEvents++; - numberOfExceptionCallbacks++; - } + // TODO: Add test case for validating name for both the types + // of exception event. + case HttpInListener.OnUnhandledHostingExceptionEvent: + case HttpInListener.OnUnHandledDiagnosticsExceptionEvent: + { + numberofSubscribedEvents++; + numberOfExceptionCallbacks++; + } - break; - default: - { - numberOfUnSubscribedEvents++; - } + break; + default: + { + numberOfUnSubscribedEvents++; + } - break; - } - }, - }) - .Build(); - } + break; + } + }, + }) + .Build(); // Arrange using (var client = this.factory .WithWebHostBuilder(builder => { - builder.ConfigureTestServices(ConfigureTestServices); builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); }) .CreateClient()) @@ -971,15 +966,15 @@ void ConfigureTestServices(IServiceCollection services) Assert.Equal(3, numberofSubscribedEvents); } - [Fact(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet/issues/4884")] + [Fact] public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHandledInMiddleware() { int numberOfUnSubscribedEvents = 0; - int numberofSubscribedEvents = 0; + int numberOfSubscribedEvents = 0; int numberOfExceptionCallbacks = 0; // configure SDK - using var tracerprovider = Sdk.CreateTracerProviderBuilder() + this.tracerProvider = Sdk.CreateTracerProviderBuilder() .AddAspNetCoreInstrumentation( new TestHttpInListener(new AspNetCoreInstrumentationOptions()) { @@ -989,13 +984,13 @@ public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHan { case HttpInListener.OnStartEvent: { - numberofSubscribedEvents++; + numberOfSubscribedEvents++; } break; case HttpInListener.OnStopEvent: { - numberofSubscribedEvents++; + numberOfSubscribedEvents++; } break; @@ -1005,7 +1000,7 @@ public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHan case HttpInListener.OnUnhandledHostingExceptionEvent: case HttpInListener.OnUnHandledDiagnosticsExceptionEvent: { - numberofSubscribedEvents++; + numberOfSubscribedEvents++; numberOfExceptionCallbacks++; } @@ -1021,45 +1016,35 @@ public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHan }) .Build(); - var builder = WebApplication.CreateBuilder(); - builder.Logging.ClearProviders(); - var app = builder.Build(); - - app.UseExceptionHandler(handler => - { - handler.Run(async (ctx) => + using (var client = this.factory + .WithWebHostBuilder(builder => { - await ctx.Response.WriteAsync("handled"); - }); - }); - - app.Map("/error", ThrowException); - - static void ThrowException(IApplicationBuilder app) + builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); + builder.Configure(app => app + .UseExceptionHandler(handler => + { + handler.Run(async (ctx) => + { + await ctx.Response.WriteAsync("handled"); + }); + })); + }) + .CreateClient()) { - app.Run(context => + try { - throw new Exception("CustomException"); - }); - } - - _ = app.RunAsync(); - - using var client = new HttpClient(); - try - { - await client.GetStringAsync("http://localhost:5000/error"); - } - catch - { - // ignore 500 error. + using var request = new HttpRequestMessage(HttpMethod.Get, "/api/error"); + using var response = await client.SendAsync(request); + } + catch + { + // ignore exception + } } Assert.Equal(0, numberOfExceptionCallbacks); Assert.Equal(0, numberOfUnSubscribedEvents); - Assert.Equal(2, numberofSubscribedEvents); - - await app.DisposeAsync(); + Assert.Equal(2, numberOfSubscribedEvents); } public void Dispose() @@ -1128,16 +1113,10 @@ private void ConfigureExceptionFilters(IServiceCollection services, int mode, re .Build(); } - private class ExtractOnlyPropagator : TextMapPropagator + private class ExtractOnlyPropagator(ActivityContext activityContext, Baggage baggage) : TextMapPropagator { - private readonly ActivityContext activityContext; - private readonly Baggage baggage; - - public ExtractOnlyPropagator(ActivityContext activityContext, Baggage baggage) - { - this.activityContext = activityContext; - this.baggage = baggage; - } + private readonly ActivityContext activityContext = activityContext; + private readonly Baggage baggage = baggage; public override ISet Fields => throw new NotImplementedException(); @@ -1152,16 +1131,10 @@ public override void Inject(PropagationContext context, T carrier, Action> attributes = null) : Sampler { - private readonly SamplingDecision samplingDecision; - private readonly IEnumerable> attributes; - - public TestSampler(SamplingDecision samplingDecision, IEnumerable> attributes = null) - { - this.samplingDecision = samplingDecision; - this.attributes = attributes; - } + private readonly SamplingDecision samplingDecision = samplingDecision; + private readonly IEnumerable> attributes = attributes; public override SamplingResult ShouldSample(in SamplingParameters samplingParameters) { @@ -1169,15 +1142,10 @@ public override SamplingResult ShouldSample(in SamplingParameters samplingParame } } - private class TestHttpInListener : HttpInListener + private class TestHttpInListener(AspNetCoreInstrumentationOptions options) : HttpInListener(options) { public Action OnEventWrittenCallback; - public TestHttpInListener(AspNetCoreInstrumentationOptions options) - : base(options) - { - } - public override void OnEventWritten(string name, object payload) { base.OnEventWritten(name, payload); @@ -1186,17 +1154,11 @@ public override void OnEventWritten(string name, object payload) } } - private class TestNullHostActivityMiddlewareImpl : ActivityMiddleware.ActivityMiddlewareImpl + private class TestNullHostActivityMiddlewareImpl(string activitySourceName, string activityName) : ActivityMiddleware.ActivityMiddlewareImpl { - private ActivitySource activitySource; + private readonly ActivitySource activitySource = new(activitySourceName); + private readonly string activityName = activityName; private Activity activity; - private string activityName; - - public TestNullHostActivityMiddlewareImpl(string activitySourceName, string activityName) - { - this.activitySource = new ActivitySource(activitySourceName); - this.activityName = activityName; - } public override void PreProcess(HttpContext context) { @@ -1214,17 +1176,11 @@ public override void PostProcess(HttpContext context) } } - private class TestActivityMiddlewareImpl : ActivityMiddleware.ActivityMiddlewareImpl + private class TestActivityMiddlewareImpl(string activitySourceName, string activityName) : ActivityMiddleware.ActivityMiddlewareImpl { - private ActivitySource activitySource; + private readonly ActivitySource activitySource = new(activitySourceName); + private readonly string activityName = activityName; private Activity activity; - private string activityName; - - public TestActivityMiddlewareImpl(string activitySourceName, string activityName) - { - this.activitySource = new ActivitySource(activitySourceName); - this.activityName = activityName; - } public override void PreProcess(HttpContext context) { diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs index 6497fed98de..b5769f4cb48 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -26,6 +26,10 @@ #if NET8_0_OR_GREATER using Microsoft.AspNetCore.RateLimiting; #endif +#if NET8_0_OR_GREATER +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +#endif using Microsoft.Extensions.Logging; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; @@ -33,48 +37,45 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Tests; -public class MetricTests - : IClassFixture>, IDisposable +public class MetricTests(WebApplicationFactory factory) + : IClassFixture>, IDisposable { private const int StandardTagsCount = 6; - private readonly WebApplicationFactory factory; + private readonly WebApplicationFactory factory = factory; private MeterProvider meterProvider; - public MetricTests(WebApplicationFactory factory) - { - this.factory = factory; - } - [Fact] public void AddAspNetCoreInstrumentation_BadArgs() { MeterProviderBuilder builder = null; - Assert.Throws(() => builder.AddAspNetCoreInstrumentation()); + Assert.Throws(builder.AddAspNetCoreInstrumentation); } #if NET8_0_OR_GREATER [Fact] public async Task ValidateNet8MetricsAsync() { - var metricItems = new List(); - + var exportedItems = new List(); this.meterProvider = Sdk.CreateMeterProviderBuilder() - .AddAspNetCoreInstrumentation() - .AddInMemoryExporter(metricItems) - .Build(); + .AddAspNetCoreInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build(); var builder = WebApplication.CreateBuilder(); - builder.Logging.ClearProviders(); + builder.WebHost.UseUrls("http://*:0"); var app = builder.Build(); app.MapGet("/", () => "Hello"); _ = app.RunAsync(); + var url = app.Urls.ToArray()[0]; + var portNumber = url.Substring(url.LastIndexOf(':') + 1); + using var client = new HttpClient(); - var res = await client.GetStringAsync("http://localhost:5000/"); - Assert.NotNull(res); + var res = await client.GetAsync($"http://localhost:{portNumber}/"); + Assert.True(res.IsSuccessStatusCode); // We need to let metric 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 @@ -83,20 +84,20 @@ public async Task ValidateNet8MetricsAsync() this.meterProvider.Dispose(); - var requestDurationMetric = metricItems + var requestDurationMetric = exportedItems .Count(item => item.Name == "http.server.request.duration"); - var activeRequestsMetric = metricItems. + var activeRequestsMetric = exportedItems. Count(item => item.Name == "http.server.active_requests"); - var routeMatchingMetric = metricItems. + var routeMatchingMetric = exportedItems. Count(item => item.Name == "aspnetcore.routing.match_attempts"); - var kestrelActiveConnectionsMetric = metricItems. - Count(item => item.Name == "kestrel.active_connections"); + var kestrelActiveConnectionsMetric = exportedItems. + Count(item => item.Name == "kestrel.active_connections"); - var kestrelQueuedConnectionMetric = metricItems. - Count(item => item.Name == "kestrel.queued_connections"); + var kestrelQueuedConnectionMetric = exportedItems. + Count(item => item.Name == "kestrel.queued_connections"); Assert.Equal(1, requestDurationMetric); Assert.Equal(1, activeRequestsMetric); @@ -117,22 +118,28 @@ public async Task ValidateNet8MetricsAsync() [Fact] public async Task ValidateNet8RateLimitingMetricsAsync() { - var metricItems = new List(); + var exportedItems = new List(); - this.meterProvider = Sdk.CreateMeterProviderBuilder() - .AddAspNetCoreInstrumentation() - .AddInMemoryExporter(metricItems) - .Build(); + void ConfigureTestServices(IServiceCollection services) + { + this.meterProvider = Sdk.CreateMeterProviderBuilder() + .AddAspNetCoreInstrumentation() + .AddInMemoryExporter(exportedItems) + .Build(); + + services.AddRateLimiter(_ => _ + .AddFixedWindowLimiter(policyName: "fixed", options => + { + options.PermitLimit = 4; + options.Window = TimeSpan.FromSeconds(12); + options.QueueProcessingOrder = QueueProcessingOrder.OldestFirst; + options.QueueLimit = 2; + })); + } var builder = WebApplication.CreateBuilder(); - builder.Services.AddRateLimiter(_ => _ - .AddFixedWindowLimiter(policyName: "fixed", options => - { - options.PermitLimit = 4; - options.Window = TimeSpan.FromSeconds(12); - options.QueueProcessingOrder = QueueProcessingOrder.OldestFirst; - options.QueueLimit = 2; - })); + builder.WebHost.UseUrls("http://*:0"); + ConfigureTestServices(builder.Services); builder.Logging.ClearProviders(); var app = builder.Build(); @@ -146,8 +153,11 @@ public async Task ValidateNet8RateLimitingMetricsAsync() _ = app.RunAsync(); + var url = app.Urls.ToArray()[0]; + var portNumber = url.Substring(url.LastIndexOf(':') + 1); + using var client = new HttpClient(); - var res = await client.GetStringAsync("http://localhost:5000/"); + var res = await client.GetAsync($"http://localhost:{portNumber}/"); Assert.NotNull(res); // We need to let metric callback execute as it is executed AFTER response was returned. @@ -157,19 +167,19 @@ public async Task ValidateNet8RateLimitingMetricsAsync() this.meterProvider.Dispose(); - var activeRequestleasesMetric = metricItems + var activeRequestLeasesMetric = exportedItems .Where(item => item.Name == "aspnetcore.rate_limiting.active_request_leases") .ToArray(); - var requestLeaseDurationMetric = metricItems. + var requestLeaseDurationMetric = exportedItems. Where(item => item.Name == "aspnetcore.rate_limiting.request_lease.duration") .ToArray(); - var limitingRequestsMetric = metricItems. + var limitingRequestsMetric = exportedItems. Where(item => item.Name == "aspnetcore.rate_limiting.requests") .ToArray(); - Assert.Single(activeRequestleasesMetric); + Assert.Single(activeRequestLeasesMetric); Assert.Single(requestLeaseDurationMetric); Assert.Single(limitingRequestsMetric);