From aa8fd7ea4d1306a886c8059ca806d78b91eed019 Mon Sep 17 00:00:00 2001 From: Steve Gordon Date: Tue, 16 Apr 2024 16:26:32 +0100 Subject: [PATCH] Add support for `transaction_name_groups` and `use_path_as_transaction_name` (#2331) Add new config options and their implementation. --- docs/configuration.asciidoc | 49 +++++++++++++++++++ .../CentralConfig/CentralConfiguration.cs | 6 +++ .../DynamicConfigurationOption.cs | 12 ++++- .../RuntimeConfigurationSnapshot.cs | 5 ++ .../Config/AbstractConfigurationReader.cs | 26 ++++++++++ src/Elastic.Apm/Config/ConfigConsts.cs | 3 ++ src/Elastic.Apm/Config/ConfigurationOption.cs | 12 ++++- .../FallbackToEnvironmentConfigurationBase.cs | 8 +++ src/Elastic.Apm/Config/IConfiguration.cs | 1 - .../Config/IConfigurationReader.cs | 24 +++++++++ src/Elastic.Apm/Helpers/WildcardMatcher.cs | 1 - src/Elastic.Apm/Model/Transaction.cs | 21 ++++++++ .../WebRequestTransactionCreator.cs | 6 ++- .../ElasticApmModule.cs | 5 +- .../TestConfiguration.cs | 2 + .../MockConfiguration.cs | 6 ++- .../CentralConfigResponseParserTests.cs | 25 ++++++++++ test/Elastic.Apm.Tests/ConstructorTests.cs | 7 ++- .../TransactionGroupNamesTests.cs | 44 +++++++++++++++++ .../TransactionNameGroupsTests.cs | 47 ++++++++++++++++++ .../TransactionNameTests.cs | 1 + .../UsePathAsTransactionNameTests.cs | 40 +++++++++++++++ .../DistributedTracingAspNetCoreTests.cs | 1 + .../FailedRequestTests.cs | 9 ++-- 24 files changed, 346 insertions(+), 15 deletions(-) create mode 100644 test/Elastic.Apm.Tests/TransactionGroupNamesTests.cs create mode 100644 test/iis/Elastic.Apm.AspNetFullFramework.Tests/TransactionNameGroupsTests.cs create mode 100644 test/iis/Elastic.Apm.AspNetFullFramework.Tests/UsePathAsTransactionNameTests.cs diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index e4604ddfc..eb3d9678b 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -1074,6 +1074,28 @@ NOTE: All errors that are captured during a request to an ignored URL are still NOTE: Changing this configuration will overwrite the default value. +[float] +[[config-transaction-name-groups]] +==== `TransactionNameGroups` (added[1.27.0]) + +<> + +With this option, you can group transaction names that contain dynamic parts with a wildcard expression. For example, the pattern `GET /user/*/cart` would consolidate transactions, such as `GET /users/42/cart` and `GET /users/73/cart` into a single transaction name `GET /users/*/cart`, hence reducing the transaction name cardinality. + +This option supports the wildcard `*`, which matches zero or more characters. Examples: `GET /foo/*/bar/*/baz*``, `*foo*`. Matching is case insensitive by default. Prepending an element with (?-i) makes the matching case sensitive. + +[options="header"] +|============ +| Environment variable name | IConfiguration or Web.config key +| `ELASTIC_APM_TRANSACTION_NAME_GROUPS` | `ElasticApm:TransactionNameGroups` +|============ + +[options="header"] +|============ +| Default | Type +| `` | String +|============ + [float] [[config-use-elastic-apm-traceparent-header]] ==== `UseElasticTraceparentHeader` (added[1.3.0]) @@ -1094,6 +1116,31 @@ When this setting is `true`, the agent also adds the header `elasticapm-tracepar | `true` | Boolean |============ +[float] +[[config-use-path-as-transaction-name]] +==== `UsePathAsTransactionName` (added[1.27.0]) + +<> + +If set to `true`, transaction names of unsupported or partially-supported frameworks will be in the form of `$method $path` instead of just `$method unknown route`. + +WARNING: If your URLs contain path parameters like `/user/$userId`, +you should be very careful when enabling this flag, +as it can lead to an explosion of transaction groups. +Take a look at the <> option on how to mitigate this problem by grouping URLs together. + +[options="header"] +|============ +| Environment variable name | IConfiguration or Web.config key +| `ELASTIC_APM_USE_PATH_AS_TRANSACTION_NAME` | `ElasticApm:UsePathAsTransactionName` +|============ + +[options="header"] +|============ +| Default | Type +| `true` | Boolean +|============ + [float] [[config-use-windows-credentials]] ==== `UseWindowsCredentials` @@ -1368,10 +1415,12 @@ you must instead set the `LogLevel` for the internal APM logger under the `Loggi | <> | Yes | Stacktrace, Performance | <> | No | Core | <> | Yes | HTTP, Performance +| <> | Yes | HTTP | <> | Yes | Core, Performance | <> | Yes | Core, Performance | <> | Yes | HTTP, Performance | <> | No | HTTP +| <> | Yes | HTTP | <> | No | Reporter | <> | No | Reporter diff --git a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfiguration.cs b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfiguration.cs index b16526e1e..4a50c9b5e 100644 --- a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfiguration.cs +++ b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfiguration.cs @@ -54,6 +54,8 @@ public CentralConfiguration(IApmLogger logger, CentralConfigPayload configPayloa GetSimpleConfigurationValue(DynamicConfigurationOption.ExitSpanMinDuration, ParseExitSpanMinDuration); TraceContinuationStrategy = GetConfigurationValue(DynamicConfigurationOption.TraceContinuationStrategy, ParseTraceContinuationStrategy); + TransactionNameGroups = GetConfigurationValue(DynamicConfigurationOption.TransactionNameGroups, ParseTransactionNameGroups); + UsePathAsTransactionName = GetSimpleConfigurationValue(DynamicConfigurationOption.UsePathAsTransactionName, ParseUsePathAsTransactionName); } public string Description => $"Central configuration (ETag: `{ETag}')"; @@ -92,10 +94,14 @@ public CentralConfiguration(IApmLogger logger, CentralConfigPayload configPayloa internal IReadOnlyList TransactionIgnoreUrls { get; private set; } + internal IReadOnlyCollection TransactionNameGroups { get; private set; } + internal int? TransactionMaxSpans { get; private set; } internal double? TransactionSampleRate { get; private set; } + internal bool? UsePathAsTransactionName { get; private set; } + private CentralConfigurationKeyValue BuildKv(DynamicConfigurationOption option, string value) => new(option, value, Description); private T GetConfigurationValue(DynamicConfigurationOption option, Func parser) diff --git a/src/Elastic.Apm/BackendComm/CentralConfig/DynamicConfigurationOption.cs b/src/Elastic.Apm/BackendComm/CentralConfig/DynamicConfigurationOption.cs index 2e8d0a197..37901edcf 100644 --- a/src/Elastic.Apm/BackendComm/CentralConfig/DynamicConfigurationOption.cs +++ b/src/Elastic.Apm/BackendComm/CentralConfig/DynamicConfigurationOption.cs @@ -30,8 +30,10 @@ internal enum DynamicConfigurationOption StackTraceLimit = ConfigurationOption.StackTraceLimit, TraceContinuationStrategy = ConfigurationOption.TraceContinuationStrategy, TransactionIgnoreUrls = ConfigurationOption.TransactionIgnoreUrls, + TransactionNameGroups = ConfigurationOption.TransactionNameGroups, TransactionMaxSpans = ConfigurationOption.TransactionMaxSpans, TransactionSampleRate = ConfigurationOption.TransactionSampleRate, + UsePathAsTransactionName = ConfigurationOption.UsePathAsTransactionName } internal static class DynamicConfigurationExtensions @@ -64,6 +66,8 @@ public static ConfigurationOption ToConfigurationOption(this DynamicConfiguratio TransactionIgnoreUrls => ConfigurationOption.TransactionIgnoreUrls, TransactionMaxSpans => ConfigurationOption.TransactionMaxSpans, TransactionSampleRate => ConfigurationOption.TransactionSampleRate, + TransactionNameGroups => ConfigurationOption.TransactionNameGroups, + UsePathAsTransactionName => ConfigurationOption.UsePathAsTransactionName, _ => throw new ArgumentOutOfRangeException(nameof(dynamicOption), dynamicOption, null) }; @@ -75,7 +79,7 @@ public static ConfigurationOption ToConfigurationOption(this DynamicConfiguratio ConfigurationOption.CaptureHeaders => CaptureHeaders, ConfigurationOption.ExitSpanMinDuration => ExitSpanMinDuration, ConfigurationOption.IgnoreMessageQueues => IgnoreMessageQueues, - ConfigurationOption.LogLevel => DynamicConfigurationOption.LogLevel, + ConfigurationOption.LogLevel => LogLevel, ConfigurationOption.Recording => Recording, ConfigurationOption.SanitizeFieldNames => SanitizeFieldNames, ConfigurationOption.SpanCompressionEnabled => SpanCompressionEnabled, @@ -90,6 +94,8 @@ public static ConfigurationOption ToConfigurationOption(this DynamicConfiguratio ConfigurationOption.TransactionIgnoreUrls => TransactionIgnoreUrls, ConfigurationOption.TransactionMaxSpans => TransactionMaxSpans, ConfigurationOption.TransactionSampleRate => TransactionSampleRate, + ConfigurationOption.TransactionNameGroups => TransactionNameGroups, + ConfigurationOption.UsePathAsTransactionName => UsePathAsTransactionName, _ => null }; @@ -101,7 +107,7 @@ public static string ToJsonKey(this DynamicConfigurationOption dynamicOption) => CaptureHeaders => "capture_headers", ExitSpanMinDuration => "exit_span_min_duration", IgnoreMessageQueues => "ignore_message_queues", - DynamicConfigurationOption.LogLevel => "log_level", + LogLevel => "log_level", Recording => "recording", SanitizeFieldNames => "sanitize_field_names", SpanCompressionEnabled => "span_compression_enabled", @@ -116,6 +122,8 @@ public static string ToJsonKey(this DynamicConfigurationOption dynamicOption) => TransactionIgnoreUrls => "transaction_ignore_urls", TransactionMaxSpans => "transaction_max_spans", TransactionSampleRate => "transaction_sample_rate", + TransactionNameGroups => "transaction_name_groups", + UsePathAsTransactionName => "use_path_as_transaction_name", _ => throw new ArgumentOutOfRangeException(nameof(dynamicOption), dynamicOption, null) }; } diff --git a/src/Elastic.Apm/BackendComm/CentralConfig/RuntimeConfigurationSnapshot.cs b/src/Elastic.Apm/BackendComm/CentralConfig/RuntimeConfigurationSnapshot.cs index 8640058e3..0ee2f9ecc 100644 --- a/src/Elastic.Apm/BackendComm/CentralConfig/RuntimeConfigurationSnapshot.cs +++ b/src/Elastic.Apm/BackendComm/CentralConfig/RuntimeConfigurationSnapshot.cs @@ -117,12 +117,17 @@ public ConfigurationKeyValue Lookup(ConfigurationOption option) => public IReadOnlyList TransactionIgnoreUrls => _dynamicConfiguration?.TransactionIgnoreUrls ?? _mainConfiguration.TransactionIgnoreUrls; + public IReadOnlyCollection TransactionNameGroups => + _dynamicConfiguration?.TransactionNameGroups ?? _mainConfiguration.TransactionNameGroups; + public int TransactionMaxSpans => _dynamicConfiguration?.TransactionMaxSpans ?? _mainConfiguration.TransactionMaxSpans; public double TransactionSampleRate => _dynamicConfiguration?.TransactionSampleRate ?? _mainConfiguration.TransactionSampleRate; public bool UseElasticTraceparentHeader => _mainConfiguration.UseElasticTraceparentHeader; + public bool UsePathAsTransactionName => _dynamicConfiguration?.UsePathAsTransactionName ?? _mainConfiguration.UsePathAsTransactionName; + public bool VerifyServerCert => _mainConfiguration.VerifyServerCert; } } diff --git a/src/Elastic.Apm/Config/AbstractConfigurationReader.cs b/src/Elastic.Apm/Config/AbstractConfigurationReader.cs index 82e071bca..eb6a3819b 100644 --- a/src/Elastic.Apm/Config/AbstractConfigurationReader.cs +++ b/src/Elastic.Apm/Config/AbstractConfigurationReader.cs @@ -483,6 +483,29 @@ protected IReadOnlyList ParseTransactionIgnoreUrls(Configuratio } } + protected IReadOnlyCollection ParseTransactionNameGroups(ConfigurationKeyValue kv) + { + if (kv?.Value == null) + return DefaultValues.TransactionNameGroups; + + try + { + _logger?.Trace()?.Log("Try parsing TransactionNameGroups, values: {TransactionNameGroups}", kv.Value); + var transactionNameGroups = kv.Value.Split(',').Where(n => !string.IsNullOrEmpty(n)).ToList(); + + var retVal = new List(transactionNameGroups.Count); + foreach (var item in transactionNameGroups) + retVal.Add(WildcardMatcher.ValueOf(item.Trim())); + return retVal; + } + catch (Exception e) + { + _logger?.Error() + ?.LogException(e, "Failed parsing TransactionNameGroups, values in the config: {TransactionNameGroupsValues}", kv.Value); + return DefaultValues.TransactionNameGroups; + } + } + protected bool ParseSpanCompressionEnabled(ConfigurationKeyValue kv) { if (kv == null || string.IsNullOrEmpty(kv.Value)) @@ -980,6 +1003,9 @@ protected int ParseTransactionMaxSpans(ConfigurationKeyValue kv) return DefaultValues.TransactionMaxSpans; } + protected bool ParseUsePathAsTransactionName(ConfigurationKeyValue kv) => + ParseBoolOption(kv, DefaultValues.UsePathAsTransactionName, "UsePathAsTransactionName"); + internal static bool IsMsOrElastic(byte[] array) { var elasticToken = new byte[] { 174, 116, 0, 210, 193, 137, 207, 34 }; diff --git a/src/Elastic.Apm/Config/ConfigConsts.cs b/src/Elastic.Apm/Config/ConfigConsts.cs index 36671af72..4d107d6dd 100644 --- a/src/Elastic.Apm/Config/ConfigConsts.cs +++ b/src/Elastic.Apm/Config/ConfigConsts.cs @@ -51,6 +51,7 @@ public static class DefaultValues public const double TransactionSampleRate = 1.0; public const string UnknownServiceName = "unknown-" + Consts.AgentName + "-service"; public const bool UseElasticTraceparentHeader = true; + public const bool UsePathAsTransactionName = true; public const bool VerifyServerCert = true; public const string TraceContinuationStrategy = "continue"; @@ -77,6 +78,8 @@ public static class DefaultValues public static readonly IReadOnlyList TransactionIgnoreUrls; + public static readonly IReadOnlyCollection TransactionNameGroups = new List().AsReadOnly(); + static DefaultValues() { var sanitizeFieldNames = new List(); diff --git a/src/Elastic.Apm/Config/ConfigurationOption.cs b/src/Elastic.Apm/Config/ConfigurationOption.cs index 37acf2693..9a1057e5f 100644 --- a/src/Elastic.Apm/Config/ConfigurationOption.cs +++ b/src/Elastic.Apm/Config/ConfigurationOption.cs @@ -90,10 +90,14 @@ public enum ConfigurationOption TransactionIgnoreUrls, /// TransactionMaxSpans, + /// + TransactionNameGroups, /// TransactionSampleRate, /// UseElasticTraceparentHeader, + /// + UsePathAsTransactionName, /// VerifyServerCert, /// @@ -180,11 +184,13 @@ public static string ToEnvironmentVariable(this ConfigurationOption option) => TraceContinuationStrategy => EnvPrefix + "TRACE_CONTINUATION_STRATEGY", TransactionIgnoreUrls => EnvPrefix + "TRANSACTION_IGNORE_URLS", TransactionMaxSpans => EnvPrefix + "TRANSACTION_MAX_SPANS", + TransactionNameGroups => EnvPrefix + "TRANSACTION_NAME_GROUPS", TransactionSampleRate => EnvPrefix + "TRANSACTION_SAMPLE_RATE", UseElasticTraceparentHeader => EnvPrefix + "USE_ELASTIC_TRACEPARENT_HEADER", + UsePathAsTransactionName => EnvPrefix + "USE_PATH_AS_TRANSACTION_NAME", VerifyServerCert => EnvPrefix + "VERIFY_SERVER_CERT", FullFrameworkConfigurationReaderType => EnvPrefix + "FULL_FRAMEWORK_CONFIGURATION_READER_TYPE", - _ => throw new System.ArgumentOutOfRangeException(nameof(option), option, null) + _ => throw new ArgumentOutOfRangeException(nameof(option), option, null) }; public static string ToConfigKey(this ConfigurationOption option) => @@ -229,14 +235,16 @@ public static string ToConfigKey(this ConfigurationOption option) => TraceContinuationStrategy => KeyPrefix + nameof(TraceContinuationStrategy), TransactionIgnoreUrls => KeyPrefix + nameof(TransactionIgnoreUrls), TransactionMaxSpans => KeyPrefix + nameof(TransactionMaxSpans), + TransactionNameGroups => KeyPrefix + nameof(TransactionNameGroups), TransactionSampleRate => KeyPrefix + nameof(TransactionSampleRate), UseElasticTraceparentHeader => KeyPrefix + nameof(UseElasticTraceparentHeader), + UsePathAsTransactionName => KeyPrefix + nameof(UsePathAsTransactionName), VerifyServerCert => KeyPrefix + nameof(VerifyServerCert), ServerUrls => KeyPrefix + nameof(ServerUrls), SpanFramesMinDuration => KeyPrefix + nameof(SpanFramesMinDuration), TraceContextIgnoreSampledFalse => KeyPrefix + nameof(TraceContextIgnoreSampledFalse), FullFrameworkConfigurationReaderType => KeyPrefix + nameof(FullFrameworkConfigurationReaderType), - _ => throw new System.ArgumentOutOfRangeException(nameof(option), option, null) + _ => throw new ArgumentOutOfRangeException(nameof(option), option, null) }; } } diff --git a/src/Elastic.Apm/Config/FallbackToEnvironmentConfigurationBase.cs b/src/Elastic.Apm/Config/FallbackToEnvironmentConfigurationBase.cs index 65dc4a54a..5fc3c0b17 100644 --- a/src/Elastic.Apm/Config/FallbackToEnvironmentConfigurationBase.cs +++ b/src/Elastic.Apm/Config/FallbackToEnvironmentConfigurationBase.cs @@ -130,10 +130,14 @@ IConfigurationEnvironmentValueProvider environmentValueProvider ParseTraceContinuationStrategy(Lookup(ConfigurationOption.TraceContinuationStrategy)); TransactionIgnoreUrls = ParseTransactionIgnoreUrls(Lookup(ConfigurationOption.TransactionIgnoreUrls)); + TransactionNameGroups = + ParseTransactionNameGroups(Lookup(ConfigurationOption.TransactionNameGroups)); TransactionMaxSpans = ParseTransactionMaxSpans(Lookup(ConfigurationOption.TransactionMaxSpans)); TransactionSampleRate = ParseTransactionSampleRate(Lookup(ConfigurationOption.TransactionSampleRate)); UseElasticTraceparentHeader = ParseUseElasticTraceparentHeader(Lookup(ConfigurationOption.UseElasticTraceparentHeader)); + UsePathAsTransactionName = + ParseUsePathAsTransactionName(Lookup(ConfigurationOption.UsePathAsTransactionName)); VerifyServerCert = ParseVerifyServerCert(Lookup(ConfigurationOption.VerifyServerCert)); var urlConfig = Lookup(ConfigurationOption.ServerUrl); @@ -237,12 +241,16 @@ public ConfigurationKeyValue Lookup(ConfigurationOption option) => public IReadOnlyList TransactionIgnoreUrls { get; } + public IReadOnlyCollection TransactionNameGroups { get; } + public int TransactionMaxSpans { get; } public double TransactionSampleRate { get; } public bool UseElasticTraceparentHeader { get; } + public bool UsePathAsTransactionName { get; } + public bool VerifyServerCert { get; } public bool OpenTelemetryBridgeEnabled { get; } diff --git a/src/Elastic.Apm/Config/IConfiguration.cs b/src/Elastic.Apm/Config/IConfiguration.cs index 428956ba7..8a1cccf1c 100644 --- a/src/Elastic.Apm/Config/IConfiguration.cs +++ b/src/Elastic.Apm/Config/IConfiguration.cs @@ -12,5 +12,4 @@ namespace Elastic.Apm.Config public interface IConfiguration : IConfigurationReader { } - } diff --git a/src/Elastic.Apm/Config/IConfigurationReader.cs b/src/Elastic.Apm/Config/IConfigurationReader.cs index 75ef44879..34f8b79c1 100644 --- a/src/Elastic.Apm/Config/IConfigurationReader.cs +++ b/src/Elastic.Apm/Config/IConfigurationReader.cs @@ -374,6 +374,24 @@ public interface IConfigurationReader : IConfigurationDescription, IConfiguratio /// IReadOnlyList TransactionIgnoreUrls { get; } + /// + /// A list of patterns to be used to group incoming HTTP server transactions by matching names contain dynamic parts + /// to a more suitable route name. + /// + /// + /// This setting can be particularly useful in scenarios where the APM agent is unable to determine a suitable + /// transaction name for a request. For example, in ASP.NET Core, we can leverage the routing information to + /// provide sensible transaction names and avoid high-cardinality. In other frameworks, such as WCF, there is no + /// such suitable available. In that case, the APM agent will use the request path in the transaction name, which + /// can lead to a high-cardinality problem. By using this setting, you can group similar transactions together. + /// + /// For example, the pattern 'GET /user/*/cart' would consolidate transactions, such as `GET /users/42/cart` and + /// 'GET /users/73/cart' into a single transaction name 'GET /users/*/cart', hence reducing the transaction + /// name cardinality." + /// + /// + IReadOnlyCollection TransactionNameGroups { get; } + /// /// The number of spans that are recorded per transaction. /// @@ -408,6 +426,12 @@ public interface IConfigurationReader : IConfigurationDescription, IConfiguratio /// bool UseElasticTraceparentHeader { get; } + /// + /// If true, the default, the agent will use the path of the incoming HTTP request as the transaction name in situations + /// when a more accurate route name cannot be determined from route data or request headers. + /// + bool UsePathAsTransactionName { get; } + /// /// The agent verifies the server's certificate if an HTTPS connection to the APM server is used. /// Verification can be disabled by setting to false. diff --git a/src/Elastic.Apm/Helpers/WildcardMatcher.cs b/src/Elastic.Apm/Helpers/WildcardMatcher.cs index b02b4d147..ea5f5f744 100644 --- a/src/Elastic.Apm/Helpers/WildcardMatcher.cs +++ b/src/Elastic.Apm/Helpers/WildcardMatcher.cs @@ -86,7 +86,6 @@ public static WildcardMatcher ValueOf(string wildcardString) return new CompoundWildcardMatcher(matcher, matchers); } - /// /// Returns true, if any of the matchers match the provided string. /// diff --git a/src/Elastic.Apm/Model/Transaction.cs b/src/Elastic.Apm/Model/Transaction.cs index 659f24c6b..680b167e2 100644 --- a/src/Elastic.Apm/Model/Transaction.cs +++ b/src/Elastic.Apm/Model/Transaction.cs @@ -697,7 +697,28 @@ public void End() } if (IsSampled || _apmServerInfo?.Version < new ElasticVersion(8, 0, 0, string.Empty)) + { + // Apply any transaction name groups + if (Configuration.TransactionNameGroups.Count > 0) + { + var matched = WildcardMatcher.AnyMatch(Configuration.TransactionNameGroups, Name, null); + if (matched is not null) + { + var matchedTransactionNameGroup = matched.GetMatcher(); + + if (!string.IsNullOrEmpty(matchedTransactionNameGroup)) + { + _logger?.Trace()?.Log("Transaction name '{TransactionName}' matched transaction " + + "name group '{TransactionNameGroup}' from configuration", + Name, matchedTransactionNameGroup); + + Name = matchedTransactionNameGroup; + } + } + } + _sender.QueueTransaction(this); + } else { _logger?.Debug() diff --git a/src/integrations/Elastic.Apm.AspNetCore/WebRequestTransactionCreator.cs b/src/integrations/Elastic.Apm.AspNetCore/WebRequestTransactionCreator.cs index b8ad13eb8..36adeeba1 100644 --- a/src/integrations/Elastic.Apm.AspNetCore/WebRequestTransactionCreator.cs +++ b/src/integrations/Elastic.Apm.AspNetCore/WebRequestTransactionCreator.cs @@ -36,8 +36,12 @@ internal static ITransaction StartTransactionAsync(HttpContext context, IApmLogg return null; } + // For completeness we set the initial transaction name based on the config. + // I don't believe there are any valid scenarios where this will not be overwritten later. ITransaction transaction; - var transactionName = $"{context.Request.Method} {context.Request.Path}"; + var transactionName = configuration?.UsePathAsTransactionName ?? ConfigConsts.DefaultValues.UsePathAsTransactionName + ? $"{context.Request.Method} {context.Request.Path}" + : $"{context.Request.Method} unknown route"; var containsTraceParentHeader = context.Request.Headers.TryGetValue(TraceContext.TraceParentHeaderName, out var traceParentHeader); diff --git a/src/integrations/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs b/src/integrations/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs index 0bf85cac8..ef767aa47 100644 --- a/src/integrations/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs +++ b/src/integrations/Elastic.Apm.AspNetFullFramework/ElasticApmModule.cs @@ -348,7 +348,10 @@ private void ProcessBeginRequest(object sender) return; } - var transactionName = $"{request.HttpMethod} {request.Unvalidated.Path}"; + // Set the initial transaction name based on the request path, if enabled in configuration (default is true). + var transactionName = Agent.Instance.Configuration.UsePathAsTransactionName + ? $"{request.HttpMethod} {request.Unvalidated.Path}" + : $"{request.HttpMethod} unknown route"; var distributedTracingData = ExtractIncomingDistributedTracingData(request); ITransaction transaction; diff --git a/test/Elastic.Apm.Feature.Tests/TestConfiguration.cs b/test/Elastic.Apm.Feature.Tests/TestConfiguration.cs index d31cde786..b6ea5cee0 100644 --- a/test/Elastic.Apm.Feature.Tests/TestConfiguration.cs +++ b/test/Elastic.Apm.Feature.Tests/TestConfiguration.cs @@ -66,8 +66,10 @@ public class TestConfiguration : IConfiguration public string TraceContinuationStrategy { get; } = DefaultValues.TraceContinuationStrategy; public IReadOnlyList TransactionIgnoreUrls { get; set; } = DefaultValues.TransactionIgnoreUrls; public int TransactionMaxSpans { get; set; } = DefaultValues.TransactionMaxSpans; + public IReadOnlyCollection TransactionNameGroups { get; set; } = DefaultValues.TransactionNameGroups; public double TransactionSampleRate { get; set; } = DefaultValues.TransactionSampleRate; public bool UseElasticTraceparentHeader { get; set; } = DefaultValues.UseElasticTraceparentHeader; + public bool UsePathAsTransactionName { get; set; } = DefaultValues.UsePathAsTransactionName; public bool VerifyServerCert { get; set; } = DefaultValues.VerifyServerCert; public bool OpenTelemetryBridgeEnabled { get; set; } diff --git a/test/Elastic.Apm.Tests.Utilities/MockConfiguration.cs b/test/Elastic.Apm.Tests.Utilities/MockConfiguration.cs index 81fae3541..cdd499828 100644 --- a/test/Elastic.Apm.Tests.Utilities/MockConfiguration.cs +++ b/test/Elastic.Apm.Tests.Utilities/MockConfiguration.cs @@ -68,7 +68,9 @@ public MockConfiguration(IApmLogger logger = null, string spanCompressionEnabled = null, string spanCompressionExactMatchMaxDuration = null, string spanCompressionSameKindMaxDuration = null, - string traceContinuationStrategy = null + string traceContinuationStrategy = null, + string transactionNameGroups = null, + string usePathAsTransactionName = null ) : base( logger, new ConfigurationDefaults { DebugName = nameof(MockConfiguration) }, @@ -116,9 +118,11 @@ public MockConfiguration(IApmLogger logger = null, ConfigurationOption.TraceContextIgnoreSampledFalse => traceContextIgnoreSampledFalse, ConfigurationOption.TraceContinuationStrategy => traceContinuationStrategy, ConfigurationOption.TransactionIgnoreUrls => transactionIgnoreUrls, + ConfigurationOption.TransactionNameGroups => transactionNameGroups, ConfigurationOption.TransactionMaxSpans => transactionMaxSpans, ConfigurationOption.TransactionSampleRate => transactionSampleRate, ConfigurationOption.UseElasticTraceparentHeader => useElasticTraceparentHeader, + ConfigurationOption.UsePathAsTransactionName => usePathAsTransactionName, ConfigurationOption.VerifyServerCert => verifyServerCert, ConfigurationOption.FullFrameworkConfigurationReaderType => null, _ => throw new Exception($"{nameof(MockConfiguration)} does not have implementation for configuration : {key}") diff --git a/test/Elastic.Apm.Tests/BackendCommTests/CentralConfig/CentralConfigResponseParserTests.cs b/test/Elastic.Apm.Tests/BackendCommTests/CentralConfig/CentralConfigResponseParserTests.cs index 044fd9fca..a0b8ed89a 100644 --- a/test/Elastic.Apm.Tests/BackendCommTests/CentralConfig/CentralConfigResponseParserTests.cs +++ b/test/Elastic.Apm.Tests/BackendCommTests/CentralConfig/CentralConfigResponseParserTests.cs @@ -169,6 +169,8 @@ public void ParseHttpResponse_ShouldReturnEmptyConfigDelta_WhenResponseBodyIsEmp configDelta.LogLevel.Should().BeNull(); configDelta.SpanFramesMinDurationInMilliseconds.Should().BeNull(); configDelta.StackTraceLimit.Should().BeNull(); + configDelta.TransactionNameGroups.Should().BeNull(); + configDelta.UsePathAsTransactionName.Should().BeNull(); } [Fact] @@ -307,6 +309,29 @@ public static IEnumerable ConfigDeltaData }) }; } + + yield return new object[] + { + $"{{\"{DynamicConfigurationOption.UsePathAsTransactionName.ToJsonKey()}\": \"{true}\"}}", + new Action(cfg => + { + cfg.UsePathAsTransactionName.Should() + .NotBeNull() + .And.Be(true); + }) + }; + + var transactionNameGroups = "GET /customers/*, GET /orders/*"; + yield return new object[] + { + $"{{\"{DynamicConfigurationOption.TransactionNameGroups.ToJsonKey()}\": \"{transactionNameGroups}\"}}", + new Action(cfg => + { + cfg.TransactionNameGroups.Should() + .NotBeNull() + .And.HaveCount(2); + }) + }; } } diff --git a/test/Elastic.Apm.Tests/ConstructorTests.cs b/test/Elastic.Apm.Tests/ConstructorTests.cs index a897569de..f9576ad5e 100644 --- a/test/Elastic.Apm.Tests/ConstructorTests.cs +++ b/test/Elastic.Apm.Tests/ConstructorTests.cs @@ -149,8 +149,13 @@ private class LogConfiguration : IConfiguration, IConfigurationDescription public bool UseElasticTraceparentHeader => ConfigConsts.DefaultValues.UseElasticTraceparentHeader; public int TransactionMaxSpans => ConfigConsts.DefaultValues.TransactionMaxSpans; - // ReSharper restore UnassignedGetOnlyAutoProperty + public IReadOnlyCollection TransactionNameGroups => + ConfigConsts.DefaultValues.TransactionNameGroups; + + public bool UsePathAsTransactionName => ConfigConsts.DefaultValues.UsePathAsTransactionName; + + // ReSharper restore UnassignedGetOnlyAutoProperty public ConfigurationKeyValue Lookup(ConfigurationOption option) => null; } } diff --git a/test/Elastic.Apm.Tests/TransactionGroupNamesTests.cs b/test/Elastic.Apm.Tests/TransactionGroupNamesTests.cs new file mode 100644 index 000000000..145a7b619 --- /dev/null +++ b/test/Elastic.Apm.Tests/TransactionGroupNamesTests.cs @@ -0,0 +1,44 @@ +// 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.Linq; +using Elastic.Apm.Tests.Utilities; +using FluentAssertions; +using Xunit; + +namespace Elastic.Apm.Tests; + +public class TransactionGroupNamesTests +{ + [Fact] + public void TransactionGroupNames_AreApplied() + { + const string groupName = "GET /customer/*"; + + var payloadSender = new MockPayloadSender(); + + using var agent = new ApmAgent(new TestAgentComponents(payloadSender: payloadSender, + configuration: new MockConfiguration(transactionNameGroups: groupName))); + + agent.Tracer.CaptureTransaction("GET /order/1", "Test", + transaction => { }); + + payloadSender.WaitForTransactions(); + + payloadSender.Transactions.Count.Should().Be(1); + var transaction = payloadSender.Transactions.Single(); + transaction.Name.Should().Be("GET /order/1"); + + payloadSender.Clear(); + + agent.Tracer.CaptureTransaction("GET /customer/1", "Test", + transaction => { }); + + payloadSender.WaitForTransactions(); + payloadSender.Transactions.Count.Should().Be(1); + transaction = payloadSender.Transactions.Single(); + transaction.Name.Should().Be(groupName); + } +} diff --git a/test/iis/Elastic.Apm.AspNetFullFramework.Tests/TransactionNameGroupsTests.cs b/test/iis/Elastic.Apm.AspNetFullFramework.Tests/TransactionNameGroupsTests.cs new file mode 100644 index 000000000..d4c208afd --- /dev/null +++ b/test/iis/Elastic.Apm.AspNetFullFramework.Tests/TransactionNameGroupsTests.cs @@ -0,0 +1,47 @@ +// 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.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Elastic.Apm.Config; +using FluentAssertions; +using Xunit; +using Xunit.Abstractions; +using static Elastic.Apm.Config.ConfigurationOption; + +namespace Elastic.Apm.AspNetFullFramework.Tests; + +[Collection(Consts.AspNetFullFrameworkTestsCollection)] +public class TransactionNameGroupsTests : TestsBase +{ + // NOTE: The main situation where ASP.NET instrumentation may produce high-cardinality transaction names is when the application + // is using WCF. In such cases, the transaction name will default to being the path of the request. We can't easily spin up WCF in + // CI so this test simply ensures that any transaction name group configuration is working as expected by setting a transaction + // group name that matches the transaction name of a request that hits an MVC controller action from an Area. + + private const string GroupName = "GET MyArea/*"; + + public TransactionNameGroupsTests(ITestOutputHelper xUnitOutputHelper) + : base(xUnitOutputHelper, + envVarsToSetForSampleAppPool: new Dictionary + { + { TransactionNameGroups.ToEnvironmentVariable(), GroupName } + }) + { } + + [AspNetFullFrameworkFact] + public async Task TransactionName_ShouldUseMatchedTransactionGroupName() + { + var pathData = SampleAppUrlPaths.MyAreaHomePage; + await SendGetRequestToSampleAppAndVerifyResponse(pathData.Uri, pathData.StatusCode); + + await WaitAndCustomVerifyReceivedData(receivedData => + { + receivedData.Transactions.Count.Should().Be(1); + var transaction = receivedData.Transactions.Single(); + transaction.Name.Should().Be(GroupName); + }); + } +} diff --git a/test/iis/Elastic.Apm.AspNetFullFramework.Tests/TransactionNameTests.cs b/test/iis/Elastic.Apm.AspNetFullFramework.Tests/TransactionNameTests.cs index f43f7f0c7..9803e4d75 100644 --- a/test/iis/Elastic.Apm.AspNetFullFramework.Tests/TransactionNameTests.cs +++ b/test/iis/Elastic.Apm.AspNetFullFramework.Tests/TransactionNameTests.cs @@ -146,6 +146,7 @@ await WaitAndCustomVerifyReceivedData(receivedData => transaction.Name.Should().Be($"GET {AttributeRoutingWebApiController.RoutePrefix}/{AttributeRoutingWebApiController.RouteAmbiguous}"); }); } + [AspNetFullFrameworkFact] public async Task Name_Should_Be_Path_When_Webforms_Page() { diff --git a/test/iis/Elastic.Apm.AspNetFullFramework.Tests/UsePathAsTransactionNameTests.cs b/test/iis/Elastic.Apm.AspNetFullFramework.Tests/UsePathAsTransactionNameTests.cs new file mode 100644 index 000000000..56d1432fe --- /dev/null +++ b/test/iis/Elastic.Apm.AspNetFullFramework.Tests/UsePathAsTransactionNameTests.cs @@ -0,0 +1,40 @@ +// 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.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Elastic.Apm.Config; +using FluentAssertions; +using Xunit; +using Xunit.Abstractions; +using static Elastic.Apm.Config.ConfigurationOption; + +namespace Elastic.Apm.AspNetFullFramework.Tests; + +[Collection(Consts.AspNetFullFrameworkTestsCollection)] +public class UsePathAsTransactionNameTests : TestsBase +{ + public UsePathAsTransactionNameTests(ITestOutputHelper xUnitOutputHelper) + : base(xUnitOutputHelper, + envVarsToSetForSampleAppPool: new Dictionary + { + { UsePathAsTransactionName.ToEnvironmentVariable(), "false" } + }) + { } + + [AspNetFullFrameworkFact] + public async Task Name_ShouldBeUnknown_WhenWebformsPage_AndUsePathAsTransactionName_IsDisabled() + { + var pathData = SampleAppUrlPaths.WebformsPage; + await SendGetRequestToSampleAppAndVerifyResponse(pathData.Uri, pathData.StatusCode); + + await WaitAndCustomVerifyReceivedData(receivedData => + { + receivedData.Transactions.Count.Should().Be(1); + var transaction = receivedData.Transactions.Single(); + transaction.Name.Should().Be("GET unknown route"); + }); + } +} diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/DistributedTracingAspNetCoreTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/DistributedTracingAspNetCoreTests.cs index da9493a4f..46c806615 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/DistributedTracingAspNetCoreTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/DistributedTracingAspNetCoreTests.cs @@ -10,6 +10,7 @@ using System.Threading.Tasks; using Elastic.Apm.DistributedTracing; using Elastic.Apm.Tests.Utilities; +using Elastic.Apm.Tests.Utilities.XUnit; using FluentAssertions; using Xunit; using Xunit.Abstractions; diff --git a/test/integrations/Elastic.Apm.AspNetCore.Tests/FailedRequestTests.cs b/test/integrations/Elastic.Apm.AspNetCore.Tests/FailedRequestTests.cs index 22430179f..8c7cd076f 100644 --- a/test/integrations/Elastic.Apm.AspNetCore.Tests/FailedRequestTests.cs +++ b/test/integrations/Elastic.Apm.AspNetCore.Tests/FailedRequestTests.cs @@ -24,7 +24,7 @@ namespace Elastic.Apm.AspNetCore.Tests [Collection("DiagnosticListenerTest")] public class FailedRequestTests : IAsyncLifetime { - private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); + private readonly CancellationTokenSource _cancellationTokenSource = new(); private MockPayloadSender _payloadSender1; private ApmAgent _agent1; @@ -73,8 +73,8 @@ public Task InitializeAsync() /// Calls Home/TriggerError (which throws an exception) and makes sure the result and the outcome are captured /// /// - [Fact] - public async Task DistributedTraceAcross2Service() + [DisabledTestFact("Sometimes fails in CI with 'Expected _payloadSender1.Transactions.Count to be 1, but found 0.'")] + public async Task FailedRequest() { var client = new HttpClient(); var res = await client.GetAsync("http://localhost:5901/Home/TriggerError"); @@ -90,8 +90,7 @@ public async Task DistributedTraceAcross2Service() public async Task DisposeAsync() { _cancellationTokenSource.Cancel(); - await Task.WhenAll(_taskForApp1); - + await _taskForApp1; _agent1?.Dispose(); } }