From f9c300b2849f7552756d6ca23a6aaa054a117dd2 Mon Sep 17 00:00:00 2001 From: Nils Gruson Date: Wed, 22 Nov 2023 23:05:10 +0100 Subject: [PATCH] Use test host in unit tests (#5040) --- .../BasicTests.cs | 322 ++++++++---------- .../MetricTests.cs | 94 ++--- 2 files changed, 191 insertions(+), 225 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index 17a414554..a138be0d0 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -23,6 +23,7 @@ using Microsoft.AspNetCore.Mvc.Testing; 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 sealed class BasicTests { 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,87 +897,8 @@ public sealed class BasicTests int numberOfUnSubscribedEvents = 0; int numberofSubscribedEvents = 0; int numberOfExceptionCallbacks = 0; - void ConfigureTestServices(IServiceCollection services) - { - this.tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddAspNetCoreInstrumentation( - new TestHttpInListener(new AspNetCoreInstrumentationOptions()) - { - OnEventWrittenCallback = (name, payload) => - { - switch (name) - { - case HttpInListener.OnStartEvent: - { - numberofSubscribedEvents++; - } - break; - case HttpInListener.OnStopEvent: - { - numberofSubscribedEvents++; - } - - break; - - // 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; - } - }, - }) - .Build(); - } - - // Arrange - using (var client = this.factory - .WithWebHostBuilder(builder => - { - builder.ConfigureTestServices(ConfigureTestServices); - builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); - }) - .CreateClient()) - { - try - { - using var request = new HttpRequestMessage(HttpMethod.Get, "/api/error"); - - // Act - using var response = await client.SendAsync(request); - } - catch - { - // ignore exception - } - } - - Assert.Equal(1, numberOfExceptionCallbacks); - Assert.Equal(0, numberOfUnSubscribedEvents); - Assert.Equal(3, numberofSubscribedEvents); - } - - [Fact(Skip = "https://github.com/open-telemetry/opentelemetry-dotnet/issues/4884")] - public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHandledInMiddleware() - { - int numberOfUnSubscribedEvents = 0; - int numberofSubscribedEvents = 0; - int numberOfExceptionCallbacks = 0; - - // configure SDK - using var tracerprovider = Sdk.CreateTracerProviderBuilder() + this.tracerProvider = Sdk.CreateTracerProviderBuilder() .AddAspNetCoreInstrumentation( new TestHttpInListener(new AspNetCoreInstrumentationOptions()) { @@ -1015,51 +934,117 @@ public sealed class BasicTests numberOfUnSubscribedEvents++; } + break; + } + }, + }) + .Build(); + + // Arrange + using (var client = this.factory + .WithWebHostBuilder(builder => + { + builder.ConfigureLogging(loggingBuilder => loggingBuilder.ClearProviders()); + }) + .CreateClient()) + { + try + { + using var request = new HttpRequestMessage(HttpMethod.Get, "/api/error"); + + // Act + using var response = await client.SendAsync(request); + } + catch + { + // ignore exception + } + } + + Assert.Equal(1, numberOfExceptionCallbacks); + Assert.Equal(0, numberOfUnSubscribedEvents); + Assert.Equal(3, numberofSubscribedEvents); + } + + [Fact] + public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHandledInMiddleware() + { + int numberOfUnSubscribedEvents = 0; + int numberOfSubscribedEvents = 0; + int numberOfExceptionCallbacks = 0; + + // configure SDK + this.tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddAspNetCoreInstrumentation( + new TestHttpInListener(new AspNetCoreInstrumentationOptions()) + { + OnEventWrittenCallback = (name, payload) => + { + switch (name) + { + case HttpInListener.OnStartEvent: + { + numberOfSubscribedEvents++; + } + + break; + case HttpInListener.OnStopEvent: + { + numberOfSubscribedEvents++; + } + + break; + + // 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; } }, }) .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 @@ public sealed class BasicTests .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 sealed class BasicTests } } - private class TestSampler : Sampler + private class TestSampler(SamplingDecision samplingDecision, IEnumerable> 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 sealed class BasicTests } } - 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 sealed class BasicTests } } - 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 sealed class BasicTests } } - 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 6497fed98..b5769f4cb 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/MetricTests.cs @@ -26,6 +26,10 @@ using Microsoft.AspNetCore.Mvc.Testing; #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 @@ using Xunit; 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 class MetricTests 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 class MetricTests [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 class MetricTests _ = 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 class MetricTests 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);