From be232d8f02c90221db31a31207ab0e0af52d76fa Mon Sep 17 00:00:00 2001 From: Gergely Kalapos Date: Fri, 18 Dec 2020 21:07:37 +0100 Subject: [PATCH] Make transaction_ignore_urls central config compatible (#1087) * Make SanitizeFieldNames config live changeable through central config * Make TransactionIgnoreUrls centrally configurable * Update configuration.asciidoc Document that SanitizeFieldNames and TransactionIgnoreUrls are now central config compatible * Update CentralConfigResponseParser.cs --- docs/configuration.asciidoc | 8 ++- src/Elastic.Apm/Api/Request.cs | 1 + .../CentralConfig/CentralConfigFetcher.cs | 2 +- .../CentralConfig/CentralConfigReader.cs | 4 ++ .../CentralConfigResponseParser.cs | 36 +++++++--- .../Filters/TransactionIgnoreUrlsFilter.cs | 14 ++-- src/Elastic.Apm/Report/PayloadSenderV2.cs | 2 +- .../TransactionIgnoreUrlsTest.cs | 68 ++++++++++++++++++- .../Mocks/MockPayloadSenderWithFilters.cs | 2 +- 9 files changed, 110 insertions(+), 27 deletions(-) diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index e7ee2f685..0ac51d285 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -411,6 +411,8 @@ the latest {apm-app-ref}/agent-configuration.html[APM Agent configuration]. [[config-sanitize-field-names]] ==== `SanitizeFieldNames` (added[1.2]) +<> + Sometimes it is necessary to sanitize, i.e., remove, sensitive data sent to Elastic APM. This config accepts a list of wildcard patterns of field names which should be sanitized. These apply for example to HTTP headers and `application/x-www-form-urlencoded` data. @@ -808,6 +810,8 @@ NOTE: Setting this to `false` reduces memory allocations, network bandwidth and [[config-transaction-ignore-urls]] ==== `TransactionIgnoreUrls` (performance) +<> + [options="header"] |============ | Environment variable name | IConfiguration or Web.config key @@ -1014,7 +1018,7 @@ you must instead set the `LogLevel` for the internal APM logger under the `Loggi | <> | No | Reporter | <> | No | Reporter | <> | Yes | Core -| <> | No | Core +| <> | Yes | Core | <> | No | Reporter | <> | No | Reporter | <> | No | Core @@ -1022,7 +1026,7 @@ you must instead set the `LogLevel` for the internal APM logger under the `Loggi | <> | No | Core | <> | No | Stacktrace, Performance | <> | No | Stacktrace, Performance -| <> | No | HTTP, Performance +| <> | Yes | HTTP, Performance | <> | Yes | Core, Performance | <> | Yes | Core, Performance | <> | No | HTTP diff --git a/src/Elastic.Apm/Api/Request.cs b/src/Elastic.Apm/Api/Request.cs index f417420ac..22453cfee 100644 --- a/src/Elastic.Apm/Api/Request.cs +++ b/src/Elastic.Apm/Api/Request.cs @@ -7,6 +7,7 @@ using System.Linq; using Elastic.Apm.Helpers; using Newtonsoft.Json; +using Newtonsoft.Json.Serialization; namespace Elastic.Apm.Api { diff --git a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs index 6eadabf02..d88f4a21f 100644 --- a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs +++ b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigFetcher.cs @@ -283,7 +283,7 @@ internal WrappingConfigSnapshot(IConfigSnapshot wrapped, CentralConfigReader cen _centralConfig.SpanFramesMinDurationInMilliseconds ?? _wrapped.SpanFramesMinDurationInMilliseconds; public int StackTraceLimit => _centralConfig.StackTraceLimit ?? _wrapped.StackTraceLimit; - public IReadOnlyList TransactionIgnoreUrls => _wrapped.TransactionIgnoreUrls; + public IReadOnlyList TransactionIgnoreUrls => _centralConfig.TransactionIgnoreUrls ?? _wrapped.TransactionIgnoreUrls; public int TransactionMaxSpans => _centralConfig.TransactionMaxSpans ?? _wrapped.TransactionMaxSpans; diff --git a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigReader.cs b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigReader.cs index 18f32721c..cff86567a 100644 --- a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigReader.cs +++ b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigReader.cs @@ -45,6 +45,8 @@ public CentralConfigReader(IApmLogger logger, CentralConfigResponseParser.Centra internal int? StackTraceLimit { get; private set; } + internal IReadOnlyList TransactionIgnoreUrls { get; private set; } + internal int? TransactionMaxSpans { get; private set; } internal double? TransactionSampleRate { get; private set; } @@ -67,6 +69,8 @@ private void UpdateConfigurationValues() Recording = GetSimpleConfigurationValue(CentralConfigResponseParser.CentralConfigPayload.Recording, ParseRecording); SanitizeFieldNames = GetConfigurationValue(CentralConfigResponseParser.CentralConfigPayload.SanitizeFieldNames, ParseSanitizeFieldNames); + TransactionIgnoreUrls = + GetConfigurationValue(CentralConfigResponseParser.CentralConfigPayload.TransactionIgnoreUrls, ParseTransactionIgnoreUrls); } private ConfigurationKeyValue BuildKv(string key, string value) => diff --git a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigResponseParser.cs b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigResponseParser.cs index 042c9927c..b304f6a4b 100644 --- a/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigResponseParser.cs +++ b/src/Elastic.Apm/BackendComm/CentralConfig/CentralConfigResponseParser.cs @@ -1,4 +1,5 @@ -// Licensed to Elasticsearch B.V under one or more agreements. +// 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 @@ -15,9 +16,8 @@ namespace Elastic.Apm.BackendComm.CentralConfig { internal class CentralConfigResponseParser : ICentralConfigResponseParser { - private readonly IApmLogger _logger; - internal static readonly TimeSpan WaitTimeIfNoCacheControlMaxAge = TimeSpan.FromMinutes(5); + private readonly IApmLogger _logger; internal CentralConfigResponseParser(IApmLogger logger) => _logger = logger?.Scoped(nameof(CentralConfigResponseParser)); @@ -136,30 +136,44 @@ internal class CentralConfigPayload { internal const string CaptureBodyContentTypesKey = "capture_body_content_types"; internal const string CaptureBodyKey = "capture_body"; - internal const string TransactionMaxSpansKey = "transaction_max_spans"; - internal const string TransactionSampleRateKey = "transaction_sample_rate"; internal const string CaptureHeadersKey = "capture_headers"; internal const string LogLevelKey = "log_level"; - internal const string SpanFramesMinDurationKey = "span_frames_min_duration"; - internal const string StackTraceLimitKey = "stack_trace_limit"; internal const string Recording = "recording"; internal const string SanitizeFieldNames = "sanitize_field_names"; + internal const string SpanFramesMinDurationKey = "span_frames_min_duration"; + internal const string StackTraceLimitKey = "stack_trace_limit"; + internal const string TransactionIgnoreUrls = "transaction_ignore_urls"; + internal const string TransactionMaxSpansKey = "transaction_max_spans"; + internal const string TransactionSampleRateKey = "transaction_sample_rate"; internal static readonly ISet SupportedOptions = new HashSet { - CaptureBodyKey, CaptureBodyContentTypesKey, TransactionMaxSpansKey, TransactionSampleRateKey, - CaptureHeadersKey, LogLevelKey, SpanFramesMinDurationKey, StackTraceLimitKey + CaptureBodyKey, + CaptureBodyContentTypesKey, + TransactionMaxSpansKey, + TransactionSampleRateKey, + CaptureHeadersKey, + LogLevelKey, + SpanFramesMinDurationKey, + StackTraceLimitKey }; private readonly IDictionary _keyValues; public CentralConfigPayload(IDictionary keyValues) => _keyValues = keyValues; + public string this[string key] + { + get + { + _keyValues.TryGetValue(key, out var val); + return val; + } + } + [JsonIgnore] public IEnumerable> UnknownKeys => _keyValues.Where(x => !SupportedOptions.Contains(x.Key)); - - public string this[string key] => _keyValues.ContainsKey(key) ? _keyValues[key] : null; } } } diff --git a/src/Elastic.Apm/Filters/TransactionIgnoreUrlsFilter.cs b/src/Elastic.Apm/Filters/TransactionIgnoreUrlsFilter.cs index b59990492..1b923e4db 100644 --- a/src/Elastic.Apm/Filters/TransactionIgnoreUrlsFilter.cs +++ b/src/Elastic.Apm/Filters/TransactionIgnoreUrlsFilter.cs @@ -4,7 +4,6 @@ // See the LICENSE file in the project root for more information using Elastic.Apm.Api; -using Elastic.Apm.Config; using Elastic.Apm.Helpers; using Elastic.Apm.Model; @@ -15,10 +14,6 @@ namespace Elastic.Apm.Filters /// internal class TransactionIgnoreUrlsFilter { - private readonly IConfigSnapshot _configSnapshot; - - public TransactionIgnoreUrlsFilter(IConfigSnapshot configSnapshot) => _configSnapshot = configSnapshot; - public ITransaction Filter(ITransaction transaction) { if (transaction is Transaction realTransaction) @@ -26,10 +21,13 @@ public ITransaction Filter(ITransaction transaction) // If there is no context, there is no URL either, therefore this transaction can't be filtered based on the URL. if (!realTransaction.IsContextCreated) return transaction; + + return WildcardMatcher.IsAnyMatch(realTransaction.ConfigSnapshot.TransactionIgnoreUrls, transaction.Context?.Request?.Url?.PathName) + ? null + : transaction; } - return WildcardMatcher.IsAnyMatch(_configSnapshot.TransactionIgnoreUrls, transaction?.Context?.Request?.Url?.PathName) - ? null - : transaction; + + return transaction; } } } diff --git a/src/Elastic.Apm/Report/PayloadSenderV2.cs b/src/Elastic.Apm/Report/PayloadSenderV2.cs index a69323087..68c8543dc 100644 --- a/src/Elastic.Apm/Report/PayloadSenderV2.cs +++ b/src/Elastic.Apm/Report/PayloadSenderV2.cs @@ -116,7 +116,7 @@ internal static void SetUpFilters(List> transac IConfigSnapshot configSnapshot, IApmServerInfo apmServerInfo, IApmLogger logger ) { - transactionFilters.Add(new TransactionIgnoreUrlsFilter(configSnapshot).Filter); + transactionFilters.Add(new TransactionIgnoreUrlsFilter().Filter); transactionFilters.Add(new HeaderDictionarySanitizerFilter().Filter); // with this, stack trace demystification and conversion to the intake API model happens on a non-application thread: spanFilters.Add(new SpanStackTraceCapturingFilter(logger, apmServerInfo).Filter); diff --git a/test/Elastic.Apm.AspNetCore.Tests/TransactionIgnoreUrlsTest.cs b/test/Elastic.Apm.AspNetCore.Tests/TransactionIgnoreUrlsTest.cs index d06f5d35b..9e8398496 100644 --- a/test/Elastic.Apm.AspNetCore.Tests/TransactionIgnoreUrlsTest.cs +++ b/test/Elastic.Apm.AspNetCore.Tests/TransactionIgnoreUrlsTest.cs @@ -1,4 +1,9 @@ -using System; +// 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.Net.Http; using System.Threading.Tasks; using Elastic.Apm.Extensions.Hosting; @@ -14,8 +19,6 @@ namespace Elastic.Apm.AspNetCore.Tests [Collection("DiagnosticListenerTest")] public class TransactionIgnoreUrlsTest : IClassFixture>, IDisposable { - private readonly ApmAgent _agent; - private readonly MockPayloadSender _capturedPayload; private readonly WebApplicationFactory _factory; // ReSharper disable once PrivateFieldCanBeConvertedToLocalVariable @@ -38,6 +41,9 @@ public TransactionIgnoreUrlsTest(WebApplicationFactory factory) _capturedPayload = _agent.PayloadSender as MockPayloadSender; } + private ApmAgent _agent; + private MockPayloadSender _capturedPayload; + private HttpClient _client; private void Setup(bool useOnlyDiagnosticSource) @@ -50,6 +56,62 @@ private void Setup(bool useOnlyDiagnosticSource) #endif } + /// + /// Changes the transactionIgnoreUrls during startup and asserts that the agent reacts accordingly. + /// + /// + /// + [InlineData(true)] + [InlineData(false)] + [Theory] + public async Task ChangeTransactionIgnoreUrlsAfterStart(bool useDiagnosticSourceOnly) + { + // Start with default config + var startConfigSnapshot = new MockConfigSnapshot(new NoopLogger()); + _capturedPayload = new MockPayloadSender(); + + var agentComponents = new TestAgentComponents( + _logger, + startConfigSnapshot, _capturedPayload, + new CurrentExecutionSegmentsContainer()); + + _agent = new ApmAgent(agentComponents); + _client = Helper.GetClient(_agent, _factory, useDiagnosticSourceOnly); + + _client.DefaultRequestHeaders.Add("foo", "bar"); + await _client.GetAsync("/Home/SimplePage"); + + _capturedPayload.Transactions.Should().ContainSingle(); + _capturedPayload.FirstTransaction.Context.Request.Url.Full.ToLower().Should().Contain("simplepage"); + + _capturedPayload.ResetTransactionTaskCompletionSource(); + + //change config to ignore urls with SimplePage + var updateConfigSnapshot = new MockConfigSnapshot( + new NoopLogger() + , transactionIgnoreUrls: "*SimplePage*" + ); + _agent.ConfigStore.CurrentSnapshot = updateConfigSnapshot; + + await _client.GetAsync("/Home/SimplePage"); + + //assert that no more transaction is captured - so still 1 captured transaction + _capturedPayload.Transactions.Should().ContainSingle(); + + _capturedPayload.ResetTransactionTaskCompletionSource(); + //update config again + updateConfigSnapshot = new MockConfigSnapshot( + new NoopLogger() + , transactionIgnoreUrls: "FooBar" + ); + _agent.ConfigStore.CurrentSnapshot = updateConfigSnapshot; + + await _client.GetAsync("/Home/SimplePage"); + + //assert that the number of captured transaction increased + _capturedPayload.Transactions.Count.Should().Be(2); + } + /// /// In the ctor we add `*SimplePage` to the ignoreUrl list. This test makes sure that /home/SimplePage is indeed ignored. /// diff --git a/test/Elastic.Apm.Tests/Mocks/MockPayloadSenderWithFilters.cs b/test/Elastic.Apm.Tests/Mocks/MockPayloadSenderWithFilters.cs index 287ce63de..eeb7e4a12 100644 --- a/test/Elastic.Apm.Tests/Mocks/MockPayloadSenderWithFilters.cs +++ b/test/Elastic.Apm.Tests/Mocks/MockPayloadSenderWithFilters.cs @@ -14,7 +14,7 @@ internal class MockPayloadSenderWithFilters : MockPayloadSender { private readonly List> _transactionFilters = new List>(); - public MockPayloadSenderWithFilters() => _transactionFilters.Add(new TransactionIgnoreUrlsFilter(new MockConfigSnapshot()).Filter); + public MockPayloadSenderWithFilters() => _transactionFilters.Add(new TransactionIgnoreUrlsFilter().Filter); public override void QueueTransaction(ITransaction transaction) {