From a42721dfa759fad58134e992a78a71bdbfbe0096 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Wed, 29 Nov 2023 18:14:48 +0100 Subject: [PATCH 1/8] Remove ApmMiddleWare, only use DiagnosticSource listener for ASP.NET Core --- ElasticApmAgent.sln | 7 ++ .../Controllers/ErrorController.cs | 16 +++++ .../Controllers/WeatherForecastController.cs | 29 ++++++++ sample/WebApiExample/Program.cs | 28 ++++++++ sample/WebApiExample/WeatherForecast.cs | 12 ++++ sample/WebApiExample/WebApiExample.csproj | 18 +++++ .../Elastic.Apm.AspNetCore/ApmMiddleware.cs | 72 ------------------- ...ion.cs => ApplicationBuilderExtensions.cs} | 21 +++--- .../AspNetCoreErrorDiagnosticListener.cs | 38 ---------- .../AspNetCoreErrorDiagnosticsSubscriber.cs | 36 ---------- ...ion.cs => ApplicationBuilderExtensions.cs} | 4 +- .../elastic_apm_profiler/src/profiler/mod.rs | 2 + ...ApplicationBuilderExtensionLoggingTest.cs} | 4 +- .../AspNetCoreBasicTests.cs | 6 +- .../AspNetCoreDiagnosticListenerTest.cs | 9 +++ .../BodyCapturingTests.cs | 14 +++- .../CaptureUserTest.cs | 33 ++------- .../Elastic.Apm.AspNetCore.Tests/Helper.cs | 2 +- .../Controllers/AccountController.cs | 3 +- 19 files changed, 161 insertions(+), 193 deletions(-) create mode 100644 sample/WebApiExample/Controllers/ErrorController.cs create mode 100644 sample/WebApiExample/Controllers/WeatherForecastController.cs create mode 100644 sample/WebApiExample/Program.cs create mode 100644 sample/WebApiExample/WeatherForecast.cs create mode 100644 sample/WebApiExample/WebApiExample.csproj delete mode 100644 src/integrations/Elastic.Apm.AspNetCore/ApmMiddleware.cs rename src/integrations/Elastic.Apm.AspNetCore/{ApmMiddlewareExtension.cs => ApplicationBuilderExtensions.cs} (87%) delete mode 100644 src/integrations/Elastic.Apm.AspNetCore/DiagnosticListener/AspNetCoreErrorDiagnosticListener.cs delete mode 100644 src/integrations/Elastic.Apm.AspNetCore/DiagnosticListener/AspNetCoreErrorDiagnosticsSubscriber.cs rename src/integrations/Elastic.Apm.NetCoreAll/{ApmMiddlewareExtension.cs => ApplicationBuilderExtensions.cs} (96%) rename test/integrations/Elastic.Apm.AspNetCore.Tests/{ApmMiddlewareExtensionTest.cs => ApplicationBuilderExtensionLoggingTest.cs} (88%) diff --git a/ElasticApmAgent.sln b/ElasticApmAgent.sln index c82db139d..aa8ce27e2 100644 --- a/ElasticApmAgent.sln +++ b/ElasticApmAgent.sln @@ -231,6 +231,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Elastic.Apm.AzureFunctionAp EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elastic.Apm.Profiling", "benchmarks\Elastic.Apm.Profiling\Elastic.Apm.Profiling.csproj", "{CB6B3BA6-9D16-4CDC-95C2-7680CF50747D}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WebApiExample", "sample\WebApiExample\WebApiExample.csproj", "{00A025F1-0A31-4676-AA06-1773FC9744ED}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -575,6 +577,10 @@ Global {CB6B3BA6-9D16-4CDC-95C2-7680CF50747D}.Debug|Any CPU.Build.0 = Debug|Any CPU {CB6B3BA6-9D16-4CDC-95C2-7680CF50747D}.Release|Any CPU.ActiveCfg = Release|Any CPU {CB6B3BA6-9D16-4CDC-95C2-7680CF50747D}.Release|Any CPU.Build.0 = Release|Any CPU + {00A025F1-0A31-4676-AA06-1773FC9744ED}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {00A025F1-0A31-4676-AA06-1773FC9744ED}.Debug|Any CPU.Build.0 = Debug|Any CPU + {00A025F1-0A31-4676-AA06-1773FC9744ED}.Release|Any CPU.ActiveCfg = Release|Any CPU + {00A025F1-0A31-4676-AA06-1773FC9744ED}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -677,6 +683,7 @@ Global {09CE5AC1-01F6-48C8-B266-2F891C408051} = {C2378D3C-2BA8-410F-AEF7-547C411C71C7} {50F14EA5-DF72-425B-81A6-C7D532D2DD07} = {09CE5AC1-01F6-48C8-B266-2F891C408051} {CB6B3BA6-9D16-4CDC-95C2-7680CF50747D} = {2825A761-5372-4620-99AB-253AD953E8CD} + {00A025F1-0A31-4676-AA06-1773FC9744ED} = {3C791D9C-6F19-4F46-B367-2EC0F818762D} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {69E02FD9-C9DE-412C-AB6B-5B8BECC6BFA5} diff --git a/sample/WebApiExample/Controllers/ErrorController.cs b/sample/WebApiExample/Controllers/ErrorController.cs new file mode 100644 index 000000000..b7e9fc2b8 --- /dev/null +++ b/sample/WebApiExample/Controllers/ErrorController.cs @@ -0,0 +1,16 @@ +using Microsoft.AspNetCore.Mvc; + +namespace WebApiExample.Controllers; + +[ApiController] +[Route("[controller]")] +public class ErrorController : ControllerBase +{ + private readonly ILogger _logger; + + public ErrorController(ILogger logger) => _logger = logger; + + [HttpGet(Name = "GetError")] + public IEnumerable Get() => + throw new Exception("This exception triggers a 500"); +} diff --git a/sample/WebApiExample/Controllers/WeatherForecastController.cs b/sample/WebApiExample/Controllers/WeatherForecastController.cs new file mode 100644 index 000000000..3aafe321d --- /dev/null +++ b/sample/WebApiExample/Controllers/WeatherForecastController.cs @@ -0,0 +1,29 @@ +using Microsoft.AspNetCore.Mvc; + +namespace WebApiExample.Controllers; + +[ApiController] +[Route("[controller]")] +public class WeatherForecastController : ControllerBase +{ + private static readonly string[] Summaries = new[] + { + "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching" + }; + + private readonly ILogger _logger; + + public WeatherForecastController(ILogger logger) => _logger = logger; + + [HttpGet(Name = "GetWeatherForecast")] + public IEnumerable Get() => + Enumerable.Range(1, 5) + .Select(index => new WeatherForecast + { + Date = DateOnly.FromDateTime(DateTime.Now.AddDays(index)), + TemperatureC = Random.Shared.Next(-20, 55), + Summary = Summaries[Random.Shared.Next(Summaries.Length)] + }) + .ToArray(); + +} diff --git a/sample/WebApiExample/Program.cs b/sample/WebApiExample/Program.cs new file mode 100644 index 000000000..757c87055 --- /dev/null +++ b/sample/WebApiExample/Program.cs @@ -0,0 +1,28 @@ +using Elastic.Apm.AspNetCore; + +var builder = WebApplication.CreateBuilder(args); + +// Add services to the container. + +builder.Services.AddControllers(); +// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle +builder.Services.AddEndpointsApiExplorer(); +builder.Services.AddSwaggerGen(); + +var app = builder.Build(); +app.UseElasticApm(app.Configuration); + +// Configure the HTTP request pipeline. +if (app.Environment.IsDevelopment()) +{ + app.UseSwagger(); + app.UseSwaggerUI(); +} + +app.UseHttpsRedirection(); + +app.UseAuthorization(); + +app.MapControllers(); + +app.Run(); diff --git a/sample/WebApiExample/WeatherForecast.cs b/sample/WebApiExample/WeatherForecast.cs new file mode 100644 index 000000000..2528b8620 --- /dev/null +++ b/sample/WebApiExample/WeatherForecast.cs @@ -0,0 +1,12 @@ +namespace WebApiExample; + +public class WeatherForecast +{ + public DateOnly Date { get; set; } + + public int TemperatureC { get; set; } + + public int TemperatureF => 32 + (int)(TemperatureC / 0.5556); + + public string? Summary { get; set; } +} diff --git a/sample/WebApiExample/WebApiExample.csproj b/sample/WebApiExample/WebApiExample.csproj new file mode 100644 index 000000000..834a55587 --- /dev/null +++ b/sample/WebApiExample/WebApiExample.csproj @@ -0,0 +1,18 @@ + + + + net7.0 + enable + enable + + + + + + + + + + + + diff --git a/src/integrations/Elastic.Apm.AspNetCore/ApmMiddleware.cs b/src/integrations/Elastic.Apm.AspNetCore/ApmMiddleware.cs deleted file mode 100644 index 1fa5a191a..000000000 --- a/src/integrations/Elastic.Apm.AspNetCore/ApmMiddleware.cs +++ /dev/null @@ -1,72 +0,0 @@ -// Licensed to Elasticsearch B.V under one or more agreements. -// Elasticsearch B.V 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.Threading.Tasks; -using Elastic.Apm.Api; -using Elastic.Apm.Extensions; -using Elastic.Apm.Logging; -using Elastic.Apm.Model; -using Microsoft.AspNetCore.Http; - -namespace Elastic.Apm.AspNetCore -{ - // ReSharper disable once ClassNeverInstantiated.Global - internal class ApmMiddleware - { - private readonly IApmLogger _logger; - - private readonly RequestDelegate _next; - private readonly Tracer _tracer; - private readonly ApmAgent _agent; - - public ApmMiddleware(RequestDelegate next, Tracer tracer, ApmAgent agent) - { - _next = next; - _tracer = tracer; - _logger = agent.Logger.Scoped(nameof(ApmMiddleware)); - _agent = agent; - } - - public async Task InvokeAsync(HttpContext context) - { - var createdTransaction = WebRequestTransactionCreator.StartTransactionAsync(context, _logger, _tracer, _agent.ConfigurationStore.CurrentSnapshot); - - Transaction transaction = null; - if (createdTransaction is Transaction t) - transaction = t; - - if (transaction != null) - WebRequestTransactionCreator.FillSampledTransactionContextRequest(transaction, context, _logger); - - try - { - await _next(context); - } - catch (Exception e) when (CaptureExceptionAndRequestBody(e, context, transaction)) { } - finally - { - // In case an error handler middleware is registered, the catch block above won't be executed, because the - // error handler handles all the exceptions - in this case, based on the response code and the config, we may capture the body here - if (transaction != null && transaction.IsContextCreated && context?.Response.StatusCode >= 400 - && transaction.Context?.Request?.Body is string body - && (string.IsNullOrEmpty(body) || body == Apm.Consts.Redacted)) - transaction.CollectRequestBody(true, new AspNetCoreHttpRequest(context.Request), _logger); - - if (transaction != null) - WebRequestTransactionCreator.StopTransaction(transaction, context, _logger); - else - createdTransaction?.End(); - } - } - - private bool CaptureExceptionAndRequestBody(Exception e, HttpContext context, Transaction transaction) - { - transaction?.CaptureException(e); - if (context != null) - transaction?.CollectRequestBody(true, new AspNetCoreHttpRequest(context.Request), _logger); - return false; - } - } -} diff --git a/src/integrations/Elastic.Apm.AspNetCore/ApmMiddlewareExtension.cs b/src/integrations/Elastic.Apm.AspNetCore/ApplicationBuilderExtensions.cs similarity index 87% rename from src/integrations/Elastic.Apm.AspNetCore/ApmMiddlewareExtension.cs rename to src/integrations/Elastic.Apm.AspNetCore/ApplicationBuilderExtensions.cs index 162785907..54445323a 100644 --- a/src/integrations/Elastic.Apm.AspNetCore/ApmMiddlewareExtension.cs +++ b/src/integrations/Elastic.Apm.AspNetCore/ApplicationBuilderExtensions.cs @@ -18,17 +18,21 @@ namespace Elastic.Apm.AspNetCore { - public static class ApmMiddlewareExtension + public static class ApplicationBuilderExtensions { /// - /// Adds the Elastic APM Middleware to the ASP.NET Core pipeline. + /// Sets up ASP.NET Core instrumentation to be sent over to Elastic APM. + /// /// You can customize the agent by passing additional IDiagnosticsSubscriber components to this method. /// Use this method if you want to control what tracing capability of the agent you would like to use /// or in case you want to minimize the number of dependencies added to your application. + /// + /// /// Please note that by default without additional parameters this method only enables ASP.NET Core /// monitoring - e.g. database statements or outgoing HTTP calls won't be traced. /// If you want to simply enable every tracing component without configuration please use the - /// UseAllElasticApm extension method from the Elastic.Apm.NetCoreAll package. + /// UseAllElasticApm() extension method from the Elastic.Apm.NetCoreAll package. + /// /// /// The elastic apm. /// Builder. @@ -38,8 +42,8 @@ public static class ApmMiddlewareExtension /// If no is passed to the agent then it will read configs from environment variables. /// /// - /// Specify which diagnostic source subscribers you want to connect. The - /// is by default enabled. + /// Specify which diagnostic source subscribers you want to connect. + /// The will always be injected if not specified. /// public static IApplicationBuilder UseElasticApm( this IApplicationBuilder builder, @@ -80,11 +84,12 @@ params IDiagnosticsSubscriber[] subscribers var subs = subscribers?.ToList() ?? new List(1); - if (subs.Count == 0 || subs.All(s => s.GetType() != typeof(AspNetCoreErrorDiagnosticsSubscriber))) - subs.Add(new AspNetCoreErrorDiagnosticsSubscriber()); + if (subs.Count == 0 || subs.All(s => s.GetType() != typeof(AspNetCoreDiagnosticSubscriber))) + subs.Add(new AspNetCoreDiagnosticSubscriber()); agent.Subscribe(subs.ToArray()); - return builder.UseMiddleware(agent.Tracer, agent); + //return builder.UseMiddleware(agent.Tracer, agent); + return builder; } private static string GetEnvironmentName(this IServiceProvider serviceProvider) => diff --git a/src/integrations/Elastic.Apm.AspNetCore/DiagnosticListener/AspNetCoreErrorDiagnosticListener.cs b/src/integrations/Elastic.Apm.AspNetCore/DiagnosticListener/AspNetCoreErrorDiagnosticListener.cs deleted file mode 100644 index b60ab3472..000000000 --- a/src/integrations/Elastic.Apm.AspNetCore/DiagnosticListener/AspNetCoreErrorDiagnosticListener.cs +++ /dev/null @@ -1,38 +0,0 @@ -// Licensed to Elasticsearch B.V under -// one or more agreements. -// Elasticsearch B.V 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.Reflection; -using Elastic.Apm.DiagnosticListeners; -using Elastic.Apm.Extensions; -using Elastic.Apm.Model; -using Microsoft.AspNetCore.Http; - -namespace Elastic.Apm.AspNetCore.DiagnosticListener -{ - internal class AspNetCoreErrorDiagnosticListener : DiagnosticListenerBase - { - public AspNetCoreErrorDiagnosticListener(IApmAgent agent) : base(agent) { } - - public override string Name => "Microsoft.AspNetCore"; - - protected override void HandleOnNext(KeyValuePair kv) - { - if (kv.Key != "Microsoft.AspNetCore.Diagnostics.UnhandledException" - && kv.Key != "Microsoft.AspNetCore.Diagnostics.HandledException") - return; - - var exception = kv.Value.GetType().GetTypeInfo().GetDeclaredProperty("exception")?.GetValue(kv.Value) as Exception; - var httpContextUnhandledException = - kv.Value.GetType().GetTypeInfo().GetDeclaredProperty("httpContext")?.GetValue(kv.Value) as DefaultHttpContext; - - var transaction = ApmAgent.Tracer.CurrentTransaction as Transaction; - transaction?.CollectRequestBody(true, new AspNetCoreHttpRequest(httpContextUnhandledException?.Request), Logger); - transaction?.CaptureException(exception, "ASP.NET Core Unhandled Exception", - kv.Key == "Microsoft.AspNetCore.Diagnostics.HandledException"); - } - } -} diff --git a/src/integrations/Elastic.Apm.AspNetCore/DiagnosticListener/AspNetCoreErrorDiagnosticsSubscriber.cs b/src/integrations/Elastic.Apm.AspNetCore/DiagnosticListener/AspNetCoreErrorDiagnosticsSubscriber.cs deleted file mode 100644 index 5e4a1ca4b..000000000 --- a/src/integrations/Elastic.Apm.AspNetCore/DiagnosticListener/AspNetCoreErrorDiagnosticsSubscriber.cs +++ /dev/null @@ -1,36 +0,0 @@ -// Licensed to Elasticsearch B.V under one or more agreements. -// Elasticsearch B.V 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 Elastic.Apm.DiagnosticSource; - -namespace Elastic.Apm.AspNetCore.DiagnosticListener -{ - /// - /// Activates the which enables - /// capturing errors within an ASP.NET Core application. - /// This is used in combination with to capture errors handled by error pages - /// - public class AspNetCoreErrorDiagnosticsSubscriber : IDiagnosticsSubscriber - { - /// - /// Start listening for ASP.NET Core related diagnostic source events. - /// - public IDisposable Subscribe(IApmAgent agent) - { - var retVal = new CompositeDisposable(); - if (!agent.Configuration.Enabled) - return retVal; - - var subscriber = new DiagnosticInitializer(agent, new AspNetCoreErrorDiagnosticListener(agent)); - retVal.Add(subscriber); - - retVal.Add(System.Diagnostics.DiagnosticListener - .AllListeners - .Subscribe(subscriber)); - - return retVal; - } - } -} diff --git a/src/integrations/Elastic.Apm.NetCoreAll/ApmMiddlewareExtension.cs b/src/integrations/Elastic.Apm.NetCoreAll/ApplicationBuilderExtensions.cs similarity index 96% rename from src/integrations/Elastic.Apm.NetCoreAll/ApmMiddlewareExtension.cs rename to src/integrations/Elastic.Apm.NetCoreAll/ApplicationBuilderExtensions.cs index 2f8304166..1d24ac0ba 100644 --- a/src/integrations/Elastic.Apm.NetCoreAll/ApmMiddlewareExtension.cs +++ b/src/integrations/Elastic.Apm.NetCoreAll/ApplicationBuilderExtensions.cs @@ -16,7 +16,7 @@ namespace Elastic.Apm.NetCoreAll { - public static class ApmMiddlewareExtension + public static class ApplicationBuilderExtensions { /// /// Adds the Elastic APM Middleware to the ASP.NET Core pipeline and enables @@ -45,7 +45,7 @@ public static class ApmMiddlewareExtension public static IApplicationBuilder UseAllElasticApm( this IApplicationBuilder builder, IConfiguration configuration = null - ) => AspNetCore.ApmMiddlewareExtension + ) => AspNetCore.ApplicationBuilderExtensions .UseElasticApm(builder, configuration, new HttpDiagnosticsSubscriber(), new SqlClientDiagnosticSubscriber(), diff --git a/src/profiler/elastic_apm_profiler/src/profiler/mod.rs b/src/profiler/elastic_apm_profiler/src/profiler/mod.rs index 465ce3ae6..dcbd200b3 100644 --- a/src/profiler/elastic_apm_profiler/src/profiler/mod.rs +++ b/src/profiler/elastic_apm_profiler/src/profiler/mod.rs @@ -487,6 +487,8 @@ impl Profiler { unsafe { unknown.AddRef(); } + + println!("hello world init"); let process_path = std::env::current_exe().map_err(|e| { // logging hasn't yet been initialized so unable to log diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/ApmMiddlewareExtensionTest.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/ApplicationBuilderExtensionLoggingTest.cs similarity index 88% rename from test/integrations/Elastic.Apm.AspNetCore.Tests/ApmMiddlewareExtensionTest.cs rename to test/integrations/Elastic.Apm.AspNetCore.Tests/ApplicationBuilderExtensionLoggingTest.cs index 6879b1a19..849f2e77f 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/ApmMiddlewareExtensionTest.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/ApplicationBuilderExtensionLoggingTest.cs @@ -10,9 +10,9 @@ namespace Elastic.Apm.AspNetCore.Tests { /// - /// Tests the type. + /// Tests the type. /// - public class ApmMiddlewareExtensionTest + public class ApplicationBuilderExtensionLoggingTest { [Fact] public void UseElasticApmShouldUseAspNetLoggerWhenLoggingIsConfigured() diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreBasicTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreBasicTests.cs index 1ed31ebc4..89ad5335e 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreBasicTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreBasicTests.cs @@ -476,18 +476,18 @@ public async Task FailingPostRequestWithoutConfiguredExceptionPage(bool withDiag } [Fact] - public async Task AspNetCoreErrorDiagnosticsSubscriber_Should_Be_Registered_Only_Once() + public async Task AspNetCoreDiagnosticSubscriber_Should_Be_Registered_Only_Once() { using var agent = CreateAspNetCoreAgent(out _); await using var factory = new WebApplicationFactory(); await using var builder = factory .WithWebHostBuilder(n => n.Configure(app => - app.UseElasticApm(agent, agent.Logger, new AspNetCoreErrorDiagnosticsSubscriber()))); + app.UseElasticApm(agent, agent.Logger, new AspNetCoreDiagnosticSubscriber()))); using var client = builder.CreateClient(); agent.Disposables.Should().NotBeNull(); - agent.SubscribedListeners().Should().Contain(typeof(AspNetCoreErrorDiagnosticListener)); + agent.SubscribedListeners().Should().Contain(typeof(AspNetCoreDiagnosticListener)); } /// diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreDiagnosticListenerTest.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreDiagnosticListenerTest.cs index d0dd798e0..a8a10974e 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreDiagnosticListenerTest.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreDiagnosticListenerTest.cs @@ -7,6 +7,7 @@ using System.Net.Http; using System.Text; using System.Threading.Tasks; +using Elastic.Apm.Api; using Elastic.Apm.AspNetCore.DiagnosticListener; using Elastic.Apm.Config; using Elastic.Apm.DiagnosticSource; @@ -72,6 +73,14 @@ public async Task TestErrorInAspNetCore(bool useOnlyDiagnosticSource) var context = error?.Context; context?.Request.Url.Full.Should().Be("http://localhost/Home/TriggerError"); context?.Request.Method.Should().Be(HttpMethod.Get.Method); + + var transaction = capturedPayload.FirstTransaction; + transaction.Outcome.Should().Be(Outcome.Failure); + + var transactionContext = capturedPayload.FirstTransaction.Context; + transactionContext.Should().NotBeNull(); + transactionContext.Response.Should().NotBeNull(); + transactionContext.Response.StatusCode.Should().Be(500); } } diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/BodyCapturingTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/BodyCapturingTests.cs index bca63a5f6..c70b0a609 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/BodyCapturingTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/BodyCapturingTests.cs @@ -131,6 +131,7 @@ public async Task Body_Capture_Should_Not_Error_When_Large_File() response.IsSuccessStatusCode.Should().BeTrue(); var responseContent = int.Parse(await response.Content.ReadAsStringAsync()); responseContent.Should().Be(repeat * count); + sutEnv.MockPayloadSender.WaitForTransactions(TimeSpan.FromSeconds(5)); sutEnv.MockPayloadSender.Transactions.Should().HaveCount(1); sutEnv.MockPayloadSender.Errors.Should().BeEmpty(); } @@ -461,14 +462,23 @@ private class SutEnv private const string UrlForTestApp = "http://localhost:5903"; internal readonly ApmAgent Agent; internal readonly HttpClient HttpClient; - internal readonly MockPayloadSender MockPayloadSender; + private readonly MockPayloadSender _mockPayloadSender; + + public MockPayloadSender MockPayloadSender + { + get + { + _mockPayloadSender.WaitForAny(TimeSpan.FromSeconds(5)); + return _mockPayloadSender; + } + } private readonly CancellationTokenSource _cancellationTokenSource = new(); private readonly Task _taskForSampleApp; internal SutEnv(IConfiguration startConfiguration, IApmLogger logger, ITestOutputHelper output) { - MockPayloadSender = new MockPayloadSender(logger); + _mockPayloadSender = new MockPayloadSender(logger); Agent = new ApmAgent(new TestAgentComponents(logger, startConfiguration, MockPayloadSender)); _taskForSampleApp = Program.CreateWebHostBuilder(null) diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/CaptureUserTest.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/CaptureUserTest.cs index 9519eca39..20490b7d7 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/CaptureUserTest.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/CaptureUserTest.cs @@ -106,35 +106,14 @@ public async Task RegisterAndLogInUser() transactionWithUser .Context.User.UserName.Should() .Be(userName); - } - - /// - /// A unit test that directly calls InvokeAsync on the . - /// It tests for OpenID claims. - /// It creates a with email and sub claims on it (those are OpenID standard) - /// and make sure that the agent captured the userid and the email address of the user. - /// - [Fact] - public async Task OpenIdClaimsTest() - { - const string mail = "my@mail.com"; - const string sub = "123-456"; - var payloadSender = new MockPayloadSender(); - using (var agent = new ApmAgent(new TestAgentComponents(payloadSender: payloadSender))) - { - var context = new DefaultHttpContext - { - User = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("email", mail), new Claim("sub", sub) }, "someAuthTypeName")) - }; - - var middleware = new ApmMiddleware(async _ => { await Task.Delay(1); }, agent.TracerInternal, agent); - - await middleware.InvokeAsync(context); - } + transactionWithUser + .Context.User.Email.Should() + .StartWith(userName); - payloadSender.FirstTransaction.Context.User.Email.Should().Be(mail); - payloadSender.FirstTransaction.Context.User.Id.Should().Be(sub); + transactionWithUser + .Context.User.Id.Should() + .NotBeEmpty(); } public void Dispose() => _agent?.Dispose(); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs index 81f64f4d2..bd07275d6 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs @@ -99,7 +99,7 @@ internal static HttpClient GetClientWithoutDiagnosticListeners(ApmAgent agent { n.Configure(app => { - app.UseMiddleware(agent.Tracer, agent); + //app.UseMiddleware(agent.Tracer, agent); app.UseDeveloperExceptionPage(); app.UseHsts(); diff --git a/test/integrations/applications/SampleAspNetCoreApp/Controllers/AccountController.cs b/test/integrations/applications/SampleAspNetCoreApp/Controllers/AccountController.cs index fb0d8ba1b..21ffa5f80 100644 --- a/test/integrations/applications/SampleAspNetCoreApp/Controllers/AccountController.cs +++ b/test/integrations/applications/SampleAspNetCoreApp/Controllers/AccountController.cs @@ -23,7 +23,6 @@ public AccountController(UserManager userManager, SignInManager LoginUser([FromForm] string userName, [FromForm] string password) { var res = await _signInManager.PasswordSignInAsync(userName, password, true, false); - if (res.Succeeded) return Redirect("/Home/Index"); @@ -35,7 +34,7 @@ public async Task LoginUser([FromForm] string userName, [FromForm [HttpPost] public async Task RegisterUser([FromForm] string userName, [FromForm] string password) { - var newUser = new IdentityUser { UserName = userName }; + var newUser = new IdentityUser { UserName = userName, Email = $"{userName}@test.example" , Id = "123-456"}; var res = await _userManager.CreateAsync(newUser, password); if (res.Succeeded) From 069171fd5441d279429228fba2745a46b7f7ff42 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Wed, 29 Nov 2023 18:24:00 +0100 Subject: [PATCH 2/8] dotnet format --- .../SampleAspNetCoreApp/Controllers/AccountController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integrations/applications/SampleAspNetCoreApp/Controllers/AccountController.cs b/test/integrations/applications/SampleAspNetCoreApp/Controllers/AccountController.cs index 21ffa5f80..6ad8da3b9 100644 --- a/test/integrations/applications/SampleAspNetCoreApp/Controllers/AccountController.cs +++ b/test/integrations/applications/SampleAspNetCoreApp/Controllers/AccountController.cs @@ -34,7 +34,7 @@ public async Task LoginUser([FromForm] string userName, [FromForm [HttpPost] public async Task RegisterUser([FromForm] string userName, [FromForm] string password) { - var newUser = new IdentityUser { UserName = userName, Email = $"{userName}@test.example" , Id = "123-456"}; + var newUser = new IdentityUser { UserName = userName, Email = $"{userName}@test.example", Id = "123-456" }; var res = await _userManager.CreateAsync(newUser, password); if (res.Succeeded) From 187f6f7c60b0fbc6ab64a4b103f863c7116038f3 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 30 Nov 2023 15:38:49 +0100 Subject: [PATCH 3/8] update tests to more explicitly wait for payloadsender --- .../BaggageAspNetCoreTests.cs | 3 ++ .../DistributedTracingAspNetCoreTests.cs | 34 +++++++++++++++++++ .../FailedRequestTests.cs | 13 +++++-- .../MultiApplicationTestBase.cs | 22 ++++++++---- 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/BaggageAspNetCoreTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/BaggageAspNetCoreTests.cs index f668e462b..1f431fa77 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/BaggageAspNetCoreTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/BaggageAspNetCoreTests.cs @@ -9,6 +9,7 @@ using Elastic.Apm.Model; using FluentAssertions; using Xunit; +using Xunit.Abstractions; namespace Elastic.Apm.AspNetCore.Tests; @@ -16,6 +17,8 @@ namespace Elastic.Apm.AspNetCore.Tests; public class BaggageAspNetCoreTests : MultiApplicationTestBase { + public BaggageAspNetCoreTests(ITestOutputHelper output) : base(output) { } + private void ValidateOtelAttribute(Transaction transaction, string key, string value) => transaction.Otel.Attributes.Should().Contain(new KeyValuePair($"baggage.{key}", value)); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/DistributedTracingAspNetCoreTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/DistributedTracingAspNetCoreTests.cs index 494c94c99..ab3400335 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/DistributedTracingAspNetCoreTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/DistributedTracingAspNetCoreTests.cs @@ -3,6 +3,7 @@ // Elasticsearch B.V 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.Diagnostics; using System.Linq; using System.Net.Http; @@ -18,6 +19,8 @@ namespace Elastic.Apm.AspNetCore.Tests [Collection("DiagnosticListenerTest")] public class DistributedTracingAspNetCoreTests : MultiApplicationTestBase { + public DistributedTracingAspNetCoreTests(ITestOutputHelper output) : base(output) { } + /// /// Distributed tracing integration test. /// It starts with 1 agent and with another agent. @@ -112,6 +115,8 @@ public async Task DistributedTraceAcross2ServicesWithTraceState() var res = await client.GetAsync("http://localhost:5901/Home/DistributedTracingMiniSample"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.FirstTransaction.IsSampled.Should().BeTrue(); _payloadSender2.FirstTransaction.IsSampled.Should().BeTrue(); @@ -136,6 +141,8 @@ public async Task PreferW3CTraceHeaderOverElasticTraceHeader() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.FirstTransaction.TraceId.Should().Be(expectedTraceId); _payloadSender1.FirstTransaction.ParentId.Should().Be(expectedParentId); } @@ -158,6 +165,8 @@ public async Task TraceContextIgnoreSampledFalse_WithNoTraceState() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + // Assert that the transaction is sampled and the traceparent header was ignored _payloadSender1.FirstTransaction.IsSampled.Should().BeTrue(); @@ -185,6 +194,8 @@ public async Task TraceContextIgnoreSampledFalse_WithEsTraceState_NotSampled() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + // Assert that the transaction is not sampled, so the traceparent header was not ignored _payloadSender1.FirstTransaction.IsSampled.Should().BeFalse(); @@ -212,6 +223,8 @@ public async Task TraceContextIgnoreSampledFalse_WithNonEsTraceState_NotSampled( var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + // Assert that the transaction is sampled and the traceparent header was ignored _payloadSender1.FirstTransaction.IsSampled.Should().BeTrue(); @@ -240,6 +253,8 @@ public async Task TraceContextIgnoreSampledFalse_NotSet_WithNonEsTraceState_NotS var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + // Assert that the transaction is not sampled and the traceparent header was not ignored _payloadSender1.FirstTransaction.IsSampled.Should().BeFalse(); @@ -262,6 +277,8 @@ public async Task TraceContinuationStrategyContinue() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.FirstTransaction.IsSampled.Should().BeTrue(); _payloadSender1.FirstTransaction.ParentId.Should().Be("b7ad6b7169203331"); _payloadSender1.FirstTransaction.TraceId.Should().Be("0af7651916cd43dd8448eb211c80319c"); @@ -284,6 +301,8 @@ public async Task TraceContinuationStrategyDefault() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.FirstTransaction.IsSampled.Should().BeTrue(); _payloadSender1.FirstTransaction.ParentId.Should().Be("b7ad6b7169203331"); _payloadSender1.FirstTransaction.TraceId.Should().Be("0af7651916cd43dd8448eb211c80319c"); @@ -306,6 +325,8 @@ public async Task TraceContinuationStrategyRestartExternalWithNoEsTag() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.FirstTransaction.IsSampled.Should().BeTrue(); _payloadSender1.FirstTransaction.ParentId.Should().NotBe("b7ad6b7169203331"); _payloadSender1.FirstTransaction.TraceId.Should().NotBe("0af7651916cd43dd8448eb211c80319c"); @@ -330,6 +351,8 @@ public async Task TraceContinuationStrategyRestartExternalWithEsTag() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.FirstTransaction.IsSampled.Should().BeTrue(); _payloadSender1.FirstTransaction.ParentId.Should().Be("b7ad6b7169203331"); _payloadSender1.FirstTransaction.TraceId.Should().Be("0af7651916cd43dd8448eb211c80319c"); @@ -352,6 +375,8 @@ public async Task TraceContinuationStrategyRestartWithEsTag() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.FirstTransaction.IsSampled.Should().BeTrue(); _payloadSender1.FirstTransaction.ParentId.Should().NotBe("b7ad6b7169203331"); _payloadSender1.FirstTransaction.TraceId.Should().NotBe("0af7651916cd43dd8448eb211c80319c"); @@ -376,6 +401,8 @@ public async Task TraceContinuationStrategyRestartWithoutEsTag() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.FirstTransaction.IsSampled.Should().BeTrue(); _payloadSender1.FirstTransaction.ParentId.Should().NotBe("b7ad6b7169203331"); _payloadSender1.FirstTransaction.TraceId.Should().NotBe("0af7651916cd43dd8448eb211c80319c"); @@ -401,6 +428,8 @@ public async Task TraceContinuationStrategyRestartExternalAndNoTraceParent() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.Transactions.Should().NotBeNullOrEmpty(); } @@ -423,6 +452,8 @@ public async Task TraceContinuationStrategyRestartExternalAndNoTraceState() var res = await client.GetAsync("http://localhost:5901/Home/Index"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.Transactions.Should().NotBeNullOrEmpty(); // The trace is restarted (due to `traceContinuationStrategy=restart_external`), so assert that the traceId and @@ -454,6 +485,9 @@ private async Task ExecuteAndCheckDistributedCall(bool startActivityBeforeHttpCa var res = await client.GetAsync("http://localhost:5901/Home/DistributedTracingMiniSample"); res.IsSuccessStatusCode.Should().BeTrue(); + _payloadSender1.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender2.WaitForTransactions(TimeSpan.FromSeconds(10)); + _payloadSender1.Transactions.Count.Should().Be(1); _payloadSender2.Transactions.Count.Should().Be(1); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/FailedRequestTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/FailedRequestTests.cs index 159e7b3d4..22430179f 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/FailedRequestTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/FailedRequestTests.cs @@ -9,7 +9,9 @@ using Elastic.Apm.Api; using Elastic.Apm.DiagnosticSource; using Elastic.Apm.EntityFrameworkCore; +using Elastic.Apm.Logging; using Elastic.Apm.Tests.Utilities; +using Elastic.Apm.Tests.Utilities.XUnit; using FluentAssertions; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.DependencyInjection; @@ -24,7 +26,7 @@ public class FailedRequestTests : IAsyncLifetime { private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); - private readonly MockPayloadSender _payloadSender1 = new MockPayloadSender(); + private MockPayloadSender _payloadSender1; private ApmAgent _agent1; private Task _taskForApp1; @@ -34,7 +36,13 @@ public class FailedRequestTests : IAsyncLifetime public Task InitializeAsync() { - _agent1 = new ApmAgent(new TestAgentComponents(payloadSender: _payloadSender1, configuration: new MockConfiguration(exitSpanMinDuration: "0"))); + var logger = new XUnitLogger(LogLevel.Trace, _output); + _payloadSender1 = new MockPayloadSender(logger); + _agent1 = new ApmAgent(new TestAgentComponents( + payloadSender: _payloadSender1, + configuration: new MockConfiguration(exitSpanMinDuration: "0", flushInterval: "0"), + logger: logger + )); _taskForApp1 = Program.CreateWebHostBuilder(null) .ConfigureLogging(logging => logging.AddXunit(_output)) @@ -71,6 +79,7 @@ public async Task DistributedTraceAcross2Service() var client = new HttpClient(); var res = await client.GetAsync("http://localhost:5901/Home/TriggerError"); res.IsSuccessStatusCode.Should().BeFalse(); + _payloadSender1.WaitForAny(); _payloadSender1.Transactions.Count.Should().Be(1); _payloadSender1.FirstTransaction.Should().NotBeNull(); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/MultiApplicationTestBase.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/MultiApplicationTestBase.cs index 105b6036e..6bcca0f95 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/MultiApplicationTestBase.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/MultiApplicationTestBase.cs @@ -8,11 +8,14 @@ using System.Threading.Tasks; using Elastic.Apm.DiagnosticSource; using Elastic.Apm.EntityFrameworkCore; +using Elastic.Apm.Logging; using Elastic.Apm.Tests.Utilities; +using Elastic.Apm.Tests.Utilities.XUnit; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.DependencyInjection; using SampleAspNetCoreApp; using Xunit; +using Xunit.Abstractions; namespace Elastic.Apm.AspNetCore.Tests; @@ -21,8 +24,9 @@ namespace Elastic.Apm.AspNetCore.Tests; /// public abstract class MultiApplicationTestBase : IAsyncLifetime { - internal readonly MockPayloadSender _payloadSender1 = new(); - internal readonly MockPayloadSender _payloadSender2 = new(); + private readonly ITestOutputHelper _output; + internal MockPayloadSender _payloadSender1; + internal MockPayloadSender _payloadSender2; private readonly CancellationTokenSource _cancellationTokenSource = new(); internal ApmAgent _agent1; internal ApmAgent _agent2; @@ -30,12 +34,18 @@ public abstract class MultiApplicationTestBase : IAsyncLifetime private Task _taskForApp1; private Task _taskForApp2; + public MultiApplicationTestBase(ITestOutputHelper output) => _output = output; + public Task InitializeAsync() { - _agent1 = new ApmAgent(new TestAgentComponents(payloadSender: _payloadSender1, - configuration: new MockConfiguration(exitSpanMinDuration: "0"))); - _agent2 = new ApmAgent(new TestAgentComponents(payloadSender: _payloadSender2, - configuration: new MockConfiguration(exitSpanMinDuration: "0"))); + var logger1 = new XUnitLogger(LogLevel.Trace, _output, "Sender 1"); + _payloadSender1 = new MockPayloadSender(logger1); + var logger2 = new XUnitLogger(LogLevel.Trace, _output, "Sender 2"); + _payloadSender2 = new MockPayloadSender(logger2); + + var configuration = new MockConfiguration(exitSpanMinDuration: "0", flushInterval: "3s"); + _agent1 = new ApmAgent(new TestAgentComponents(payloadSender: _payloadSender1, configuration: configuration, logger: logger1)); + _agent2 = new ApmAgent(new TestAgentComponents(payloadSender: _payloadSender2, configuration: configuration, logger: logger2)); _taskForApp1 = Program.CreateWebHostBuilder(null) .ConfigureServices(services => From 7a2b3a7f4a6e7265afadacb848e9ca71a75719ac Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Fri, 1 Dec 2023 10:55:44 +0100 Subject: [PATCH 4/8] update tests --- .../Extensions/RequestExtensions.cs | 17 ++-- .../XUnit/DisabledTestFact.cs | 11 +++ .../XUnit/MemberData.cs | 2 +- .../ConfigTests.cs | 10 +- .../ApmConfigurationTests.cs | 2 +- .../AspNetCoreBasicTests.cs | 71 ++++++-------- .../AspNetCoreDiagnosticListenerTest.cs | 14 ++- .../DiagnosticListenerTests.cs | 24 ++++- .../Elastic.Apm.AspNetCore.Tests.csproj | 2 +- .../Elastic.Apm.AspNetCore.Tests/Helper.cs | 27 ++---- .../SanitizeFieldNamesTests.cs | 95 +++++++++---------- .../TraceIdWithActivityTests.cs | 2 +- .../TransactionIgnoreUrlsTest.cs | 19 ++-- .../TransactionNameTests.cs | 66 ++++++------- 14 files changed, 173 insertions(+), 189 deletions(-) diff --git a/src/integrations/Elastic.Apm.AspNetCore/Extensions/RequestExtensions.cs b/src/integrations/Elastic.Apm.AspNetCore/Extensions/RequestExtensions.cs index 483e94231..f507638ad 100644 --- a/src/integrations/Elastic.Apm.AspNetCore/Extensions/RequestExtensions.cs +++ b/src/integrations/Elastic.Apm.AspNetCore/Extensions/RequestExtensions.cs @@ -14,6 +14,7 @@ internal static class HttpRequestExtensions internal static string ExtractRequestBody(this HttpRequest request, IConfiguration configuration, out bool longerThanMaxLength) { //Ensure the request Microsoft.AspNetCore.Http.HttpRequest.Body can be read multiple times +#pragma warning disable CS0162 // Unreachable code detected request.EnableBuffering(); if (request.HasFormContentType) @@ -21,16 +22,14 @@ internal static string ExtractRequestBody(this HttpRequest request, IConfigurati var form = new AspNetCoreHttpForm(request.Form); return form.AsSanitizedString(configuration, out longerThanMaxLength); } - else - { - // allow synchronous reading of the request stream, which is false by default from 3.0 onwards. - // Reading must be synchronous as it can happen within a synchronous diagnostic listener method - var bodyControlFeature = request.HttpContext.Features.Get(); - if (bodyControlFeature != null) - bodyControlFeature.AllowSynchronousIO = true; + // allow synchronous reading of the request stream, which is false by default from 3.0 onwards. + // Reading must be synchronous as it can happen within a synchronous diagnostic listener method + var bodyControlFeature = request.HttpContext.Features.Get(); + if (bodyControlFeature != null) + bodyControlFeature.AllowSynchronousIO = true; - return RequestBodyStreamHelper.ToString(request.Body, out longerThanMaxLength); - } + return RequestBodyStreamHelper.ToString(request.Body, out longerThanMaxLength); +#pragma warning restore CS0162 // Unreachable code detected } } } diff --git a/test/Elastic.Apm.Tests.Utilities/XUnit/DisabledTestFact.cs b/test/Elastic.Apm.Tests.Utilities/XUnit/DisabledTestFact.cs index 0b41a5508..24f2355cd 100644 --- a/test/Elastic.Apm.Tests.Utilities/XUnit/DisabledTestFact.cs +++ b/test/Elastic.Apm.Tests.Utilities/XUnit/DisabledTestFact.cs @@ -3,6 +3,7 @@ // Elasticsearch B.V 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 Xunit; namespace Elastic.Apm.Tests.Utilities.XUnit; @@ -16,3 +17,13 @@ public DisabledTestFact(string reason, string issueLink = null) Skip += $", issue link: {issueLink}"; } } + +public sealed class FactRequiresMvcTestingFix : FactAttribute +{ + public FactRequiresMvcTestingFix() + { + if (Environment.Version.Major < 7) return; + Skip = $"This Test is disabled on .NET 7 until https://github.com/dotnet/aspnetcore/issues/45233"; + } +} + diff --git a/test/Elastic.Apm.Tests.Utilities/XUnit/MemberData.cs b/test/Elastic.Apm.Tests.Utilities/XUnit/MemberData.cs index a8f6816eb..f7a07e897 100644 --- a/test/Elastic.Apm.Tests.Utilities/XUnit/MemberData.cs +++ b/test/Elastic.Apm.Tests.Utilities/XUnit/MemberData.cs @@ -13,7 +13,7 @@ public static IEnumerable TestWithDiagnosticSourceOnly() yield return new object[] { false }; // // Skip "DiagnosticSourceOnly" tests on .NET 7 - // until https://github.com/dotnet/aspnetcore/issues/45233 is resolved. + // until is resolved. // if (Environment.Version.Major < 7) yield return new object[] { true }; diff --git a/test/integrations/Elastic.Apm.AspNetCore.Static.Tests/ConfigTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Static.Tests/ConfigTests.cs index e1f601e6c..530881891 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Static.Tests/ConfigTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Static.Tests/ConfigTests.cs @@ -35,9 +35,8 @@ public ConfigTests(WebApplicationFactory factory, ITestOutputHelper xUn /// Tests for: https://github.com/elastic/apm-agent-dotnet/issues/1077 /// /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task AgentDisabledInAppConfig(bool withDiagnosticSourceOnly) + [Fact] + public async Task AgentDisabledInAppConfig() { var defaultServerUrlConnectionMade = false; @@ -65,9 +64,8 @@ public async Task AgentDisabledInAppConfig(bool withDiagnosticSourceOnly) new NoopLogger(), new RuntimeConfigurationSnapshot(configReader))); - var client = Helper.ConfigureHttpClient(true, withDiagnosticSourceOnly, agent, _factory); - if (withDiagnosticSourceOnly) - Agent.Setup(agent); + var client = Helper.ConfigureHttpClient(true, agent, _factory); + Agent.Setup(agent); var response = await client.GetAsync("/Home/StartTransactionWithAgentApi"); response.IsSuccessStatusCode.Should().BeTrue(); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/ApmConfigurationTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/ApmConfigurationTests.cs index 5017d316e..2c7c79820 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/ApmConfigurationTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/ApmConfigurationTests.cs @@ -244,7 +244,7 @@ public ApmConfigurationIntegrationTests(WebApplicationFactory factory) _agent = new ApmAgent( new AgentComponents(payloadSender: capturedPayload, configurationReader: config, logger: _logger)); - _client = Helper.GetClient(_agent, _factory, true); + _client = Helper.GetClient(_agent, _factory); } /// diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreBasicTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreBasicTests.cs index 89ad5335e..628247eba 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreBasicTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreBasicTests.cs @@ -60,13 +60,12 @@ private ApmAgent CreateAspNetCoreAgent(out MockPayloadSender payloadSender) /// /// Simulates an HTTP GET call to /home/simplePage and asserts on what the agent should send to the server /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task HomeSimplePageTransactionTest(bool withDiagnosticSourceOnly) + [Fact] + public async Task HomeSimplePageTransactionTest() { using var agent = CreateAspNetCoreAgent(out var payloadSender); await using var factory = new WebApplicationFactory(); - using var client = Helper.ConfigureHttpClient(true, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(true, agent, factory); var headerKey = "X-Additional-Header"; var headerValue = "For-Elastic-Apm-Agent"; @@ -149,11 +148,9 @@ public async Task HomeSimplePageTransactionTest(bool withDiagnosticSourceOnly) /// /// Creates an agent with Enabled=false and makes sure the agent does not capture anything. /// - /// /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task HomeIndexTransactionWithEnabledFalse(bool withDiagnosticSourceOnly) + [Fact] + public async Task HomeIndexTransactionWithEnabledFalse() { var payloadSender = new MockPayloadSender(); await using var factory = new WebApplicationFactory(); @@ -161,7 +158,7 @@ public async Task HomeIndexTransactionWithEnabledFalse(bool withDiagnosticSource _logger, new MockConfiguration(_logger, enabled: "false", exitSpanMinDuration: "0"), payloadSender)); - using var client = Helper.ConfigureHttpClient(true, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(true, agent, factory); var response = await client.GetAsync("/Home/Index"); @@ -173,16 +170,15 @@ public async Task HomeIndexTransactionWithEnabledFalse(bool withDiagnosticSource payloadSender.Errors.Should().BeNullOrEmpty(); } - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task HomeIndexTransactionWithToggleRecording(bool withDiagnosticSourceOnly) + [Fact] + public async Task HomeIndexTransactionWithToggleRecording() { var payloadSender = new MockPayloadSender(); await using var factory = new WebApplicationFactory(); using var agent = new ApmAgent(new TestAgentComponents( _logger, new MockConfiguration(recording: "false", exitSpanMinDuration: "0"), payloadSender)); - using var client = Helper.ConfigureHttpClient(true, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(true, agent, factory); var response = await client.GetAsync("/Home/Index"); @@ -214,13 +210,12 @@ public async Task HomeIndexTransactionWithToggleRecording(bool withDiagnosticSou /// Simulates an HTTP POST call to /home/simplePage and asserts on what the agent should send to the server /// to test the 'CaptureBody' configuration option /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task HomeSimplePagePostTransactionTest(bool withDiagnosticSourceOnly) + [FactRequiresMvcTestingFix] + public async Task HomeSimplePagePostTransactionTest() { using var agent = CreateAspNetCoreAgent(out var payloadSender); await using var factory = new WebApplicationFactory(); - using var client = Helper.ConfigureHttpClient(true, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(true, agent, factory); var headerKey = "X-Additional-Header"; var headerValue = "For-Elastic-Apm-Agent"; client.DefaultRequestHeaders.Add(headerKey, headerValue); @@ -310,13 +305,12 @@ public async Task HomeSimplePagePostTransactionTest(bool withDiagnosticSourceOnl /// Simulates an HTTP GET call to /home/index and asserts that the agent captures spans. /// Prerequisite: The /home/index has to generate spans (which should be the case). /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task HomeIndexSpanTest(bool withDiagnosticSourceOnly) + [Fact] + public async Task HomeIndexSpanTest() { using var agent = CreateAspNetCoreAgent(out var payloadSender); await using var factory = new WebApplicationFactory(); - using var client = Helper.ConfigureHttpClient(true, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(true, agent, factory); var response = await client.GetAsync("/Home/Index"); response.IsSuccessStatusCode.Should().BeTrue(); @@ -331,13 +325,12 @@ public async Task HomeIndexSpanTest(bool withDiagnosticSourceOnly) /// Prerequisite: The /home/index has to generate spans (which should be the case). /// It also assumes that /home/index makes a requrst to github.com /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task HomeIndexDestinationTest(bool withDiagnosticSourceOnly) + [Fact] + public async Task HomeIndexDestinationTest() { using var agent = CreateAspNetCoreAgent(out var payloadSender); await using var factory = new WebApplicationFactory(); - using var client = Helper.ConfigureHttpClient(true, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(true, agent, factory); var response = await client.GetAsync("/Home/Index"); response.IsSuccessStatusCode.Should().BeTrue(); @@ -357,13 +350,12 @@ public async Task HomeIndexDestinationTest(bool withDiagnosticSourceOnly) /// Simulates an HTTP GET call to /Home/Index?captureControllerActionAsSpan=true /// and asserts that all automatically captured spans are children of the span for controller's action. /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task HomeIndexAutoCapturedSpansAreChildrenOfControllerActionAsSpan(bool withDiagnosticSourceOnly) + [Fact] + public async Task HomeIndexAutoCapturedSpansAreChildrenOfControllerActionAsSpan() { using var agent = CreateAspNetCoreAgent(out var payloadSender); await using var factory = new WebApplicationFactory(); - using var client = Helper.ConfigureHttpClient(true, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(true, agent, factory); var response = await client.GetAsync("/Home/Index?captureControllerActionAsSpan=true"); response.IsSuccessStatusCode.Should().BeTrue(); @@ -398,13 +390,12 @@ public async Task HomeIndexAutoCapturedSpansAreChildrenOfControllerActionAsSpan( /// With other words: there is no error page with an exception handler configured in the ASP.NET Core pipeline. /// Makes sure that we still capture the failed request. /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task FailingRequestWithoutConfiguredExceptionPage(bool withDiagnosticSourceOnly) + [Fact] + public async Task FailingRequestWithoutConfiguredExceptionPage() { using var agent = CreateAspNetCoreAgent(out var payloadSender); await using var factory = new WebApplicationFactory(); - using var client = Helper.ConfigureHttpClient(false, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(false, agent, factory); Func act = async () => await client.GetAsync("Home/TriggerError"); await act.Should().ThrowAsync(); @@ -436,13 +427,12 @@ public async Task FailingRequestWithoutConfiguredExceptionPage(bool withDiagnost /// With other words: there is no error page with an exception handler configured in the ASP.NET Core pipeline. /// Makes sure that we still capture the failed request along with the request body /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task FailingPostRequestWithoutConfiguredExceptionPage(bool withDiagnosticSourceOnly) + [FactRequiresMvcTestingFix] + public async Task FailingPostRequestWithoutConfiguredExceptionPage() { using var agent = CreateAspNetCoreAgent(out var payloadSender); await using var factory = new WebApplicationFactory(); - using var client = Helper.ConfigureHttpClient(false, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(false, agent, factory); client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); @@ -494,13 +484,12 @@ public async Task AspNetCoreDiagnosticSubscriber_Should_Be_Registered_Only_Once( /// An HTTP call to an action method which manually sets . /// Makes sure auto instrumentation does not overwrite the outcome. /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task ManualTransactionOutcomeTest(bool withDiagnosticSourceOnly) + [Fact] + public async Task ManualTransactionOutcomeTest() { await using var factory = new WebApplicationFactory(); using var agent = CreateAspNetCoreAgent(out var payloadSender); - using var client = Helper.ConfigureHttpClient(true, withDiagnosticSourceOnly, agent, factory); + using var client = Helper.ConfigureHttpClient(true, agent, factory); await client.GetAsync("/Home/SampleWithManuallySettingOutcome"); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreDiagnosticListenerTest.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreDiagnosticListenerTest.cs index a8a10974e..443d6955d 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreDiagnosticListenerTest.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/AspNetCoreDiagnosticListenerTest.cs @@ -39,14 +39,13 @@ public class AspNetCoreDiagnosticListenerTest : IClassFixture /// The error in ASP net core. - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task TestErrorInAspNetCore(bool useOnlyDiagnosticSource) + [Fact] + public async Task TestErrorInAspNetCore() { var capturedPayload = new MockPayloadSender(); using (var agent = new ApmAgent(new TestAgentComponents(payloadSender: capturedPayload, configuration: new MockConfiguration(exitSpanMinDuration: "0")))) { - var client = Helper.GetClient(agent, _factory, useOnlyDiagnosticSource); + var client = Helper.GetClient(agent, _factory); try { @@ -90,9 +89,8 @@ public async Task TestErrorInAspNetCore(bool useOnlyDiagnosticSource) /// retrieved from the HttpRequest /// /// The error in ASP net core. - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task TestJsonBodyRetrievalOnRequestFailureInAspNetCore(bool useOnlyDiagnosticSource) + [Fact] + public async Task TestJsonBodyRetrievalOnRequestFailureInAspNetCore() { var capturedPayload = new MockPayloadSender(); using (var agent = new ApmAgent(new TestAgentComponents(configuration: new MockConfiguration( @@ -101,7 +99,7 @@ public async Task TestJsonBodyRetrievalOnRequestFailureInAspNetCore(bool useOnly captureBodyContentTypes: ConfigConsts.DefaultValues.CaptureBodyContentTypes), payloadSender: capturedPayload))) { - var client = Helper.GetClient(agent, _factory, useOnlyDiagnosticSource); + var client = Helper.GetClient(agent, _factory); var body = "{\"id\" : \"1\"}"; await client.PostAsync("api/Home/PostError", new StringContent(body, Encoding.UTF8, "application/json")); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/DiagnosticListenerTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/DiagnosticListenerTests.cs index e7ff435c1..af833bc18 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/DiagnosticListenerTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/DiagnosticListenerTests.cs @@ -7,11 +7,14 @@ using System.Threading.Tasks; using Elastic.Apm.AspNetCore.DiagnosticListener; using Elastic.Apm.EntityFrameworkCore; +using Elastic.Apm.Logging; using Elastic.Apm.Tests.Utilities; +using Elastic.Apm.Tests.Utilities.XUnit; using FluentAssertions; using Microsoft.AspNetCore.Mvc.Testing; using SampleAspNetCoreApp; using Xunit; +using Xunit.Abstractions; namespace Elastic.Apm.AspNetCore.Tests { @@ -26,14 +29,21 @@ public class DiagnosticListenerTests : IClassFixture factory) + public DiagnosticListenerTests(WebApplicationFactory factory, ITestOutputHelper output) { - _agent = new ApmAgent(new TestAgentComponents(configuration: new MockConfiguration(exitSpanMinDuration: "0"))); - _capturedPayload = _agent.PayloadSender as MockPayloadSender; + var logger = new XUnitLogger(LogLevel.Trace, output); + _capturedPayload = new MockPayloadSender(logger); + _agent = new ApmAgent( + new TestAgentComponents( + configuration: new MockConfiguration(exitSpanMinDuration: "0", flushInterval: "0"), + payloadSender: _capturedPayload, + logger: logger + ) + ); //This registers the middleware without activating any listeners, //so no error capturing and no EFCore listener. - _client = Helper.GetClientWithoutDiagnosticListeners(_agent, factory); + _client = Helper.GetClientWithoutDiagnosticListeners(factory); } /// @@ -65,6 +75,8 @@ public async Task SubscribeAndUnsubscribeAspNetCoreDiagnosticListener() await _client.GetAsync("/Home/TriggerError"); + _capturedPayload.WaitForTransactions(TimeSpan.FromSeconds(10)); + _capturedPayload.Transactions.Should().ContainSingle(); _capturedPayload.Errors.Should().BeEmpty(); } @@ -83,6 +95,8 @@ public async Task SubscribeAndUnsubscribeEfCoreDiagnosticListener() { await _client.GetAsync("/Home/Index"); + _capturedPayload.WaitForTransactions(TimeSpan.FromSeconds(10)); + _capturedPayload.Transactions.Should().ContainSingle(); _capturedPayload.SpansOnFirstTransaction.Should() @@ -94,6 +108,8 @@ public async Task SubscribeAndUnsubscribeEfCoreDiagnosticListener() await _client.GetAsync("/Home/Index"); + _capturedPayload.WaitForTransactions(TimeSpan.FromSeconds(10)); + _capturedPayload.Transactions.Should().ContainSingle(); _capturedPayload.SpansOnFirstTransaction.Should().BeEmpty(); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/Elastic.Apm.AspNetCore.Tests.csproj b/test/integrations/Elastic.Apm.AspNetCore.Tests/Elastic.Apm.AspNetCore.Tests.csproj index 8c500301a..e677b027b 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/Elastic.Apm.AspNetCore.Tests.csproj +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/Elastic.Apm.AspNetCore.Tests.csproj @@ -12,7 +12,7 @@ - + diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs index bd07275d6..cde7d0c4f 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs @@ -17,7 +17,7 @@ namespace Elastic.Apm.AspNetCore.Tests { public static class Helper { - internal static HttpClient ConfigureHttpClient(bool createDefaultClient, bool useOnlyDiagnosticSource, ApmAgent agent, + internal static HttpClient ConfigureHttpClient(bool createDefaultClient, ApmAgent agent, WebApplicationFactory factory ) where T : class { @@ -25,34 +25,29 @@ WebApplicationFactory factory if (createDefaultClient) { #pragma warning disable IDE0022 // Use expression body for methods - client = GetClient(agent, factory, useOnlyDiagnosticSource); + client = GetClient(agent, factory); #pragma warning restore IDE0022 // Use expression body for methods #if NETCOREAPP3_0 || NETCOREAPP3_1 client.DefaultRequestVersion = new Version(2, 0); #endif } else - client = GetClientWithoutExceptionPage(agent, factory, useOnlyDiagnosticSource); + client = GetClientWithoutExceptionPage(agent, factory); return client; } - private static IDiagnosticsSubscriber[] DiagnosticsOnlyListeners => - new IDiagnosticsSubscriber[] { new AspNetCoreDiagnosticSubscriber(), new HttpDiagnosticsSubscriber(), new EfCoreDiagnosticsSubscriber() }; private static IDiagnosticsSubscriber[] UseApmListeners => new IDiagnosticsSubscriber[] { new HttpDiagnosticsSubscriber(), new EfCoreDiagnosticsSubscriber() }; - internal static HttpClient GetClient(ApmAgent agent, WebApplicationFactory factory, bool useOnlyDiagnosticSource) where T : class + internal static HttpClient GetClient(ApmAgent agent, WebApplicationFactory factory) where T : class { var builder = factory .WithWebHostBuilder(n => { n.Configure(app => { - if (useOnlyDiagnosticSource) - agent.Subscribe(DiagnosticsOnlyListeners); - else - app.UseElasticApm(agent, agent.Logger, UseApmListeners); + app.UseElasticApm(agent, agent.Logger, UseApmListeners); app.UseDeveloperExceptionPage(); @@ -69,7 +64,7 @@ internal static HttpClient GetClient(ApmAgent agent, WebApplicationFactory return builder.CreateClient(); } - internal static HttpClient GetClientWithoutExceptionPage(ApmAgent agent, WebApplicationFactory factory, bool useOnlyDiagnosticSource) + internal static HttpClient GetClientWithoutExceptionPage(ApmAgent agent, WebApplicationFactory factory) where T : class { var builder = factory @@ -77,10 +72,7 @@ internal static HttpClient GetClientWithoutExceptionPage(ApmAgent agent, WebA { n.Configure(app => { - if (useOnlyDiagnosticSource) - agent.Subscribe(DiagnosticsOnlyListeners); - else - app.UseElasticApm(agent, agent.Logger, UseApmListeners); + app.UseElasticApm(agent, agent.Logger, UseApmListeners); Startup.ConfigureRoutingAndMvc(app); }); @@ -94,13 +86,12 @@ internal static HttpClient GetClientWithoutExceptionPage(ApmAgent agent, WebA /// /// Configures the sample app without any diagnostic listener /// - internal static HttpClient GetClientWithoutDiagnosticListeners(ApmAgent agent, WebApplicationFactory factory) where T : class + internal static HttpClient GetClientWithoutDiagnosticListeners(WebApplicationFactory factory) where T : class => factory.WithWebHostBuilder(n => { n.Configure(app => { - //app.UseMiddleware(agent.Tracer, agent); - + app.UseElasticApm(); app.UseDeveloperExceptionPage(); app.UseHsts(); app.UseHttpsRedirection(); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs index 67910bf68..c4b93c5a1 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs @@ -63,15 +63,15 @@ public static IEnumerable GetData(Tests test) testData.Add(new object[] { "mySecurityHeader", new[] { "mySECURITYHeader" } }); break; case Tests.CustomSanitizeFieldNameSettingWithCaseSensitivityWithHeaders: - testData.Add(new object[] { "mysecurityheader", "mySECURITYHeader", true }); - testData.Add(new object[] { "(?-i)mysecurityheader", "mySECURITYHeader", false }); - testData.Add(new object[] { "(?-i)mySECURITYheader", "mySECURITYheader", true }); - testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmySECURITYheader", true }); - testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmysecURITYheader", false }); - testData.Add(new object[] { "(?-i)mySECURITYheader*", "mySECURITYheaderTest", true }); - testData.Add(new object[] { "(?-i)mySECURITYheader*", "mysecURITYheaderTest", false }); - testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmySECURITYheaderTest", true }); - testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmysecURITYheaderTest", false }); + testData.Add(new object[] { "mysecurityheader", "mySECURITYHeader" }); + testData.Add(new object[] { "(?-i)mysecurityheader", "mySECURITYHeader" }); + testData.Add(new object[] { "(?-i)mySECURITYheader", "mySECURITYheader" }); + testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmySECURITYheader" }); + testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmysecURITYheader" }); + testData.Add(new object[] { "(?-i)mySECURITYheader*", "mySECURITYheaderTest" }); + testData.Add(new object[] { "(?-i)mySECURITYheader*", "mysecURITYheaderTest" }); + testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmySECURITYheaderTest" }); + testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmysecURITYheaderTest" }); break; case Tests.DefaultsWithHeaders: testData.Add(new object[] { "password" }); @@ -106,9 +106,9 @@ public static IEnumerable GetData(Tests test) testData.Add(new object[] { "creditcardnumber" }); //*card break; case Tests.DefaultWithRequestBodySingleValueNoError: - testData.Add(new object[] { "password", true }); - testData.Add(new object[] { "pwd", true }); - testData.Add(new object[] { "Input", false }); + testData.Add(new object[] { "password" }); + testData.Add(new object[] { "pwd" }); + testData.Add(new object[] { "Input" }); break; case Tests.DefaultWithRequestBodyWithError: testData.Add(new object[] { "password" }); @@ -121,15 +121,15 @@ public static IEnumerable GetData(Tests test) testData.Add(new object[] { "creditcardnumber" }); //*card break; case Tests.CustomWithRequestBodyNoError: - testData.Add(new object[] { "mysecurityheader", "mySECURITYHeader", true }); - testData.Add(new object[] { "(?-i)mysecurityheader", "mySECURITYHeader", false }); - testData.Add(new object[] { "(?-i)mySECURITYheader", "mySECURITYheader", true }); - testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmySECURITYheader", true }); - testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmysecURITYheader", false }); - testData.Add(new object[] { "(?-i)mySECURITYheader*", "mySECURITYheaderTest", true }); - testData.Add(new object[] { "(?-i)mySECURITYheader*", "mysecURITYheaderTest", false }); - testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmySECURITYheaderTest", true }); - testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmysecURITYheaderTest", false }); + testData.Add(new object[] { "mysecurityheader", "mySECURITYHeader" }); + testData.Add(new object[] { "(?-i)mysecurityheader", "mySECURITYHeader" }); + testData.Add(new object[] { "(?-i)mySECURITYheader", "mySECURITYheader" }); + testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmySECURITYheader" }); + testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmysecURITYheader" }); + testData.Add(new object[] { "(?-i)mySECURITYheader*", "mySECURITYheaderTest" }); + testData.Add(new object[] { "(?-i)mySECURITYheader*", "mysecURITYheaderTest" }); + testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmySECURITYheaderTest" }); + testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmysecURITYheaderTest" }); break; } @@ -164,7 +164,7 @@ public static IEnumerable GetData(Tests test) return retVal; } - private void CreateAgent(bool useDiagnosticSourceOnly, string sanitizeFieldNames = null) + private void CreateAgent(string sanitizeFieldNames = null) { var configSnapshot = sanitizeFieldNames == null ? new MockConfiguration(_logger, captureBody: "all") @@ -179,7 +179,7 @@ private void CreateAgent(bool useDiagnosticSourceOnly, string sanitizeFieldNames new CurrentExecutionSegmentsContainer()); _agent = new ApmAgent(agentComponents); - _client = Helper.GetClient(_agent, _factory, useDiagnosticSourceOnly); + _client = Helper.GetClient(_agent, _factory); } /// @@ -188,13 +188,12 @@ private void CreateAgent(bool useDiagnosticSourceOnly, string sanitizeFieldNames /// /// /// - /// /// [MemberData(nameof(GetData), Tests.CustomSanitizeFieldNameSettingWithHeaders)] [Theory] - public async Task CustomSanitizeFieldNameSettingWithHeaders(string sanitizeFieldNames, string[] headerNames, bool useOnlyDiagnosticSource) + public async Task CustomSanitizeFieldNameSettingWithHeaders(string sanitizeFieldNames, string[] headerNames) { - CreateAgent(useOnlyDiagnosticSource, sanitizeFieldNames); + CreateAgent(sanitizeFieldNames); foreach (var header in headerNames) _client.DefaultRequestHeaders.Add(header, "123"); @@ -218,9 +217,8 @@ public async Task CustomSanitizeFieldNameSettingWithHeaders(string sanitizeField /// /// /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task ChangeSanitizeFieldNamesAfterStart(bool useDiagnosticSourceOnly) + [Fact] + public async Task ChangeSanitizeFieldNamesAfterStart() { var startConfigSnapshot = new MockConfiguration(new NoopLogger()); _capturedPayload = new MockPayloadSender(); @@ -231,7 +229,7 @@ public async Task ChangeSanitizeFieldNamesAfterStart(bool useDiagnosticSourceOnl new CurrentExecutionSegmentsContainer()); _agent = new ApmAgent(agentComponents); - _client = Helper.GetClient(_agent, _factory, useDiagnosticSourceOnly); + _client = Helper.GetClient(_agent, _factory); _client.DefaultRequestHeaders.Add("foo", "bar"); await _client.GetAsync("/Home/SimplePage"); @@ -275,10 +273,10 @@ public async Task ChangeSanitizeFieldNamesAfterStart(bool useDiagnosticSourceOnl [Theory] [MemberData(nameof(GetData), Tests.CustomSanitizeFieldNameSettingWithCaseSensitivityWithHeaders)] public async Task CustomSanitizeFieldNameSettingWithCaseSensitivityWithHeaders(string sanitizeFieldNames, string headerName, - bool shouldBeSanitized, bool useOnlyDiagnosticSource + bool shouldBeSanitized ) { - CreateAgent(useOnlyDiagnosticSource, sanitizeFieldNames); + CreateAgent(sanitizeFieldNames); const string headerValue = "123"; _client.DefaultRequestHeaders.Add(headerName, headerValue); @@ -302,9 +300,9 @@ public async Task CustomSanitizeFieldNameSettingWithCaseSensitivityWithHeaders(s ///// [Theory] [MemberData(nameof(GetData), Tests.DefaultsWithHeaders)] - public async Task DefaultsWithHeaders(string headerName, bool useOnlyDiagnosticSource) + public async Task DefaultsWithHeaders(string headerName) { - CreateAgent(useOnlyDiagnosticSource); + CreateAgent(); _client.DefaultRequestHeaders.Add(headerName, "123"); await _client.GetAsync("/Home/SimplePage"); @@ -320,12 +318,11 @@ public async Task DefaultsWithHeaders(string headerName, bool useOnlyDiagnosticS /// Asserts that context on error is sanitized in case of HTTP calls. /// /// - /// [Theory] [MemberData(nameof(GetData), Tests.DefaultsWithHeaders)] - public async Task SanitizeHeadersOnError(string headerName, bool useOnlyDiagnosticSource) + public async Task SanitizeHeadersOnError(string headerName) { - CreateAgent(useOnlyDiagnosticSource); + CreateAgent(); _client.DefaultRequestHeaders.Add(headerName, "123"); await _client.GetAsync("/Home/TriggerError"); @@ -357,9 +354,9 @@ public async Task SanitizeHeadersOnError(string headerName, bool useOnlyDiagnost ///// [MemberData(nameof(GetData), Tests.DefaultsWithKnownHeaders)] [Theory] - public async Task DefaultsWithKnownHeaders(string headerName, string returnedHeaderName, bool useOnlyDiagnosticSource) + public async Task DefaultsWithKnownHeaders(string headerName, string returnedHeaderName) { - CreateAgent(useOnlyDiagnosticSource); + CreateAgent(); _client.DefaultRequestHeaders.Add(headerName, "123"); await _client.GetAsync("/Home/SimplePage"); @@ -380,9 +377,9 @@ public async Task DefaultsWithKnownHeaders(string headerName, string returnedHea ///// [MemberData(nameof(GetData), Tests.DefaultWithRequestBodyNoError)] [Theory] - public async Task DefaultWithRequestBodyNoError(string formName, bool useOnlyDiagnosticSource) + public async Task DefaultWithRequestBodyNoError(string formName) { - CreateAgent(useOnlyDiagnosticSource); + CreateAgent(); var nvc = new List> { @@ -405,9 +402,9 @@ public async Task DefaultWithRequestBodyNoError(string formName, bool useOnlyDia [MemberData(nameof(GetData), Tests.DefaultWithRequestBodySingleValueNoError)] [Theory] - public async Task DefaultWithRequestBodySingleValueNoError(string formName, bool shouldBeSanitized, bool useOnlyDiagnosticSource) + public async Task DefaultWithRequestBodySingleValueNoError(string formName, bool shouldBeSanitized) { - CreateAgent(useOnlyDiagnosticSource); + CreateAgent(); var nvc = new List> { new KeyValuePair(formName, "test") }; @@ -431,9 +428,9 @@ public async Task DefaultWithRequestBodySingleValueNoError(string formName, bool ///// [MemberData(nameof(GetData), Tests.DefaultWithRequestBodyWithError)] [Theory] - public async Task DefaultWithRequestBodyWithError(string formName, bool useOnlyDiagnosticSource) + public async Task DefaultWithRequestBodyWithError(string formName) { - CreateAgent(useOnlyDiagnosticSource); + CreateAgent(); var nvc = new List> { @@ -468,15 +465,13 @@ public async Task DefaultWithRequestBodyWithError(string formName, bool useOnlyD ///// [MemberData(nameof(GetData), Tests.CustomWithRequestBodyNoError)] [Theory] - public async Task CustomWithRequestBodyNoError(string sanitizeFieldNames, string formName, bool shouldBeSanitized, - bool useOnlyDiagnosticSource - ) + public async Task CustomWithRequestBodyNoError(string sanitizeFieldNames, string formName, bool shouldBeSanitized) { - CreateAgent(useOnlyDiagnosticSource, sanitizeFieldNames); + CreateAgent(sanitizeFieldNames); var nvc = new List> { - new KeyValuePair("Input1", "test1"), new KeyValuePair(formName, "test2") + new("Input1", "test1"), new(formName, "test2") }; var req = new HttpRequestMessage(HttpMethod.Post, "api/Home/Post") { Content = new FormUrlEncodedContent(nvc) }; diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/TraceIdWithActivityTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/TraceIdWithActivityTests.cs index b226c5146..b7226f89a 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/TraceIdWithActivityTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/TraceIdWithActivityTests.cs @@ -33,7 +33,7 @@ public TraceIdWithActivityTests(WebApplicationFactory factory) // because the sample application used by the tests (SampleAspNetCoreApp) uses Agent.Instance.Tracer.CurrentTransaction/CurrentSpan currentExecutionSegmentsContainer: Agent.Instance.TracerInternal.CurrentExecutionSegmentsContainer)); HostBuilderExtensions.UpdateServiceInformation(_agent.Service); - _client = Helper.GetClient(_agent, _factory, true); + _client = Helper.GetClient(_agent, _factory); } public void Dispose() diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/TransactionIgnoreUrlsTest.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/TransactionIgnoreUrlsTest.cs index 4f48f9f69..369380440 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/TransactionIgnoreUrlsTest.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/TransactionIgnoreUrlsTest.cs @@ -47,10 +47,10 @@ public TransactionIgnoreUrlsTest(WebApplicationFactory factory) private HttpClient _client; - private void Setup(bool useOnlyDiagnosticSource) + private void Setup() { #pragma warning disable IDE0022 // Use expression body for methods - _client = Helper.GetClient(_agent, _factory, useOnlyDiagnosticSource); + _client = Helper.GetClient(_agent, _factory); #pragma warning restore IDE0022 // Use expression body for methods #if NETCOREAPP3_0 || NETCOREAPP3_1 _client.DefaultRequestVersion = new Version(2, 0); @@ -60,11 +60,9 @@ private void Setup(bool useOnlyDiagnosticSource) /// /// Changes the transactionIgnoreUrls during startup and asserts that the agent reacts accordingly. /// - /// /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task ChangeTransactionIgnoreUrlsAfterStart(bool useDiagnosticSourceOnly) + [Fact] + public async Task ChangeTransactionIgnoreUrlsAfterStart() { // Start with default config var startConfigSnapshot = new MockConfiguration(new NoopLogger()); @@ -76,7 +74,7 @@ public async Task ChangeTransactionIgnoreUrlsAfterStart(bool useDiagnosticSource new CurrentExecutionSegmentsContainer()); _agent = new ApmAgent(agentComponents); - _client = Helper.GetClient(_agent, _factory, useDiagnosticSourceOnly); + _client = Helper.GetClient(_agent, _factory); _client.DefaultRequestHeaders.Add("foo", "bar"); await _client.GetAsync("/Home/SimplePage"); @@ -118,11 +116,10 @@ public async Task ChangeTransactionIgnoreUrlsAfterStart(bool useDiagnosticSource /// In the ctor we add `*SimplePage` to the ignoreUrl list. This test makes sure that /home/SimplePage is indeed ignored. /// /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task IgnoreSimplePage(bool useOnlyDiagnosticSource) + [Fact] + public async Task IgnoreSimplePage() { - Setup(useOnlyDiagnosticSource); + Setup(); var response = await _client.GetAsync("/Home/SimplePage?myUrlParam=123"); response.IsSuccessStatusCode.Should().BeTrue(); diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/TransactionNameTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/TransactionNameTests.cs index 7f5d33129..7280d5cf7 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/TransactionNameTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/TransactionNameTests.cs @@ -49,11 +49,10 @@ public TransactionNameTests(WebApplicationFactory factory, ITestOutputH /// Calls a URL that maps to a route with optional parameter (id). /// Makes sure the Transaction.Name contains "{id}" instead of the value. /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task OptionalRouteParameter(bool diagnosticSourceOnly) + [Fact] + public async Task OptionalRouteParameter() { - var httpClient = Helper.GetClient(_agent, _factory, diagnosticSourceOnly); + var httpClient = Helper.GetClient(_agent, _factory); await httpClient.GetAsync("home/sample/3"); await httpClient.GetAsync("home/sample/2"); @@ -64,11 +63,10 @@ public async Task OptionalRouteParameter(bool diagnosticSourceOnly) /// Calls a URL and sets custom transaction name. /// Makes sure the Transaction.Name can be set to custom name. /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task CustomTransactionName(bool diagnosticSourceOnly) + [Fact] + public async Task CustomTransactionName() { - var httpClient = Helper.GetClient(_agent, _factory, diagnosticSourceOnly); + var httpClient = Helper.GetClient(_agent, _factory); await httpClient.GetAsync("home/TransactionWithCustomName"); _payloadSender.WaitForTransactions(); @@ -81,11 +79,10 @@ public async Task CustomTransactionName(bool diagnosticSourceOnly) /// change that. /// See: https://github.com/elastic/apm-agent-dotnet/pull/258#discussion_r291025014 /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task CustomTransactionNameWithNameUsingRequestInfo(bool diagnosticSourceOnly) + [Fact] + public async Task CustomTransactionNameWithNameUsingRequestInfo() { - var httpClient = Helper.GetClient(_agent, _factory, diagnosticSourceOnly); + var httpClient = Helper.GetClient(_agent, _factory); if (httpClient != null) { await httpClient.GetAsync("home/TransactionWithCustomNameUsingRequestInfo"); @@ -99,11 +96,10 @@ public async Task CustomTransactionNameWithNameUsingRequestInfo(bool diagnosticS /// Calls a URL that would map to a route with optional parameter (id), but does not pass the id value. /// Makes sure no template value is captured in Transaction.Name. /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task OptionalRouteParameterWithNull(bool diagnosticSourceOnly) + [Fact] + public async Task OptionalRouteParameterWithNull() { - var httpClient = Helper.GetClient(_agent, _factory, diagnosticSourceOnly); + var httpClient = Helper.GetClient(_agent, _factory); await httpClient.GetAsync("home/sample"); _payloadSender.WaitForTransactions(); @@ -114,11 +110,10 @@ public async Task OptionalRouteParameterWithNull(bool diagnosticSourceOnly) /// Tests a URL that maps to a route with default values. Calls "/", which maps to "home/index". /// Makes sure "Home/Index" is in the Transaction.Name. /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task DefaultRouteParameterValues(bool diagnosticSourceOnly) + [Fact] + public async Task DefaultRouteParameterValues() { - var httpClient = Helper.GetClient(_agent, _factory, diagnosticSourceOnly); + var httpClient = Helper.GetClient(_agent, _factory); await httpClient.GetAsync("/"); _payloadSender.WaitForTransactions(); @@ -130,11 +125,10 @@ public async Task DefaultRouteParameterValues(bool diagnosticSourceOnly) /// Tests a URL that maps to an area route with an area route value and default values. Calls "/MyArea", which maps to "MyArea/Home/Index". /// Makes sure "GET MyArea/Home/Index" is the Transaction.Name. /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task Name_Should_Be_Area_Controller_Action_When_Mvc_Area_Controller_Action(bool diagnosticSourceOnly) + [Fact] + public async Task Name_Should_Be_Area_Controller_Action_When_Mvc_Area_Controller_Action() { - var httpClient = Helper.GetClient(_agent, _factory, diagnosticSourceOnly); + var httpClient = Helper.GetClient(_agent, _factory); await httpClient.GetAsync("/MyArea"); _payloadSender.WaitForTransactions(); @@ -146,11 +140,10 @@ public async Task Name_Should_Be_Area_Controller_Action_When_Mvc_Area_Controller /// Tests a URL that maps to an explicit area route with default values. Calls "/MyOtherArea", which maps to "MyOtherArea/Home/Index". /// Makes sure "GET MyOtherArea/Home/Index" is the Transaction.Name. /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task Name_Should_Be_Area_Controller_Action_When_Mvc_Area_Controller_Action_With_Area_Route(bool diagnosticSourceOnly) + [Fact] + public async Task Name_Should_Be_Area_Controller_Action_When_Mvc_Area_Controller_Action_With_Area_Route() { - var httpClient = Helper.GetClient(_agent, _factory, diagnosticSourceOnly); + var httpClient = Helper.GetClient(_agent, _factory); await httpClient.GetAsync("/MyOtherArea"); _payloadSender.WaitForTransactions(); @@ -161,14 +154,12 @@ public async Task Name_Should_Be_Area_Controller_Action_When_Mvc_Area_Controller /// /// Calls a URL that maps to no route and causes a 404 /// - [InlineData("home/doesnotexist", true)] - [InlineData("home/doesnotexist", false)] - [InlineData("files/doesnotexist/somefile", true)] - [InlineData("files/doesnotexist/somefile", false)] + [InlineData("home/doesnotexist")] + [InlineData("files/doesnotexist/somefile")] [Theory] - public async Task NotFoundRoute_ShouldBe_Aggregatable(string url, bool diagnosticSourceOnly) + public async Task NotFoundRoute_ShouldBe_Aggregatable(string url) { - var httpClient = Helper.GetClient(_agent, _factory, diagnosticSourceOnly); + var httpClient = Helper.GetClient(_agent, _factory); await httpClient.GetAsync(url); _payloadSender.WaitForTransactions(); @@ -181,11 +172,10 @@ public async Task NotFoundRoute_ShouldBe_Aggregatable(string url, bool diagnosti /// If a URL matches a route, but the controller method returns e.g. HTTP 404, /// the method name should be the real route and not "unknown route". /// - [Theory] - [MemberData(nameof(MemberData.TestWithDiagnosticSourceOnly), MemberType = typeof(MemberData))] - public async Task Http404WithValidRoute(bool diagnosticSourceOnly) + [Fact] + public async Task Http404WithValidRoute() { - var httpClient = Helper.GetClient(_agent, _factory, diagnosticSourceOnly); + var httpClient = Helper.GetClient(_agent, _factory); await httpClient.GetAsync("api/Home/ReturnNotFound/42"); _payloadSender.WaitForTransactions(); From 4af01f1c23587733a77a40fe0707686e28a17b29 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Fri, 1 Dec 2023 11:02:43 +0100 Subject: [PATCH 5/8] fix diagnosticlistener tests in Elastic.Apm.AspNetCore.Tests --- .../DiagnosticListenerTests.cs | 37 +------------------ .../Elastic.Apm.AspNetCore.Tests/Helper.cs | 4 +- 2 files changed, 3 insertions(+), 38 deletions(-) diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/DiagnosticListenerTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/DiagnosticListenerTests.cs index af833bc18..6dc6ca36a 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/DiagnosticListenerTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/DiagnosticListenerTests.cs @@ -43,42 +43,7 @@ public DiagnosticListenerTests(WebApplicationFactory factory, ITestOutp //This registers the middleware without activating any listeners, //so no error capturing and no EFCore listener. - _client = Helper.GetClientWithoutDiagnosticListeners(factory); - } - - /// - /// Manually starts and does 1 HTTP call - /// that throws an exception, - /// then it disposes the (aka unsubsribes) - /// and does another HTTP call that throws an exception. - /// It makes sure that for the 1. HTTP call the errors is captured and for the 2. it isn't. - /// - [Fact] - public async Task SubscribeAndUnsubscribeAspNetCoreDiagnosticListener() - { - //For reference: unsubscribing from AspNetCoreDiagnosticListener does not seem to work. - //TODO: this should be investigated. This is more relevant for testing. - // using (_agent.Subscribe(new AspNetCoreDiagnosticsSubscriber())) - // { - // await _client.GetAsync("/Home/TriggerError"); - // - // _capturedPayload.Transactions.Should().ContainSingle(); - // - // _capturedPayload.Errors.Should().NotBeEmpty(); - // _capturedPayload.Errors.Should().ContainSingle(); - // _capturedPayload.Errors[0].CapturedException.Type.Should().Be(typeof(Exception).FullName); - // } //here we unsubsribe, so no errors should be captured after this line. - - _agent.Dispose(); - - _capturedPayload.Clear(); - - await _client.GetAsync("/Home/TriggerError"); - - _capturedPayload.WaitForTransactions(TimeSpan.FromSeconds(10)); - - _capturedPayload.Transactions.Should().ContainSingle(); - _capturedPayload.Errors.Should().BeEmpty(); + _client = Helper.GetClientWithoutDiagnosticListeners(_agent, factory); } /// diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs index cde7d0c4f..8fed78d03 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/Helper.cs @@ -86,12 +86,12 @@ internal static HttpClient GetClientWithoutExceptionPage(ApmAgent agent, WebA /// /// Configures the sample app without any diagnostic listener /// - internal static HttpClient GetClientWithoutDiagnosticListeners(WebApplicationFactory factory) where T : class + internal static HttpClient GetClientWithoutDiagnosticListeners(ApmAgent agent, WebApplicationFactory factory) where T : class => factory.WithWebHostBuilder(n => { n.Configure(app => { - app.UseElasticApm(); + app.UseElasticApm(agent, agent.Logger); app.UseDeveloperExceptionPage(); app.UseHsts(); app.UseHttpsRedirection(); From 43b15e6e1c4ad065eff16daec983b55d3f03e739 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Fri, 1 Dec 2023 13:54:24 +0100 Subject: [PATCH 6/8] fix testing requiring Mvc.Testing fix --- .../XUnit/DisabledTestFact.cs | 11 --- .../XUnit/FactRequiresMvcTestingFix.cs | 27 ++++++ .../BodyCapturingTests.cs | 2 +- .../SanitizeFieldNamesTests.cs | 87 +++++++------------ 4 files changed, 57 insertions(+), 70 deletions(-) create mode 100644 test/Elastic.Apm.Tests.Utilities/XUnit/FactRequiresMvcTestingFix.cs diff --git a/test/Elastic.Apm.Tests.Utilities/XUnit/DisabledTestFact.cs b/test/Elastic.Apm.Tests.Utilities/XUnit/DisabledTestFact.cs index 24f2355cd..0b41a5508 100644 --- a/test/Elastic.Apm.Tests.Utilities/XUnit/DisabledTestFact.cs +++ b/test/Elastic.Apm.Tests.Utilities/XUnit/DisabledTestFact.cs @@ -3,7 +3,6 @@ // Elasticsearch B.V 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 Xunit; namespace Elastic.Apm.Tests.Utilities.XUnit; @@ -17,13 +16,3 @@ public DisabledTestFact(string reason, string issueLink = null) Skip += $", issue link: {issueLink}"; } } - -public sealed class FactRequiresMvcTestingFix : FactAttribute -{ - public FactRequiresMvcTestingFix() - { - if (Environment.Version.Major < 7) return; - Skip = $"This Test is disabled on .NET 7 until https://github.com/dotnet/aspnetcore/issues/45233"; - } -} - diff --git a/test/Elastic.Apm.Tests.Utilities/XUnit/FactRequiresMvcTestingFix.cs b/test/Elastic.Apm.Tests.Utilities/XUnit/FactRequiresMvcTestingFix.cs new file mode 100644 index 000000000..51b985fbb --- /dev/null +++ b/test/Elastic.Apm.Tests.Utilities/XUnit/FactRequiresMvcTestingFix.cs @@ -0,0 +1,27 @@ +// Licensed to Elasticsearch B.V under +// one or more agreements. +// Elasticsearch B.V 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 Xunit; + +namespace Elastic.Apm.Tests.Utilities.XUnit; + +public sealed class FactRequiresMvcTestingFix : FactAttribute +{ + public FactRequiresMvcTestingFix() + { + if (Environment.Version.Major < 7) return; + Skip = $"This test is disabled on .NET 7 until https://github.com/dotnet/aspnetcore/issues/45233"; + } +} + +public sealed class TheoryRequiresMvcTestingFix : TheoryAttribute +{ + public TheoryRequiresMvcTestingFix() + { + if (Environment.Version.Major < 7) return; + Skip = $"This test is disabled on .NET 7 until https://github.com/dotnet/aspnetcore/issues/45233"; + } +} diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/BodyCapturingTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/BodyCapturingTests.cs index c70b0a609..dd6f5ca78 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/BodyCapturingTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/BodyCapturingTests.cs @@ -310,7 +310,7 @@ private static IEnumerable BuildOptionsTestVariants() } } - [Theory] + [TheoryRequiresMvcTestingFix] [MemberData(nameof(OptionsChangedAfterStartTestVariants))] public async Task Options_changed_after_start(int startCfgVariantIndex, OptionsTestVariant startCfgVariant) { diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs index c4b93c5a1..c612d0c04 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/SanitizeFieldNamesTests.cs @@ -63,15 +63,15 @@ public static IEnumerable GetData(Tests test) testData.Add(new object[] { "mySecurityHeader", new[] { "mySECURITYHeader" } }); break; case Tests.CustomSanitizeFieldNameSettingWithCaseSensitivityWithHeaders: - testData.Add(new object[] { "mysecurityheader", "mySECURITYHeader" }); - testData.Add(new object[] { "(?-i)mysecurityheader", "mySECURITYHeader" }); - testData.Add(new object[] { "(?-i)mySECURITYheader", "mySECURITYheader" }); - testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmySECURITYheader" }); - testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmysecURITYheader" }); - testData.Add(new object[] { "(?-i)mySECURITYheader*", "mySECURITYheaderTest" }); - testData.Add(new object[] { "(?-i)mySECURITYheader*", "mysecURITYheaderTest" }); - testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmySECURITYheaderTest" }); - testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmysecURITYheaderTest" }); + testData.Add(new object[] { "mysecurityheader", "mySECURITYHeader", true }); + testData.Add(new object[] { "(?-i)mysecurityheader", "mySECURITYHeader", false }); + testData.Add(new object[] { "(?-i)mySECURITYheader", "mySECURITYheader", true }); + testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmySECURITYheader", true }); + testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmysecURITYheader", false }); + testData.Add(new object[] { "(?-i)mySECURITYheader*", "mySECURITYheaderTest", true }); + testData.Add(new object[] { "(?-i)mySECURITYheader*", "mysecURITYheaderTest", false }); + testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmySECURITYheaderTest", true }); + testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmysecURITYheaderTest", false }); break; case Tests.DefaultsWithHeaders: testData.Add(new object[] { "password" }); @@ -106,9 +106,9 @@ public static IEnumerable GetData(Tests test) testData.Add(new object[] { "creditcardnumber" }); //*card break; case Tests.DefaultWithRequestBodySingleValueNoError: - testData.Add(new object[] { "password" }); - testData.Add(new object[] { "pwd" }); - testData.Add(new object[] { "Input" }); + testData.Add(new object[] { "password", true }); + testData.Add(new object[] { "pwd", true }); + testData.Add(new object[] { "Input", false }); break; case Tests.DefaultWithRequestBodyWithError: testData.Add(new object[] { "password" }); @@ -121,47 +121,18 @@ public static IEnumerable GetData(Tests test) testData.Add(new object[] { "creditcardnumber" }); //*card break; case Tests.CustomWithRequestBodyNoError: - testData.Add(new object[] { "mysecurityheader", "mySECURITYHeader" }); - testData.Add(new object[] { "(?-i)mysecurityheader", "mySECURITYHeader" }); - testData.Add(new object[] { "(?-i)mySECURITYheader", "mySECURITYheader" }); - testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmySECURITYheader" }); - testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmysecURITYheader" }); - testData.Add(new object[] { "(?-i)mySECURITYheader*", "mySECURITYheaderTest" }); - testData.Add(new object[] { "(?-i)mySECURITYheader*", "mysecURITYheaderTest" }); - testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmySECURITYheaderTest" }); - testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmysecURITYheaderTest" }); + testData.Add(new object[] { "mysecurityheader", "mySECURITYHeader", true }); + testData.Add(new object[] { "(?-i)mysecurityheader", "mySECURITYHeader", false }); + testData.Add(new object[] { "(?-i)mySECURITYheader", "mySECURITYheader", true }); + testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmySECURITYheader", true }); + testData.Add(new object[] { "(?-i)*mySECURITYheader", "TestmysecURITYheader", false }); + testData.Add(new object[] { "(?-i)mySECURITYheader*", "mySECURITYheaderTest", true }); + testData.Add(new object[] { "(?-i)mySECURITYheader*", "mysecURITYheaderTest", false }); + testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmySECURITYheaderTest", true }); + testData.Add(new object[] { "(?-i)*mySECURITYheader*", "TestmysecURITYheaderTest", false }); break; } - - var retVal = new List(); - - // Add true and false to the end of each test data, so we test it both with middleware and with diagnosticsource - foreach (var testDataItem in testData) - { - // - // Skip "DiagnosticSourceOnly" tests on .NET 7 - // until https://github.com/dotnet/aspnetcore/issues/45233 is resolved. - // - if (Environment.Version.Major < 7) - { - var newItem = new List(); - foreach (var item in testDataItem) - newItem.Add(item); - newItem.Add(true); - - retVal.Add(newItem.ToArray()); - } - { - var newItem = new List(); - foreach (var item in testDataItem) - newItem.Add(item); - newItem.Add(false); - - retVal.Add(newItem.ToArray()); - } - } - - return retVal; + return testData; } private void CreateAgent(string sanitizeFieldNames = null) @@ -376,14 +347,14 @@ public async Task DefaultsWithKnownHeaders(string headerName, string returnedHea ///// ///// [MemberData(nameof(GetData), Tests.DefaultWithRequestBodyNoError)] - [Theory] + [TheoryRequiresMvcTestingFix] public async Task DefaultWithRequestBodyNoError(string formName) { CreateAgent(); var nvc = new List> { - new KeyValuePair("Input1", "test1"), new KeyValuePair(formName, "test2") + new("Input1", "test1"), new(formName, "test2") }; var req = new HttpRequestMessage(HttpMethod.Post, "api/Home/Post") { Content = new FormUrlEncodedContent(nvc) }; @@ -401,12 +372,12 @@ public async Task DefaultWithRequestBodyNoError(string formName) } [MemberData(nameof(GetData), Tests.DefaultWithRequestBodySingleValueNoError)] - [Theory] + [TheoryRequiresMvcTestingFix] public async Task DefaultWithRequestBodySingleValueNoError(string formName, bool shouldBeSanitized) { CreateAgent(); - var nvc = new List> { new KeyValuePair(formName, "test") }; + var nvc = new List> { new(formName, "test") }; var req = new HttpRequestMessage(HttpMethod.Post, "api/Home/Post") { Content = new FormUrlEncodedContent(nvc) }; var res = await _client.SendAsync(req); @@ -427,14 +398,14 @@ public async Task DefaultWithRequestBodySingleValueNoError(string formName, bool ///// ///// [MemberData(nameof(GetData), Tests.DefaultWithRequestBodyWithError)] - [Theory] + [TheoryRequiresMvcTestingFix] public async Task DefaultWithRequestBodyWithError(string formName) { CreateAgent(); var nvc = new List> { - new KeyValuePair("Input1", "test1"), new KeyValuePair(formName, "test2") + new("Input1", "test1"), new(formName, "test2") }; var req = new HttpRequestMessage(HttpMethod.Post, "api/Home/PostError") { Content = new FormUrlEncodedContent(nvc) }; @@ -464,7 +435,7 @@ public async Task DefaultWithRequestBodyWithError(string formName) ///// ///// [MemberData(nameof(GetData), Tests.CustomWithRequestBodyNoError)] - [Theory] + [TheoryRequiresMvcTestingFix] public async Task CustomWithRequestBodyNoError(string sanitizeFieldNames, string formName, bool shouldBeSanitized) { CreateAgent(sanitizeFieldNames); From 34e7158961de0556a2454990762b7b6467fee012 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Fri, 1 Dec 2023 14:55:29 +0100 Subject: [PATCH 7/8] dotnet format --- .../XUnit/FactRequiresMvcTestingFix.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/Elastic.Apm.Tests.Utilities/XUnit/FactRequiresMvcTestingFix.cs b/test/Elastic.Apm.Tests.Utilities/XUnit/FactRequiresMvcTestingFix.cs index 51b985fbb..0974a18af 100644 --- a/test/Elastic.Apm.Tests.Utilities/XUnit/FactRequiresMvcTestingFix.cs +++ b/test/Elastic.Apm.Tests.Utilities/XUnit/FactRequiresMvcTestingFix.cs @@ -12,7 +12,8 @@ public sealed class FactRequiresMvcTestingFix : FactAttribute { public FactRequiresMvcTestingFix() { - if (Environment.Version.Major < 7) return; + if (Environment.Version.Major < 7) + return; Skip = $"This test is disabled on .NET 7 until https://github.com/dotnet/aspnetcore/issues/45233"; } } @@ -21,7 +22,8 @@ public sealed class TheoryRequiresMvcTestingFix : TheoryAttribute { public TheoryRequiresMvcTestingFix() { - if (Environment.Version.Major < 7) return; + if (Environment.Version.Major < 7) + return; Skip = $"This test is disabled on .NET 7 until https://github.com/dotnet/aspnetcore/issues/45233"; } } From 9a6a03acc2126fc3ea370d0b45dcc1743c3dd968 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Wed, 6 Dec 2023 14:53:04 +0100 Subject: [PATCH 8/8] address PR feedback --- .../Elastic.Apm.AspNetCore/ApplicationBuilderExtensions.cs | 1 - test/Elastic.Apm.Tests.Utilities/XUnit/MemberData.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/integrations/Elastic.Apm.AspNetCore/ApplicationBuilderExtensions.cs b/src/integrations/Elastic.Apm.AspNetCore/ApplicationBuilderExtensions.cs index 54445323a..97be645cf 100644 --- a/src/integrations/Elastic.Apm.AspNetCore/ApplicationBuilderExtensions.cs +++ b/src/integrations/Elastic.Apm.AspNetCore/ApplicationBuilderExtensions.cs @@ -88,7 +88,6 @@ params IDiagnosticsSubscriber[] subscribers subs.Add(new AspNetCoreDiagnosticSubscriber()); agent.Subscribe(subs.ToArray()); - //return builder.UseMiddleware(agent.Tracer, agent); return builder; } diff --git a/test/Elastic.Apm.Tests.Utilities/XUnit/MemberData.cs b/test/Elastic.Apm.Tests.Utilities/XUnit/MemberData.cs index f7a07e897..a8f6816eb 100644 --- a/test/Elastic.Apm.Tests.Utilities/XUnit/MemberData.cs +++ b/test/Elastic.Apm.Tests.Utilities/XUnit/MemberData.cs @@ -13,7 +13,7 @@ public static IEnumerable TestWithDiagnosticSourceOnly() yield return new object[] { false }; // // Skip "DiagnosticSourceOnly" tests on .NET 7 - // until is resolved. + // until https://github.com/dotnet/aspnetcore/issues/45233 is resolved. // if (Environment.Version.Major < 7) yield return new object[] { true };