diff --git a/src/Elastic.Apm/Helpers/DbgInstanceNameGenerator.cs b/src/Elastic.Apm/Helpers/DbgInstanceNameGenerator.cs deleted file mode 100644 index 6fa1e7613..000000000 --- a/src/Elastic.Apm/Helpers/DbgInstanceNameGenerator.cs +++ /dev/null @@ -1,30 +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.Text; -using System.Threading; - -namespace Elastic.Apm.Helpers -{ - internal struct DbgInstanceNameGenerator - { - private long _lastId; - - // ReSharper disable once UnusedMember.Global - internal DbgInstanceNameGenerator(long startId = 1) => _lastId = startId - 1; - - internal string Generate(string prefix = null) - { - var result = new StringBuilder(); - - if (prefix != null) - result.Append(prefix); - - var currentId = Interlocked.Increment(ref _lastId); - result.Append(currentId); - - return result.ToString(); - } - } -} diff --git a/src/integrations/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs b/src/integrations/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs index 95f3bcaa2..8ccd015d0 100644 --- a/src/integrations/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs +++ b/src/integrations/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.Collections.Specialized; using System.Data.SqlClient; +using System.Diagnostics; using System.Linq; using System.Reflection; using System.Security.Claims; @@ -15,7 +16,6 @@ using Elastic.Apm.Api; using Elastic.Apm.AspNetFullFramework.Extensions; using Elastic.Apm.Config.Net4FullFramework; -using Elastic.Apm.DiagnosticSource; using Elastic.Apm.Extensions; using Elastic.Apm.Helpers; using Elastic.Apm.Logging; @@ -31,47 +31,144 @@ namespace Elastic.Apm.AspNetFullFramework /// public class ElasticApmModule : IHttpModule { - private static bool _isCaptureHeadersEnabled; - // ReSharper disable once RedundantDefaultMemberInitializer - private static readonly DbgInstanceNameGenerator DbgInstanceNameGenerator = new(); + private static volatile bool ApplicationStarted = false; + private static readonly object ApplicationStartedLock = new(); + private static bool IsCaptureHeadersEnabled; + private static bool UsingIntegratedPipeline = true; + private static readonly LazyContextualInit InitOnceHelper = new(); private static readonly MethodInfo OnExecuteRequestStepMethodInfo = typeof(HttpApplication).GetMethod("OnExecuteRequestStep"); - private readonly string _dbgInstanceName; - private HttpApplication _application; private IApmLogger _logger; private readonly Lazy _httpRouteDataInterfaceType = - new Lazy(() => Type.GetType("System.Web.Http.Routing.IHttpRouteData,System.Web.Http")); + new(() => Type.GetType("System.Web.Http.Routing.IHttpRouteData,System.Web.Http")); private Func _routeDataTemplateGetter; private Func _routePrecedenceGetter; + private static int InstanceCount; - public ElasticApmModule() => - // ReSharper disable once ImpureMethodCallOnReadonlyValueField - _dbgInstanceName = DbgInstanceNameGenerator.Generate($"{nameof(ElasticApmModule)}.#"); - - public void Dispose() => _application = null; + public ElasticApmModule() + { + var instanceCounter = Interlocked.Increment(ref InstanceCount); + _dbgInstanceName = $"{nameof(ElasticApmModule)}.#{instanceCounter}"; + } /// public void Init(HttpApplication application) { try { - InitImpl(application); + // If we've already attempted initialisation and determined we are not in integrated mode, we can return quickly here. + // As `UsingIntegratedPipeline` is initialised as `true`, we pass through here at least once. + if (!UsingIntegratedPipeline) + return; + + if (!ApplicationStarted) + { + AttemptAgentInitialization(); + _logger.Trace() + ?.Log($"{nameof(ElasticApmModule)}.{nameof(Init)} was invoked and called {nameof(AttemptAgentInitialization)}."); + } + else + { + _logger ??= CreateScopedLogger(); + _logger.Trace() + ?.Log($"{nameof(ElasticApmModule)}.{nameof(Init)} was invoked by an instance when the Agent has already been initialized. " + + "No further initialization required."); + } + + if (!Agent.Config.Enabled) + return; + + _routeDataTemplateGetter = CreateWebApiAttributeRouteTemplateGetter(); + _routePrecedenceGetter = CreateRoutePrecedenceGetter(); + + application.BeginRequest += OnBeginRequest; + application.EndRequest += OnEndRequest; + application.Error += OnError; + + if (OnExecuteRequestStepMethodInfo != null) + { + // OnExecuteRequestStep is available starting with 4.7.1 + try + { +#pragma warning disable IDE0300 // Simplify collection initialization + OnExecuteRequestStepMethodInfo.Invoke(application, new object[] { (Action)OnExecuteRequestStep }); +#pragma warning restore IDE0300 // Simplify collection initialization + } + catch (Exception e) + { + _logger.Error() + ?.LogException(e, "Failed to invoke OnExecuteRequestStep. .NET runtime: {DotNetRuntimeDescription}; IIS: {IisVersion}", + PlatformDetection.DotNetRuntimeDescription, HttpRuntime.IISVersion); + } + } } catch (Exception ex) { const string linePrefix = "Elastic APM .NET Agent: "; - System.Diagnostics.Trace.WriteLine($"{linePrefix}[CRITICAL] Exception thrown by {nameof(ElasticApmModule)}.{nameof(InitImpl)}." + Trace.WriteLine($"{linePrefix}[CRITICAL] Exception thrown by {nameof(ElasticApmModule)}.{nameof(Init)}." + Environment.NewLine + linePrefix + $"+-> Exception: {ex.GetType().FullName}: {ex.Message}" + Environment.NewLine + TextUtils.PrefixEveryLine(ex.StackTrace, linePrefix + " ".Repeat(4)) ); } } + private void AttemptAgentInitialization() + { + // We use a lock here to ensure that if multiple `HttpApplication` instances are created, each calling the `Init` method + // on registered module, that we initialise the agent only once. The first instance wins and any concurrent calls to `Init` + // should block until this is completed such that they only continue once our initialisation is completed. + lock (ApplicationStartedLock) + { + if (ApplicationStarted) + { + _logger ??= CreateScopedLogger(); + _logger.Trace()?.Log("Lock aquired, but Agent singleton has already been initialized. Skipping initialization."); + return; + } + + var agentComponents = CreateAgentComponents(_dbgInstanceName); + + _logger = agentComponents.Logger.Scoped(_dbgInstanceName); + _logger.Trace()?.Log("Initializing singleton Agent."); + + // We store this in a static field as it should be consistent for all invocations. We can then log our error once but + // also short-circuit subsequent `Init` calls when we've already determined the app is hosted on an incompatible pipeline. + UsingIntegratedPipeline = HttpRuntime.UsingIntegratedPipeline; + + if (!UsingIntegratedPipeline) + { + _logger.Error() + ?.Log("Skipping Agent initialization. Elastic APM Module requires the IIS Application Pool to run under an Integrated Pipeline." + + " .NET runtime: {DotNetRuntimeDescription}; IIS: {IisVersion}", + PlatformDetection.DotNetRuntimeDescription, HttpRuntime.IISVersion); + + return; + } + + Agent.Setup(agentComponents); + + _logger.Debug() + ?.Log("Initialized Agent singleton. .NET runtime: {DotNetRuntimeDescription}; IIS: {IisVersion}", + PlatformDetection.DotNetRuntimeDescription, HttpRuntime.IISVersion); + + if (!Agent.Instance.Configuration.Enabled) + return; + + IsCaptureHeadersEnabled = Agent.Instance.Configuration.CaptureHeaders; + + Agent.Instance.SubscribeIncludingAllDefaults(); + + ApplicationStarted = true; + } + } + + private ScopedLogger CreateScopedLogger() => Agent.Instance.Logger.Scoped(_dbgInstanceName); + /// /// Creates a new instance of configured /// to use with ASP.NET Full Framework. @@ -96,54 +193,6 @@ internal static AgentComponents CreateAgentComponents(string debugName) return agentComponents; } - private void InitImpl(HttpApplication application) - { - var isInitedByThisCall = InitOnceForAllInstancesUnderLock(_dbgInstanceName); - - _logger = Agent.Instance.Logger.Scoped(_dbgInstanceName); - - if (!Agent.Config.Enabled) - return; - - if (!HttpRuntime.UsingIntegratedPipeline) - { - _logger.Error() - ?.Log("Skipping Initialization. Elastic APM Module requires the application pool to run under an Integrated Pipeline." - + " .NET runtime: {DotNetRuntimeDescription}; IIS: {IisVersion}", - PlatformDetection.DotNetRuntimeDescription, HttpRuntime.IISVersion); - return; - } - - if (isInitedByThisCall) - { - _logger.Debug() - ?.Log("Initialized Agent singleton. .NET runtime: {DotNetRuntimeDescription}; IIS: {IisVersion}", - PlatformDetection.DotNetRuntimeDescription, HttpRuntime.IISVersion); - } - - _routeDataTemplateGetter = CreateWebApiAttributeRouteTemplateGetter(); - _routePrecedenceGetter = CreateRoutePrecedenceGetter(); - _application = application; - _application.BeginRequest += OnBeginRequest; - _application.EndRequest += OnEndRequest; - _application.Error += OnError; - - if (OnExecuteRequestStepMethodInfo != null) - { - // OnExecuteRequestStep is available starting with 4.7.1 - try - { - OnExecuteRequestStepMethodInfo.Invoke(application, new object[] { (Action)OnExecuteRequestStep }); - } - catch (Exception e) - { - _logger.Error() - ?.LogException(e, "Failed to invoke OnExecuteRequestStep. .NET runtime: {DotNetRuntimeDescription}; IIS: {IisVersion}", - PlatformDetection.DotNetRuntimeDescription, HttpRuntime.IISVersion); - } - } - } - private void RestoreContextIfNeeded(HttpContextBase context) { string EventName() => Enum.GetName(typeof(RequestNotification), context.CurrentNotification); @@ -153,7 +202,6 @@ private void RestoreContextIfNeeded(HttpContextBase context) if (urlPath != null && ignoreUrls != null && WildcardMatcher.IsAnyMatch(ignoreUrls, urlPath)) return; - if (Agent.Instance == null) { _logger.Trace()? @@ -227,7 +275,6 @@ private string TryGetUrlPath(HttpContextBase context) private void OnExecuteRequestStep(HttpContextBase context, Action step) { - RestoreContextIfNeeded(context); step(); } @@ -380,7 +427,7 @@ private static void FillSampledTransactionContextRequest(HttpRequest request, IT { Socket = new Socket { RemoteAddress = request.UserHostAddress }, HttpVersion = GetHttpVersion(request.ServerVariables["SERVER_PROTOCOL"]), - Headers = _isCaptureHeadersEnabled ? ConvertHeaders(request.Unvalidated.Headers) : null + Headers = IsCaptureHeadersEnabled ? ConvertHeaders(request.Unvalidated.Headers) : null }; } @@ -561,7 +608,7 @@ private static void FillSampledTransactionContextResponse(HttpResponse response, { Finished = true, StatusCode = response.StatusCode, - Headers = _isCaptureHeadersEnabled ? ConvertHeaders(response.Headers) : null + Headers = IsCaptureHeadersEnabled ? ConvertHeaders(response.Headers) : null }; private void FillSampledTransactionContextUser(HttpContext context, ITransaction transaction) @@ -599,20 +646,6 @@ static string GetClaimWithFallbackValue(ClaimsPrincipal principal, string claimT _logger.Debug()?.Log("Captured user - {CapturedUser}", transaction.Context.User); } - private static bool InitOnceForAllInstancesUnderLock(string dbgInstanceName) => - InitOnceHelper.IfNotInited?.Init(() => - { - var agentComponents = CreateAgentComponents(dbgInstanceName); - Agent.Setup(agentComponents); - - if (!Agent.Instance.Configuration.Enabled) - return; - - _isCaptureHeadersEnabled = Agent.Instance.Configuration.CaptureHeaders; - - Agent.Instance.SubscribeIncludingAllDefaults(); - }) ?? false; - /// /// Compiles a delegate from a lambda expression to get a route's DataTokens property, /// which holds the precedence value. @@ -680,5 +713,7 @@ private Func CreateWebApiAttributeRouteTemplateGetter() return null; } + + public void Dispose() { } } } diff --git a/test/Elastic.Apm.Tests/HelpersTests/DbgInstanceNameGeneratorTests.cs b/test/Elastic.Apm.Tests/HelpersTests/DbgInstanceNameGeneratorTests.cs deleted file mode 100644 index d0c472004..000000000 --- a/test/Elastic.Apm.Tests/HelpersTests/DbgInstanceNameGeneratorTests.cs +++ /dev/null @@ -1,40 +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 Elastic.Apm.Helpers; -using FluentAssertions; -using Xunit; - -namespace Elastic.Apm.Tests.HelpersTests -{ - public class DbgInstanceNameGeneratorTests - { - [Fact] - public void AllDefaultsTest() - { - var generator = new DbgInstanceNameGenerator(); - generator.Generate().Should().Be("1"); - generator.Generate().Should().Be("2"); - generator.Generate().Should().Be("3"); - } - - [Fact] - public void CustomStartTest() - { - var generator = new DbgInstanceNameGenerator(23); - generator.Generate().Should().Be("23"); - generator.Generate().Should().Be("24"); - generator.Generate().Should().Be("25"); - } - - [Fact] - public void CustomPrefixTest() - { - var generator = new DbgInstanceNameGenerator(); - generator.Generate("PrefixA_").Should().Be("PrefixA_1"); - generator.Generate("PrefixB_").Should().Be("PrefixB_2"); - generator.Generate("PrefixC_").Should().Be("PrefixC_3"); - } - } -}