From 8db993e785060e910b35fd38d2b3e8a15bb0e626 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Tue, 13 Sep 2022 12:34:33 -0700 Subject: [PATCH 01/29] Update existing text unrelated to dynamic sampling --- src/Sentry/TransactionSamplingContext.cs | 3 ++- test/Sentry.Tests/HubTests.cs | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Sentry/TransactionSamplingContext.cs b/src/Sentry/TransactionSamplingContext.cs index 1925dd74b3..513189a48b 100644 --- a/src/Sentry/TransactionSamplingContext.cs +++ b/src/Sentry/TransactionSamplingContext.cs @@ -3,7 +3,8 @@ namespace Sentry { /// - /// Context used by a dynamic sampler to determine whether a transaction should be sampled. + /// Context information passed into a function, + /// which can be used to determine whether a transaction should be sampled. /// public class TransactionSamplingContext { diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index 321cd0171e..73c1556759 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -627,7 +627,7 @@ public void StartTransaction_StaticSampling_AppropriateDistribution(float sample } [Fact] - public void StartTransaction_DynamicSampling_SampledIn() + public void StartTransaction_TracesSampler_SampledIn() { // Arrange var hub = new Hub(new SentryOptions @@ -644,7 +644,7 @@ public void StartTransaction_DynamicSampling_SampledIn() } [Fact] - public void StartTransaction_DynamicSampling_SampledOut() + public void StartTransaction_TracesSampler_SampledOut() { // Arrange var hub = new Hub(new SentryOptions @@ -661,7 +661,7 @@ public void StartTransaction_DynamicSampling_SampledOut() } [Fact] - public void StartTransaction_DynamicSampling_WithCustomContext_SampledIn() + public void StartTransaction_TracesSampler_WithCustomContext_SampledIn() { // Arrange var hub = new Hub(new SentryOptions @@ -680,7 +680,7 @@ public void StartTransaction_DynamicSampling_WithCustomContext_SampledIn() } [Fact] - public void StartTransaction_DynamicSampling_WithCustomContext_SampledOut() + public void StartTransaction_TracesSampler_WithCustomContext_SampledOut() { // Arrange var hub = new Hub(new SentryOptions @@ -699,7 +699,7 @@ public void StartTransaction_DynamicSampling_WithCustomContext_SampledOut() } [Fact] - public void StartTransaction_DynamicSampling_FallbackToStatic_SampledIn() + public void StartTransaction_TracesSampler_FallbackToStatic_SampledIn() { // Arrange var hub = new Hub(new SentryOptions @@ -717,7 +717,7 @@ public void StartTransaction_DynamicSampling_FallbackToStatic_SampledIn() } [Fact] - public void StartTransaction_DynamicSampling_FallbackToStatic_SampledOut() + public void StartTransaction_TracesSampler_FallbackToStatic_SampledOut() { // Arrange var hub = new Hub(new SentryOptions From ab7ad3d43388ab5dbf3e91868cc2534d987e2cca Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Tue, 27 Sep 2022 21:33:01 -0700 Subject: [PATCH 02/29] Add baggage header and DSC --- src/Sentry/BaggageHeader.cs | 229 ++++++++++++++++++ src/Sentry/DynamicSamplingContext.cs | 180 ++++++++++++++ src/Sentry/HubExtensions.cs | 10 + .../Extensions/CollectionsExtensions.cs | 4 + src/Sentry/Internal/Hub.cs | 22 +- src/Sentry/Internal/Polyfills.cs | 3 + src/Sentry/Protocol/Envelopes/Envelope.cs | 60 ++--- src/Sentry/SentryHttpMessageHandler.cs | 27 ++- src/Sentry/Transaction.cs | 19 +- src/Sentry/TransactionNameSource.cs | 15 ++ src/Sentry/TransactionTracer.cs | 13 +- 11 files changed, 536 insertions(+), 46 deletions(-) create mode 100644 src/Sentry/BaggageHeader.cs create mode 100644 src/Sentry/DynamicSamplingContext.cs diff --git a/src/Sentry/BaggageHeader.cs b/src/Sentry/BaggageHeader.cs new file mode 100644 index 0000000000..efbbd3db4d --- /dev/null +++ b/src/Sentry/BaggageHeader.cs @@ -0,0 +1,229 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using Sentry.Internal.Extensions; + +namespace Sentry +{ + /// + /// Baggage Header for dynamic sampling. + /// + /// + /// + /// + public class BaggageHeader + { + private readonly IDictionary _members; + internal const string HttpHeaderName = "baggage"; + private const string SentryKeyPrefix = "sentry-"; + + private BaggageHeader(IDictionary members) + { + _members = members; + } + + internal IReadOnlyDictionary GetRawMembers() => + _members.OrderBy(x => x.Key).ToDictionary(); + + internal IReadOnlyDictionary GetSentryMembers() => + _members + .Where(x => x.Key.StartsWith(SentryKeyPrefix)) + .OrderBy(x => x.Key) + .ToDictionary( +#if NETCOREAPP || NETSTANDARD2_1 + _ => _.Key[SentryKeyPrefix.Length..], +#else + _ => _.Key.Substring(SentryKeyPrefix.Length), +#endif + _ => Uri.UnescapeDataString(_.Value)); + + /// + /// Gets a value from the list members of the baggage header, if it exists. + /// + /// The key of the list member. + /// The value of the list member if found, or null otherwise. + public string? GetValue(string key) => _members.TryGetValue(key, out var value) + ? Uri.UnescapeDataString(value) + : null; + + /// + /// Sets a value for the a list member of the baggage header. + /// + /// The key of the list member. + /// The value of the list member. + /// + /// Only non-null members will be added to the list. + /// Attempting to set a null value will remove the member from the list if it exists. + /// Attempting to set a value that is already present in the list will overwrite the existing value. + /// + public void SetValue(string key, string? value) + { + if (IsValidKey(key)) + { + throw new ArgumentException("The provided key is invalid.", nameof(key)); + } + + SetValueInternal(key, value); + } + + private void SetValueInternal(string key, string? value) + { + if (value is null) + { + _members.Remove(key); + } + else + { + _members[key] = Uri.EscapeDataString(value); + } + } + + /// + /// Gets or sets the Sentry trace ID in the list members of the baggage header. + /// + public SentryId? SentryTraceId + { + get => _members.TryGetValue(SentryKeyPrefix + "trace_id", out var value) + ? Guid.TryParse(value, out var traceId) + ? new SentryId(traceId) + : null + : null; + set => SetValueInternal(SentryKeyPrefix + "trace_id", value?.ToString()); + } + + /// + /// Gets or sets the Sentry public key in the list members of the baggage header. + /// + public string? SentryPublicKey + { + get => GetValue(SentryKeyPrefix + "public_key"); + set => SetValueInternal(SentryKeyPrefix + "public_key", value); + } + + /// + /// Gets or sets the Sentry sample rate in the list members of the baggage header. + /// + public double? SentrySampleRate + { + get => _members.TryGetValue(SentryKeyPrefix + "sample_rate", out var value) + ? double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out var sampleRate) + ? sampleRate + : null + : null; + set => SetValueInternal(SentryKeyPrefix + "sample_rate", value?.ToString(CultureInfo.InvariantCulture)); + } + + /// + /// Gets or sets the Sentry release in the list members of the baggage header. + /// + public string? SentryRelease + { + get => GetValue(SentryKeyPrefix + "release"); + set => SetValueInternal(SentryKeyPrefix + "release", value); + } + + /// + /// Gets or sets the Sentry environment in the list members of the baggage header. + /// + public string? SentryEnvironment + { + get => GetValue(SentryKeyPrefix + "environment"); + set => SetValueInternal(SentryKeyPrefix + "environment", value); + } + + /// + /// Gets or sets the Sentry user segment in the list members of the baggage header. + /// + public string? SentryUserSegment + { + get => GetValue(SentryKeyPrefix + "user_segment"); + set => SetValueInternal(SentryKeyPrefix + "user_segment", value); + } + + /// + /// Gets or sets the Sentry transaction name in the list members of the baggage header. + /// + public string? SentryTransactionName + { + get => GetValue(SentryKeyPrefix + "transaction"); + set => SetValueInternal(SentryKeyPrefix + "transaction", value); + } + + /// + /// Creates the baggage header string based on the members of this instance. + /// + /// The baggage header string. + public override string ToString() + { + // the item keys do not require special encoding + // the item value are already encoded correctly, so we can just return them + var items = _members + .OrderBy(x => x.Key) + .Select(x => $"{x.Key}={x.Value}"); + return string.Join(", ", items); + } + + /// + /// Parses a baggage header string. + /// + /// The string to parse. + /// + /// When false, the resulting object includes all list members present in the baggage header string. + /// When true, the resulting object includes only members prefixed with "sentry-". + /// + /// + /// An object representing the members baggage header, or null if there are no members parsed. + /// + public static BaggageHeader? TryParse(string baggage, bool onlySentry = false) + { + // Example from W3C baggage spec: + // "key1=value1;property1;property2, key2 = value2, key3=value3; propertyKey=propertyValue" + + var items = baggage.Split(',', StringSplitOptions.RemoveEmptyEntries); + var resultItems = new Dictionary(items.Length, StringComparer.Ordinal); + + foreach (var item in items) + { + // Per baggage spec, the value may contain = characters, so limit the split to 2 parts. + var parts = item.Split('=', 2); + if (parts.Length != 2) + { + // malformed, missing separator, key, or value + continue; + } + + var key = parts[0].Trim(); + var value = parts[1].Trim(); + if (key.Length == 0 || value.Length == 0) + { + // malformed, key or value found empty + continue; + } + + if (!onlySentry || key.StartsWith(SentryKeyPrefix)) + { + resultItems.Add(key, value); + } + } + + return resultItems.Count == 0 ? null : new BaggageHeader(resultItems); + } + + private static bool IsValidKey(string key) + { + if (key.Length == 0) + { + return false; + } + + // The rules are the same as for HTTP headers. + // TODO: Is this public somewhere in .NET we can just call? + // https://www.w3.org/TR/baggage/#key + // https://www.rfc-editor.org/rfc/rfc7230#section-3.2.6 + // https://source.dot.net/#System.Net.Http/System/Net/Http/HttpRuleParser.cs,21 + const string delimiters = @"""(),/:;<=>?@[\]{}"; + return key.All(c => c >= 33 && c != 127 && !delimiters.Contains(c)); + } + } +} diff --git a/src/Sentry/DynamicSamplingContext.cs b/src/Sentry/DynamicSamplingContext.cs new file mode 100644 index 0000000000..2172c18118 --- /dev/null +++ b/src/Sentry/DynamicSamplingContext.cs @@ -0,0 +1,180 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using Sentry.Internal.Extensions; + +namespace Sentry +{ + /// + /// Provides the Dynamic Sampling Context for a transaction. + /// + /// + public class DynamicSamplingContext + { + // All values are stored in a dictionary, because it is possible to have other than those explicitly defined + // in this class. For example, the context can come from a baggage header with other Sentry- prefixed keys. + private readonly IReadOnlyDictionary _items; + + private DynamicSamplingContext(IReadOnlyDictionary items) => _items = items; + + /// + /// Gets an empty that can be used to "freeze" the DSC on a transaction. + /// + internal static readonly DynamicSamplingContext Empty = new(new Dictionary().AsReadOnly()); + + internal bool IsEmpty => _items.Count == 0; + + private DynamicSamplingContext( + SentryId traceId, + string publicKey, + double sampleRate, + string? release = null, + string? environment = null, + string? userSegment = null, + string? transactionName = null) + { + // Validate and set required fields + if (traceId == SentryId.Empty) + { + throw new ArgumentOutOfRangeException(nameof(traceId)); + } + + if (string.IsNullOrWhiteSpace(publicKey)) + { + throw new ArgumentException(default, nameof(publicKey)); + } + + if (sampleRate is < 0.0 or > 1.0) + { + throw new ArgumentOutOfRangeException(nameof(sampleRate)); + } + + var items = new Dictionary(capacity: 7) + { + ["trace_id"] = traceId.ToString(), + ["public_key"] = publicKey, + ["sample_rate"] = sampleRate.ToString(CultureInfo.InvariantCulture) + }; + + // Set optional fields + if (!string.IsNullOrWhiteSpace(release)) + { + items.Add("release", release); + } + + if (!string.IsNullOrWhiteSpace(environment)) + { + items.Add("environment", environment); + } + + if (!string.IsNullOrWhiteSpace(userSegment)) + { + items.Add("user_segment", userSegment); + } + + if (!string.IsNullOrWhiteSpace(transactionName)) + { + items.Add("transaction", transactionName); + } + + _items = items; + } + + /// + /// Gets the trace ID of the Dynamic Sampling Context. + /// + public SentryId TraceId => SentryId.Parse(_items["trace_id"]); + + /// + /// Gets the public key of the Dynamic Sampling Context. + /// + public string PublicKey => _items["public_key"]; + + /// + /// Gets the sample rate of the Dynamic Sampling Context. + /// + public double SampleRate => double.Parse(_items["sample_rate"], CultureInfo.InvariantCulture); + + /// + /// Gets the release of the Dynamic Sampling Context, or null if none was set. + /// + public string? Release => _items.TryGetValue("release", out var value) ? value : null; + + /// + /// Gets the environment of the Dynamic Sampling Context, or null if none was set. + /// + public string? Environment => _items.TryGetValue("environment", out var value) ? value : null; + + /// + /// Gets the user segment of the Dynamic Sampling Context, or null if none was set. + /// + public string? UserSegment => _items.TryGetValue("user_segment", out var value) ? value : null; + + /// + /// Gets the transaction name of the Dynamic Sampling Context, or null if none was set. + /// + public string? TransactionName => _items.TryGetValue("transaction", out var value) ? value : null; + + internal IReadOnlyDictionary GetItems() => _items; + + internal static DynamicSamplingContext? CreateFromBaggageHeader(BaggageHeader baggage) + { + var items = baggage.GetSentryMembers(); + + // The required items must exist and be valid to create the DSC from baggage. + // Return null if they are not, so we know we should create it from the transaction instead. + + if (!items.TryGetValue("trace_id", out var traceId) || + !Guid.TryParse(traceId, out var id) || id == Guid.Empty) + { + return null; + } + + if (!items.TryGetValue("public_key", out var publicKey) || string.IsNullOrWhiteSpace(publicKey)) + { + return null; + } + + if (!items.TryGetValue("sample_rate", out var sampleRate) || + !double.TryParse(sampleRate, NumberStyles.Float, CultureInfo.InvariantCulture, out var rate) || + rate is < 0.0 or > 1.0) + { + return null; + } + + return new DynamicSamplingContext(items); + } + + internal static DynamicSamplingContext CreateFromTransaction(TransactionTracer transaction, SentryOptions options) + { + // These should already be set on the transaction. + var publicKey = Dsn.Parse(options.Dsn!).PublicKey; + var traceId = transaction.TraceId; + var sampleRate = transaction.SampleRate!.Value; + var userSegment = transaction.User.Segment; + var transactionName = transaction.NameSource.IsHighQuality() ? transaction.Name : null; + + // These two may not have been set yet on the transaction, but we can get them directly. + var release = options.SettingLocator.GetRelease(); + var environment = options.SettingLocator.GetEnvironment(); + + return new DynamicSamplingContext( + traceId, + publicKey, + sampleRate, + release, + environment, + userSegment, + transactionName); + } + } + + internal static class DynamicSamplingContextExtensions + { + internal static DynamicSamplingContext? CreateDynamicSamplingContext(this BaggageHeader baggage) + => DynamicSamplingContext.CreateFromBaggageHeader(baggage); + + internal static DynamicSamplingContext CreateDynamicSamplingContext(this TransactionTracer transaction, SentryOptions options) + => DynamicSamplingContext.CreateFromTransaction(transaction, options); + } +} diff --git a/src/Sentry/HubExtensions.cs b/src/Sentry/HubExtensions.cs index 9eac22e9b5..2a66914f51 100644 --- a/src/Sentry/HubExtensions.cs +++ b/src/Sentry/HubExtensions.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.ComponentModel; using Sentry.Infrastructure; +using Sentry.Internal; using Sentry.Internal.Extensions; namespace Sentry @@ -191,5 +192,14 @@ public static SentryId CaptureMessage( Level = level }, configureScope); + + internal static ITransaction StartTransaction( + this IHub hub, + ITransactionContext context, + IReadOnlyDictionary customSamplingContext, + DynamicSamplingContext? dynamicSamplingContext) + => hub is Hub fullHub + ? fullHub.StartTransaction(context, customSamplingContext, dynamicSamplingContext) + : hub.StartTransaction(context, customSamplingContext); } } diff --git a/src/Sentry/Internal/Extensions/CollectionsExtensions.cs b/src/Sentry/Internal/Extensions/CollectionsExtensions.cs index 93f9c25792..56c9a5a10f 100644 --- a/src/Sentry/Internal/Extensions/CollectionsExtensions.cs +++ b/src/Sentry/Internal/Extensions/CollectionsExtensions.cs @@ -56,5 +56,9 @@ public static IEnumerable> Append( public static IReadOnlyList AsReadOnly(this IList list) => list as IReadOnlyList ?? new ReadOnlyCollection(list); + + public static IReadOnlyDictionary AsReadOnly(this IDictionary dictionary) + where TKey : notnull => + new ReadOnlyDictionary(dictionary); } } diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index ce4c9e0d73..2a1f1efcc8 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -119,6 +119,12 @@ public void WithScope(Action scopeCallback) public ITransaction StartTransaction( ITransactionContext context, IReadOnlyDictionary customSamplingContext) + => StartTransaction(context, customSamplingContext, null); + + internal ITransaction StartTransaction( + ITransactionContext context, + IReadOnlyDictionary customSamplingContext, + DynamicSamplingContext? dynamicSamplingContext) { var transaction = new TransactionTracer(this, context); @@ -133,16 +139,24 @@ public ITransaction StartTransaction( if (tracesSampler(samplingContext) is { } sampleRate) { transaction.IsSampled = _randomValuesFactory.NextBool(sampleRate); + transaction.SampleRate = sampleRate; } } - // Random sampling runs only if the sampling decision hasn't - // been made already. - transaction.IsSampled ??= _randomValuesFactory.NextBool(_options.TracesSampleRate); + // Random sampling runs only if the sampling decision hasn't been made already. + if (transaction.IsSampled == null) + { + transaction.IsSampled = _randomValuesFactory.NextBool(_options.TracesSampleRate); + transaction.SampleRate = _options.TracesSampleRate; + } + + // Use the provided DSC, or create one based on this transaction. + // This must be done AFTER the sampling decision has been made. + transaction.DynamicSamplingContext = + dynamicSamplingContext ?? transaction.CreateDynamicSamplingContext(_options); // A sampled out transaction still appears fully functional to the user // but will be dropped by the client and won't reach Sentry's servers. - return transaction; } diff --git a/src/Sentry/Internal/Polyfills.cs b/src/Sentry/Internal/Polyfills.cs index b56502b940..61055454ad 100644 --- a/src/Sentry/Internal/Polyfills.cs +++ b/src/Sentry/Internal/Polyfills.cs @@ -22,6 +22,9 @@ internal static partial class PolyfillExtensions public static string[] Split(this string str, char c, StringSplitOptions options = StringSplitOptions.None) => str.Split(new[] {c}, options); + public static string[] Split(this string str, char c, int count, StringSplitOptions options = StringSplitOptions.None) => + str.Split(new[] {c}, count, options); + public static bool Contains(this string str, char c) => str.IndexOf(c) >= 0; public static Task CopyToAsync(this Stream stream, Stream destination, CancellationToken cancellationToken) => diff --git a/src/Sentry/Protocol/Envelopes/Envelope.cs b/src/Sentry/Protocol/Envelopes/Envelope.cs index d5c5273a66..bd746f6bcb 100644 --- a/src/Sentry/Protocol/Envelopes/Envelope.cs +++ b/src/Sentry/Protocol/Envelopes/Envelope.cs @@ -17,10 +17,6 @@ namespace Sentry.Protocol.Envelopes /// public sealed class Envelope : ISerializable, IDisposable { - private const string SdkKey = "sdk"; - private const string EventIdKey = "event_id"; - private const string SentAtKey = "sent_at"; - /// /// Header associated with the envelope. /// @@ -44,7 +40,7 @@ public Envelope(IReadOnlyDictionary header, IReadOnlyList public SentryId? TryGetEventId() => - Header.TryGetValue(EventIdKey, out var value) && + Header.TryGetValue("event_id", out var value) && value is string valueString && Guid.TryParse(valueString, out var guid) ? new SentryId(guid) @@ -58,7 +54,7 @@ private async Task SerializeHeaderAsync( { // Append the sent_at header, except when writing to disk var headerItems = !stream.IsFileStream() - ? Header.Append(SentAtKey, clock.GetUtcNow()) + ? Header.Append("sent_at", clock.GetUtcNow()) : Header; var writer = new Utf8JsonWriter(stream); @@ -78,7 +74,7 @@ private void SerializeHeader(Stream stream, IDiagnosticLogger? logger, ISystemCl { // Append the sent_at header, except when writing to disk var headerItems = !stream.IsFileStream() - ? Header.Append(SentAtKey, clock.GetUtcNow()) + ? Header.Append("sent_at", clock.GetUtcNow()) : Header; using var writer = new Utf8JsonWriter(stream); @@ -133,30 +129,25 @@ internal void Serialize(Stream stream, IDiagnosticLogger? logger, ISystemClock c public void Dispose() => Items.DisposeAll(); // limited SDK information (no packages) - private static readonly IReadOnlyDictionary SdkHeader = new Dictionary(2, StringComparer.Ordinal) - { - ["name"] = SdkVersion.Instance.Name, - ["version"] = SdkVersion.Instance.Version - }; - - private static readonly IReadOnlyDictionary DefaultHeader = new Dictionary(1, StringComparer.Ordinal) - { - ["sdk"] = SdkHeader - }; + private static readonly IReadOnlyDictionary SdkHeader = + new Dictionary(2, StringComparer.Ordinal) + { + ["name"] = SdkVersion.Instance.Name, + ["version"] = SdkVersion.Instance.Version + }.AsReadOnly(); - private static IReadOnlyDictionary CreateHeader(SentryId? eventId = null) - { - if (eventId is null) + private static readonly IReadOnlyDictionary DefaultHeader = + new Dictionary(1, StringComparer.Ordinal) { - return DefaultHeader; - } + ["sdk"] = SdkHeader + }.AsReadOnly(); - return new Dictionary(2, StringComparer.Ordinal) + private static Dictionary CreateHeader(SentryId eventId, int extraCapacity = 0) => + new(2 + extraCapacity, StringComparer.Ordinal) { - [SdkKey] = SdkHeader, - [EventIdKey] = eventId.Value.ToString() + ["sdk"] = SdkHeader, + ["event_id"] = eventId.ToString() }; - } /// /// Creates an envelope that contains a single event. @@ -233,7 +224,16 @@ public static Envelope FromUserFeedback(UserFeedback sentryUserFeedback) /// public static Envelope FromTransaction(Transaction transaction) { - var header = CreateHeader(transaction.EventId); + Dictionary header; + if (transaction.DynamicSamplingContext is { } dsc) + { + header = CreateHeader(transaction.EventId, extraCapacity: 1); + header["trace"] = dsc.GetItems(); + } + else + { + header = CreateHeader(transaction.EventId); + } var items = new[] { @@ -248,7 +248,7 @@ public static Envelope FromTransaction(Transaction transaction) /// public static Envelope FromSession(SessionUpdate sessionUpdate) { - var header = CreateHeader(); + var header = DefaultHeader; var items = new[] { @@ -263,7 +263,7 @@ public static Envelope FromSession(SessionUpdate sessionUpdate) /// internal static Envelope FromClientReport(ClientReport clientReport) { - var header = CreateHeader(); + var header = DefaultHeader; var items = new[] { @@ -297,7 +297,7 @@ internal static Envelope FromClientReport(ClientReport clientReport) ?? throw new InvalidOperationException("Envelope header is malformed."); // The sent_at header should not be included in the result - header.Remove(SentAtKey); + header.Remove("sent_at"); return header; } diff --git a/src/Sentry/SentryHttpMessageHandler.cs b/src/Sentry/SentryHttpMessageHandler.cs index 35603eae8b..7cb088e734 100644 --- a/src/Sentry/SentryHttpMessageHandler.cs +++ b/src/Sentry/SentryHttpMessageHandler.cs @@ -49,12 +49,24 @@ protected override async Task SendAsync( CancellationToken cancellationToken) { // Set trace header if it hasn't already been set - if (!request.Headers.Contains(SentryTraceHeader.HttpHeaderName) && - _hub.GetTraceHeader() is { } traceHeader) + if (!request.Headers.Contains(SentryTraceHeader.HttpHeaderName) && _hub.GetTraceHeader() is { } traceHeader) { - request.Headers.Add( - SentryTraceHeader.HttpHeaderName, - traceHeader.ToString()); + request.Headers.Add(SentryTraceHeader.HttpHeaderName, traceHeader.ToString()); + } + + var transaction = _hub.GetSpan(); + + if (transaction is TransactionTracer {DynamicSamplingContext: {IsEmpty: false} dsc}) + { + // Set baggage header(s) + if (request.Headers.TryGetValues(BaggageHeader.HttpHeaderName, out var baggageHeaders)) + { + // TODO: merge baggage headers and add sentry's from the DSC + } + else + { + // TODO: Add Sentry's baggage header from DSC + } } // Prevent null reference exception in the following call @@ -66,7 +78,7 @@ protected override async Task SendAsync( // Start a span that tracks this request // (may be null if transaction is not set on the scope) - var span = _hub.GetSpan()?.StartChild( + var span = transaction?.StartChild( "http.client", // e.g. "GET https://example.com" $"{requestMethod} {url}"); @@ -84,8 +96,7 @@ protected override async Task SendAsync( _hub.AddBreadcrumb(string.Empty, "http", "http", breadcrumbData); // This will handle unsuccessful status codes as well - span?.Finish( - SpanStatusConverter.FromHttpStatusCode(response.StatusCode)); + span?.Finish(SpanStatusConverter.FromHttpStatusCode(response.StatusCode)); return response; } diff --git a/src/Sentry/Transaction.cs b/src/Sentry/Transaction.cs index f40d63cb37..5e571b0df6 100644 --- a/src/Sentry/Transaction.cs +++ b/src/Sentry/Transaction.cs @@ -96,10 +96,16 @@ public SpanStatus? Status public bool? IsSampled { get => Contexts.Trace.IsSampled; - // Internal for unit tests - internal set => Contexts.Trace.IsSampled = value; + internal set + { + Contexts.Trace.IsSampled = value; + SampleRate ??= value == null ? null : value.Value ? 1.0 : 0.0; + } } + /// + public double? SampleRate { get; internal set; } + /// public SentryLevel? Level { get; set; } @@ -181,6 +187,8 @@ public IReadOnlyList Fingerprint /// public bool IsFinished => EndTimestamp is not null; + internal DynamicSamplingContext? DynamicSamplingContext { get; set; } + // This constructor is used for deserialization purposes. // It's required because some of the fields are mapped on 'contexts.trace'. // When deserializing, we don't parse those fields explicitly, but @@ -247,6 +255,13 @@ public Transaction(ITransaction tracer) _extra = tracer.Extra.ToDictionary(); _tags = tracer.Tags.ToDictionary(); _spans = tracer.Spans.Select(s => new Span(s)).ToArray(); + + // Some items are not on the interface, but we only ever pass in a TransactionTracer anyway. + if (tracer is TransactionTracer transactionTracer) + { + SampleRate = transactionTracer.SampleRate; + DynamicSamplingContext = transactionTracer.DynamicSamplingContext; + } } /// diff --git a/src/Sentry/TransactionNameSource.cs b/src/Sentry/TransactionNameSource.cs index 8873bc318c..ccfecf6517 100644 --- a/src/Sentry/TransactionNameSource.cs +++ b/src/Sentry/TransactionNameSource.cs @@ -69,4 +69,19 @@ public enum TransactionNameSource /// Task } + + internal static class TransactionNameSourceExtensions + { + /// + /// Determines if the is considered "high quality" + /// for purposes of dynamic sampling. + /// + /// + /// Currently, only is considered low quality, + /// and the others are high quality, but this may change in the future. + /// + /// + public static bool IsHighQuality(this TransactionNameSource transactionNameSource) => + transactionNameSource != TransactionNameSource.Url; + } } diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index a93421554e..c52c5ae028 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -83,9 +83,16 @@ public SpanStatus? Status public bool? IsSampled { get => Contexts.Trace.IsSampled; - internal set => Contexts.Trace.IsSampled = value; + internal set + { + Contexts.Trace.IsSampled = value; + SampleRate ??= value == null ? null : value.Value ? 1.0 : 0.0; + } } + /// + public double? SampleRate { get; internal set; } + /// public SentryLevel? Level { get; set; } @@ -161,6 +168,8 @@ public IReadOnlyList Fingerprint /// public bool IsFinished => EndTimestamp is not null; + internal DynamicSamplingContext? DynamicSamplingContext { get; set; } + /// /// Initializes an instance of . /// @@ -183,7 +192,7 @@ public TransactionTracer(IHub hub, string name, string operation, TransactionNam } /// - /// Initializes an instance of . + /// Initializes an instance of . /// public TransactionTracer(IHub hub, ITransactionContext context) { From c42d1480836f878f7cbc9a5b37bc37d0de5ef4eb Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Tue, 27 Sep 2022 21:33:26 -0700 Subject: [PATCH 03/29] Update tracing middleware for ASP.NET Core --- .../SentryTracingMiddleware.cs | 55 +++++++++++++++---- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/src/Sentry.AspNetCore/SentryTracingMiddleware.cs b/src/Sentry.AspNetCore/SentryTracingMiddleware.cs index c6fd543b1b..cc312f8923 100644 --- a/src/Sentry.AspNetCore/SentryTracingMiddleware.cs +++ b/src/Sentry.AspNetCore/SentryTracingMiddleware.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Options; using Sentry.AspNetCore.Extensions; using Sentry.Extensibility; +using Sentry.Internal; namespace Sentry.AspNetCore { @@ -50,6 +51,27 @@ public SentryTracingMiddleware( } } + private BaggageHeader? TryGetBaggageHeader(HttpContext context) + { + var value = context.Request.Headers.GetValueOrDefault(BaggageHeader.HttpHeaderName); + if (string.IsNullOrWhiteSpace(value)) + { + return null; + } + + _options.LogDebug("Received baggage header '{0}'.", value); + + try + { + return BaggageHeader.TryParse(value, onlySentry: true); + } + catch (Exception ex) + { + _options.LogError("Invalid baggage header '{0}'.", ex, value); + return null; + } + } + private ITransaction? TryStartTransaction(HttpContext context) { if (context.Request.Method == HttpMethod.Options.Method) @@ -65,18 +87,14 @@ public SentryTracingMiddleware( // Attempt to start a transaction from the trace header if it exists var traceHeader = TryGetSentryTraceHeader(context); - // It's important to try and set the transaction name - // to some value here so that it's available for use - // in sampling. - // At a later stage, we will try to get the transaction name - // again, to account for the other middlewares that may have - // ran after ours. - var transactionName = - context.TryGetTransactionName(); + // It's important to try and set the transaction name to some value here so that it's available for use + // in sampling. At a later stage, we will try to get the transaction name again, to account for the + // other middlewares that may have ran after ours. + var transactionName = context.TryGetTransactionName() ?? string.Empty; var transactionContext = traceHeader is not null - ? new TransactionContext(transactionName ?? string.Empty, OperationName, traceHeader, TransactionNameSource.Route) - : new TransactionContext(transactionName ?? string.Empty, OperationName, TransactionNameSource.Route); + ? new TransactionContext(transactionName, OperationName, traceHeader, TransactionNameSource.Route) + : new TransactionContext(transactionName, OperationName, TransactionNameSource.Route); var customSamplingContext = new Dictionary(4, StringComparer.Ordinal) { @@ -86,7 +104,22 @@ public SentryTracingMiddleware( [SamplingExtensions.KeyForHttpContext] = context, }; - var transaction = hub.StartTransaction(transactionContext, customSamplingContext); + // Set the Dynamic Sampling Context from the baggage header, if it exists. + var baggageHeader = TryGetBaggageHeader(context); + var dynamicSamplingContext = baggageHeader?.CreateDynamicSamplingContext(); + + if (traceHeader is { } && baggageHeader is null) + { + // We received a sentry-trace header without a baggage header, which indicates the request + // originated from an older SDK that doesn't support dynamic sampling. + // Set DynamicSamplingContext.Empty to "freeze" the DSC on the transaction. + // See: + // https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#freezing-dynamic-sampling-context + // https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#unified-propagation-mechanism + dynamicSamplingContext = DynamicSamplingContext.Empty; + } + + var transaction = hub.StartTransaction(transactionContext, customSamplingContext, dynamicSamplingContext); _options.LogInfo( "Started transaction with span ID '{0}' and trace ID '{1}'.", From 532b720a5819f15efaeb3c05c7344794f0fdc057 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Tue, 27 Sep 2022 21:33:35 -0700 Subject: [PATCH 04/29] Update tests --- .../SentryTracingMiddlewareTests.cs | 87 ++++++++++++++++--- ...ebIntegrationTests.Versioning.verified.txt | 1 + .../ApiApprovalTests.Run.Core3_1.verified.txt | 26 ++++++ ...piApprovalTests.Run.DotNet4_8.verified.txt | 26 ++++++ ...piApprovalTests.Run.DotNet6_0.verified.txt | 26 ++++++ ...HeaderTests.Parse_FromExample.verified.txt | 15 ++++ ...BaggageHeaderTests.Parse_Full.verified.txt | 25 ++++++ test/Sentry.Tests/BaggageHeaderTests.cs | 53 +++++++++++ test/Sentry.Tests/SentryClientTests.cs | 1 + ...nsactionProcessorTests.Simple.verified.txt | 9 ++ .../Sentry.Tests/TransactionProcessorTests.cs | 3 +- 11 files changed, 257 insertions(+), 15 deletions(-) create mode 100644 test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt create mode 100644 test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt create mode 100644 test/Sentry.Tests/BaggageHeaderTests.cs diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs index 14828a495d..57d82882c0 100644 --- a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs @@ -1,5 +1,6 @@ #if NETCOREAPP3_1_OR_GREATER using System.Net.Http; +using System.Net.Http.Headers; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; @@ -8,6 +9,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Sentry.AspNetCore.Tests.Utils.Extensions; +using Sentry.Testing; namespace Sentry.AspNetCore.Tests; @@ -19,7 +21,7 @@ public async Task Transactions_are_grouped_by_route() // Arrange var sentryClient = Substitute.For(); - var hub = new Internal.Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); + var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); var server = new TestServer(new WebHostBuilder() .UseDefaultServiceProvider(di => di.EnableValidation()) @@ -67,7 +69,7 @@ public async Task Transaction_is_bound_on_the_scope_automatically() var sentryClient = Substitute.For(); - var hub = new Internal.Hub(new SentryOptions { Dsn = ValidDsn }, sentryClient); + var hub = new Hub(new SentryOptions { Dsn = ValidDsn }, sentryClient); var server = new TestServer(new WebHostBuilder() .UseDefaultServiceProvider(di => di.EnableValidation()) @@ -111,7 +113,7 @@ public async Task Transaction_is_started_automatically_from_incoming_trace_heade // Arrange var sentryClient = Substitute.For(); - var hub = new Internal.Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); + var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); var server = new TestServer(new WebHostBuilder() .UseDefaultServiceProvider(di => di.EnableValidation()) @@ -134,7 +136,13 @@ public async Task Transaction_is_started_automatically_from_incoming_trace_heade var client = server.CreateClient(); // Act - using var request = new HttpRequestMessage(HttpMethod.Get, "/person/13") { Headers = { { "sentry-trace", "75302ac48a024bde9a3b3734a82e36c8-1000000000000000-0" } } }; + using var request = new HttpRequestMessage(HttpMethod.Get, "/person/13") + { + Headers = + { + {"sentry-trace", "75302ac48a024bde9a3b3734a82e36c8-1000000000000000-0"} + } + }; await client.SendAsync(request); @@ -148,6 +156,63 @@ public async Task Transaction_is_started_automatically_from_incoming_trace_heade )); } + [Fact] + public async Task TraceID_from_trace_header_propagates_to_outbound_requests() + { + // Arrange + var sentryClient = Substitute.For(); + + var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); + + HttpRequestHeaders outboundRequestHeaders = null; + + var server = new TestServer(new WebHostBuilder() + .UseDefaultServiceProvider(di => di.EnableValidation()) + .UseSentry() + .ConfigureServices(services => + { + services.AddRouting(); + + services.RemoveAll(typeof(Func)); + services.AddSingleton>(() => hub); + }) + .Configure(app => + { + app.UseRouting(); + app.UseSentryTracing(); + + app.UseEndpoints(routes => routes.Map("/person/{id}", async _ => + { + // simulate an outbound request and capture the request headers + using var innerHandler = new RecordingHttpMessageHandler(new FakeHttpMessageHandler()); + using var sentryHandler = new SentryHttpMessageHandler(innerHandler, hub); + using var client = new HttpClient(sentryHandler); + await client.GetAsync("https://localhost/"); + using var request = innerHandler.GetRequests().Single(); + outboundRequestHeaders = request.Headers; + })); + })); + + var client = server.CreateClient(); + + // Act + using var request = new HttpRequestMessage(HttpMethod.Get, "/person/13") + { + Headers = + { + {"sentry-trace", "75302ac48a024bde9a3b3734a82e36c8-1000000000000000"} + } + }; + + await client.SendAsync(request); + + // Assert + Assert.NotNull(outboundRequestHeaders); + outboundRequestHeaders.Should().Contain(h => + h.Key == "sentry-trace" && + h.Value.First().StartsWith("75302ac48a024bde9a3b3734a82e36c8-")); + } + [Fact] public async Task Transaction_is_automatically_populated_with_request_data() { @@ -156,7 +221,7 @@ public async Task Transaction_is_automatically_populated_with_request_data() var sentryClient = Substitute.For(); - var hub = new Internal.Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); + var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); var server = new TestServer(new WebHostBuilder() .UseDefaultServiceProvider(di => di.EnableValidation()) @@ -206,7 +271,7 @@ public async Task Transaction_sampling_context_contains_HTTP_context_data() var sentryClient = Substitute.For(); - var hub = new Internal.Hub(new SentryOptions + var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampler = ctx => @@ -255,18 +320,12 @@ public async Task Transaction_sampling_context_contains_HTTP_context_data() public async Task Transaction_binds_exception_thrown() { // Arrange - TransactionSamplingContext samplingContext = null; - var sentryClient = Substitute.For(); - var hub = new Internal.Hub(new SentryOptions + var hub = new Hub(new SentryOptions { Dsn = ValidDsn, - TracesSampler = ctx => - { - samplingContext = ctx; - return 1; - } + TracesSampler = _ => 1.0 }, sentryClient); var exception = new Exception(); diff --git a/test/Sentry.AspNetCore.Tests/WebIntegrationTests.Versioning.verified.txt b/test/Sentry.AspNetCore.Tests/WebIntegrationTests.Versioning.verified.txt index 25fbce4393..cf72219b96 100644 --- a/test/Sentry.AspNetCore.Tests/WebIntegrationTests.Versioning.verified.txt +++ b/test/Sentry.AspNetCore.Tests/WebIntegrationTests.Versioning.verified.txt @@ -10,6 +10,7 @@ Description: , Status: Ok, IsSampled: true, + SampleRate: 1.0, Request: { Method: GET, QueryString: diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt index cf47a4a817..5b7a97b90b 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt @@ -18,6 +18,20 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, } + public class BaggageHeader + { + public string? SentryEnvironment { get; set; } + public string? SentryPublicKey { get; set; } + public string? SentryRelease { get; set; } + public double? SentrySampleRate { get; set; } + public Sentry.SentryId? SentryTraceId { get; set; } + public string? SentryTransactionName { get; set; } + public string? SentryUserSegment { get; set; } + public string? GetValue(string key) { } + public void SetValue(string key, string? value) { } + public override string ToString() { } + public static Sentry.BaggageHeader? TryParse(string baggage, bool onlySentry = false) { } + } [System.Diagnostics.DebuggerDisplay("Message: {Message}, Type: {Type}")] public sealed class Breadcrumb : Sentry.IJsonSerializable { @@ -109,6 +123,16 @@ namespace Sentry public DsnAttribute(string dsn) { } public string Dsn { get; } } + public class DynamicSamplingContext + { + public string? Environment { get; } + public string PublicKey { get; } + public string? Release { get; } + public double SampleRate { get; } + public Sentry.SentryId TraceId { get; } + public string? TransactionName { get; } + public string? UserSegment { get; } + } public static class EventLikeExtensions { public static bool HasUser(this Sentry.IEventLike eventLike) { } @@ -840,6 +864,7 @@ namespace Sentry public string? Platform { get; set; } public string? Release { get; set; } public Sentry.Request Request { get; set; } + public double? SampleRate { get; } public Sentry.SdkVersion Sdk { get; } public Sentry.SpanId SpanId { get; } public System.Collections.Generic.IReadOnlyCollection Spans { get; } @@ -909,6 +934,7 @@ namespace Sentry public string? Platform { get; set; } public string? Release { get; set; } public Sentry.Request Request { get; set; } + public double? SampleRate { get; } public Sentry.SdkVersion Sdk { get; } public Sentry.SpanId SpanId { get; } public System.Collections.Generic.IReadOnlyCollection Spans { get; } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt index fee2c93e5d..5950436015 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt @@ -18,6 +18,20 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, } + public class BaggageHeader + { + public string? SentryEnvironment { get; set; } + public string? SentryPublicKey { get; set; } + public string? SentryRelease { get; set; } + public double? SentrySampleRate { get; set; } + public Sentry.SentryId? SentryTraceId { get; set; } + public string? SentryTransactionName { get; set; } + public string? SentryUserSegment { get; set; } + public string? GetValue(string key) { } + public void SetValue(string key, string? value) { } + public override string ToString() { } + public static Sentry.BaggageHeader? TryParse(string baggage, bool onlySentry = false) { } + } [System.Diagnostics.DebuggerDisplay("Message: {Message}, Type: {Type}")] public sealed class Breadcrumb : Sentry.IJsonSerializable { @@ -109,6 +123,16 @@ namespace Sentry public DsnAttribute(string dsn) { } public string Dsn { get; } } + public class DynamicSamplingContext + { + public string? Environment { get; } + public string PublicKey { get; } + public string? Release { get; } + public double SampleRate { get; } + public Sentry.SentryId TraceId { get; } + public string? TransactionName { get; } + public string? UserSegment { get; } + } public static class EventLikeExtensions { public static bool HasUser(this Sentry.IEventLike eventLike) { } @@ -839,6 +863,7 @@ namespace Sentry public string? Platform { get; set; } public string? Release { get; set; } public Sentry.Request Request { get; set; } + public double? SampleRate { get; } public Sentry.SdkVersion Sdk { get; } public Sentry.SpanId SpanId { get; } public System.Collections.Generic.IReadOnlyCollection Spans { get; } @@ -908,6 +933,7 @@ namespace Sentry public string? Platform { get; set; } public string? Release { get; set; } public Sentry.Request Request { get; set; } + public double? SampleRate { get; } public Sentry.SdkVersion Sdk { get; } public Sentry.SpanId SpanId { get; } public System.Collections.Generic.IReadOnlyCollection Spans { get; } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index cf47a4a817..5b7a97b90b 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt @@ -18,6 +18,20 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, } + public class BaggageHeader + { + public string? SentryEnvironment { get; set; } + public string? SentryPublicKey { get; set; } + public string? SentryRelease { get; set; } + public double? SentrySampleRate { get; set; } + public Sentry.SentryId? SentryTraceId { get; set; } + public string? SentryTransactionName { get; set; } + public string? SentryUserSegment { get; set; } + public string? GetValue(string key) { } + public void SetValue(string key, string? value) { } + public override string ToString() { } + public static Sentry.BaggageHeader? TryParse(string baggage, bool onlySentry = false) { } + } [System.Diagnostics.DebuggerDisplay("Message: {Message}, Type: {Type}")] public sealed class Breadcrumb : Sentry.IJsonSerializable { @@ -109,6 +123,16 @@ namespace Sentry public DsnAttribute(string dsn) { } public string Dsn { get; } } + public class DynamicSamplingContext + { + public string? Environment { get; } + public string PublicKey { get; } + public string? Release { get; } + public double SampleRate { get; } + public Sentry.SentryId TraceId { get; } + public string? TransactionName { get; } + public string? UserSegment { get; } + } public static class EventLikeExtensions { public static bool HasUser(this Sentry.IEventLike eventLike) { } @@ -840,6 +864,7 @@ namespace Sentry public string? Platform { get; set; } public string? Release { get; set; } public Sentry.Request Request { get; set; } + public double? SampleRate { get; } public Sentry.SdkVersion Sdk { get; } public Sentry.SpanId SpanId { get; } public System.Collections.Generic.IReadOnlyCollection Spans { get; } @@ -909,6 +934,7 @@ namespace Sentry public string? Platform { get; set; } public string? Release { get; set; } public Sentry.Request Request { get; set; } + public double? SampleRate { get; } public Sentry.SdkVersion Sdk { get; } public Sentry.SpanId SpanId { get; } public System.Collections.Generic.IReadOnlyCollection Spans { get; } diff --git a/test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt b/test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt new file mode 100644 index 0000000000..95759dd0ff --- /dev/null +++ b/test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt @@ -0,0 +1,15 @@ +{ + target: { + SentryTraceId: 771a43a4192642f0b136d5159a501700, + SentryPublicKey: 49d0f7386ad645858ae85020e393bef3, + SentrySampleRate: 0.01337 + }, + items: { + other-vendor-value-1: foo;bar;baz, + other-vendor-value-2: foo;bar;, + sentry-public_key: 49d0f7386ad645858ae85020e393bef3, + sentry-sample_rate: 0.01337, + sentry-trace_id: 771a43a4192642f0b136d5159a501700, + sentry-user_id: Am%C3%A9lie + } +} diff --git a/test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt b/test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt new file mode 100644 index 0000000000..1b757735a4 --- /dev/null +++ b/test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt @@ -0,0 +1,25 @@ +{ + target: { + SentryTraceId: 771a43a4192642f0b136d5159a501700, + SentryPublicKey: 49d0f7386ad645858ae85020e393bef3, + SentrySampleRate: 0.01337, + SentryRelease: foo@abc+123, + SentryEnvironment: production, + SentryUserSegment: segment-a, + SentryTransactionName: something,I think + }, + items: { + other-vendor-value-1: foo, + other-vendor-value-2: foo;bar;, + sentry-environment: production, + sentry-other_value1: Am%C3%A9lie, + sentry-other_value2: Foo%20Bar%20Baz, + sentry-public_key: 49d0f7386ad645858ae85020e393bef3, + sentry-release: foo@abc+123, + sentry-sample_rate: 0.01337, + sentry-trace_id: 771a43a4192642f0b136d5159a501700, + sentry-transaction: something%2cI%20think, + sentry-user_segment: segment-a + }, + SentryOtherValue1: Amélie +} diff --git a/test/Sentry.Tests/BaggageHeaderTests.cs b/test/Sentry.Tests/BaggageHeaderTests.cs new file mode 100644 index 0000000000..87dcb549b7 --- /dev/null +++ b/test/Sentry.Tests/BaggageHeaderTests.cs @@ -0,0 +1,53 @@ +namespace Sentry.Tests; + +[UsesVerify] +public class BaggageHeaderTests +{ + [Fact] + public Task Parse_Full() + { + var header = BaggageHeader.TryParse( + "sentry-trace_id=771a43a4192642f0b136d5159a501700," + + "sentry-public_key=49d0f7386ad645858ae85020e393bef3," + + "sentry-sample_rate=0.01337," + + "sentry-release=foo@abc+123," + + "sentry-environment=production," + + "sentry-user_segment=segment-a," + + "sentry-transaction=something%2cI%20think," + + "sentry-other_value1=Am%C3%A9lie," + + "sentry-other_value2=Foo%20Bar%20Baz," + + "other-vendor-value-1=foo," + + "other-vendor-value-2=foo;bar;"); + + Assert.NotNull(header); + + return VerifyHeader(header) + .AppendValue("SentryOtherValue1", header.GetValue("sentry-other_value1")!); + } + + [Fact] + public Task Parse_FromExample() + { + // Taken from https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#baggage + var header = BaggageHeader.TryParse( + "other-vendor-value-1=foo;bar;baz, sentry-trace_id=771a43a4192642f0b136d5159a501700, sentry-public_key=49d0f7386ad645858ae85020e393bef3, sentry-sample_rate=0.01337, sentry-user_id=Am%C3%A9lie, other-vendor-value-2=foo;bar;"); + + return VerifyHeader(header); + } + + private static SettingsTask VerifyHeader(BaggageHeader header) + { + return Verify(header) + .DontScrubGuids() + .AddExtraSettings(x => x.Converters.Add(new SentryIdConverter())) + .AppendValue("items", header.GetRawMembers()); + } + + private class SentryIdConverter : WriteOnlyJsonConverter + { + public override void Write(VerifyJsonWriter writer, SentryId value) + { + writer.WriteValue(value.ToString()); + } + } +} diff --git a/test/Sentry.Tests/SentryClientTests.cs b/test/Sentry.Tests/SentryClientTests.cs index 24acebfbb8..4915d5b771 100644 --- a/test/Sentry.Tests/SentryClientTests.cs +++ b/test/Sentry.Tests/SentryClientTests.cs @@ -13,6 +13,7 @@ private class Fixture { public SentryOptions SentryOptions { get; set; } = new() { + Dsn = ValidDsn, AttachStacktrace = false, AutoSessionTracking = false }; diff --git a/test/Sentry.Tests/TransactionProcessorTests.Simple.verified.txt b/test/Sentry.Tests/TransactionProcessorTests.Simple.verified.txt index 596b35be93..381470823b 100644 --- a/test/Sentry.Tests/TransactionProcessorTests.Simple.verified.txt +++ b/test/Sentry.Tests/TransactionProcessorTests.Simple.verified.txt @@ -39,6 +39,14 @@ event_id: Guid_2, sdk: { name: sentry.dotnet + }, + trace: { + environment: production, + public_key: d4d82fc1c2c4032a83f3a29aa3a3aff, + release: Sentry.Tests@1.0.0, + sample_rate: 1, + trace_id: Guid_3, + transaction: my transaction } }, Items: [ @@ -54,6 +62,7 @@ Description: , Status: UnknownError, IsSampled: true, + SampleRate: 1.0, Request: {}, Contexts: { key: value, diff --git a/test/Sentry.Tests/TransactionProcessorTests.cs b/test/Sentry.Tests/TransactionProcessorTests.cs index b8a35f058c..b06411f28e 100644 --- a/test/Sentry.Tests/TransactionProcessorTests.cs +++ b/test/Sentry.Tests/TransactionProcessorTests.cs @@ -104,6 +104,7 @@ private SentryOptions Options(ITransport transport) => Transport = transport, Dsn = ValidDsn, DiagnosticLogger = _logger, - AttachStacktrace = false + AttachStacktrace = false, + Release = "Sentry.Tests@1.0.0" }; } From 3a7595bac1fff77b88d1fa130b19a2774b25353d Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 09:21:51 -0700 Subject: [PATCH 05/29] Update verify tests for Windows build --- .../SqlListenerTests.RecordsEf.Core3_1.verified.txt | 1 + .../SqlListenerTests.RecordsEf.DotNet6_0.verified.txt | 1 + .../SqlListenerTests.RecordsEf.Net4_8.verified.txt | 1 + .../SqlListenerTests.RecordsSql.verified.txt | 1 + .../IntegrationTests.Simple.verified.txt | 1 + 5 files changed, 5 insertions(+) diff --git a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.Core3_1.verified.txt b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.Core3_1.verified.txt index 9c4bbc3f1c..1c4ce12998 100644 --- a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.Core3_1.verified.txt +++ b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.Core3_1.verified.txt @@ -35,6 +35,7 @@ Description: , Status: UnknownError, IsSampled: true, + SampleRate: 1.0, Request: {}, Contexts: { trace: { diff --git a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.DotNet6_0.verified.txt b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.DotNet6_0.verified.txt index a0171b685d..99b6ee225e 100644 --- a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.DotNet6_0.verified.txt +++ b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.DotNet6_0.verified.txt @@ -35,6 +35,7 @@ Description: , Status: UnknownError, IsSampled: true, + SampleRate: 1.0, Request: {}, Contexts: { trace: { diff --git a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.Net4_8.verified.txt b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.Net4_8.verified.txt index 2845ee61aa..31d0635ef2 100644 --- a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.Net4_8.verified.txt +++ b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.Net4_8.verified.txt @@ -35,6 +35,7 @@ Description: , Status: UnknownError, IsSampled: true, + SampleRate: 1.0, Request: {}, Contexts: { trace: { diff --git a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsSql.verified.txt b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsSql.verified.txt index b41f6c2e8c..499b2d34f3 100644 --- a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsSql.verified.txt +++ b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsSql.verified.txt @@ -35,6 +35,7 @@ Description: , Status: UnknownError, IsSampled: true, + SampleRate: 1.0, Request: {}, Contexts: { trace: { diff --git a/test/Sentry.EntityFramework.Tests/IntegrationTests.Simple.verified.txt b/test/Sentry.EntityFramework.Tests/IntegrationTests.Simple.verified.txt index d36d946fb1..1c61b239aa 100644 --- a/test/Sentry.EntityFramework.Tests/IntegrationTests.Simple.verified.txt +++ b/test/Sentry.EntityFramework.Tests/IntegrationTests.Simple.verified.txt @@ -35,6 +35,7 @@ Description: , Status: UnknownError, IsSampled: true, + SampleRate: 1.0, Request: {}, Contexts: { trace: { From 9ad0fe1c0aa507e1f7c6029752fc823704b41d39 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 09:24:06 -0700 Subject: [PATCH 06/29] mark verify tests --- test/Sentry.Tests/BaggageHeaderTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Sentry.Tests/BaggageHeaderTests.cs b/test/Sentry.Tests/BaggageHeaderTests.cs index 87dcb549b7..151f483e57 100644 --- a/test/Sentry.Tests/BaggageHeaderTests.cs +++ b/test/Sentry.Tests/BaggageHeaderTests.cs @@ -4,6 +4,7 @@ namespace Sentry.Tests; public class BaggageHeaderTests { [Fact] + [Trait("Category", "Verify")] public Task Parse_Full() { var header = BaggageHeader.TryParse( @@ -26,6 +27,7 @@ public Task Parse_Full() } [Fact] + [Trait("Category", "Verify")] public Task Parse_FromExample() { // Taken from https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#baggage From 85858884455af385c09ce6847a5ff8e0c4a2c55c Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 09:26:29 -0700 Subject: [PATCH 07/29] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75dc16887d..164c2ed2d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Add `User.Segment` property ([#1920](https://github.com/getsentry/sentry-dotnet/pull/1920)) - Add support for custom `JsonConverter`s ([#1934](https://github.com/getsentry/sentry-dotnet/pull/1934)) - Support more types for message template tags in SentryLogger ([#1945](https://github.com/getsentry/sentry-dotnet/pull/1945)) +- Support Dynamic Sampling ([#1953](https://github.com/getsentry/sentry-dotnet/pull/1953)) ## Fixes From 5bd35042b489c6c2a5450eafbfe88f6d5b96d336 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 15:05:49 -0700 Subject: [PATCH 08/29] Apply baggage headers --- src/Sentry/BaggageHeader.cs | 29 +++++++++++++++++-- src/Sentry/DynamicSamplingContext.cs | 2 ++ .../Extensions/CollectionsExtensions.cs | 3 ++ src/Sentry/SentryHttpMessageHandler.cs | 21 ++++++++++---- 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/Sentry/BaggageHeader.cs b/src/Sentry/BaggageHeader.cs index efbbd3db4d..a7069e1211 100644 --- a/src/Sentry/BaggageHeader.cs +++ b/src/Sentry/BaggageHeader.cs @@ -18,9 +18,9 @@ public class BaggageHeader internal const string HttpHeaderName = "baggage"; private const string SentryKeyPrefix = "sentry-"; - private BaggageHeader(IDictionary members) + private BaggageHeader(IDictionary? members = null) { - _members = members; + _members = members ?? new Dictionary(); } internal IReadOnlyDictionary GetRawMembers() => @@ -181,7 +181,7 @@ public override string ToString() // "key1=value1;property1;property2, key2 = value2, key3=value3; propertyKey=propertyValue" var items = baggage.Split(',', StringSplitOptions.RemoveEmptyEntries); - var resultItems = new Dictionary(items.Length, StringComparer.Ordinal); + var resultItems = new Dictionary(items.Length); foreach (var item in items) { @@ -210,6 +210,29 @@ public override string ToString() return resultItems.Count == 0 ? null : new BaggageHeader(resultItems); } + public static BaggageHeader Create(IReadOnlyCollection> members) + { + var items = new Dictionary(members.Count); + + foreach (var member in members) + { + if (!IsValidKey(member.Key)) + { + // drop members with invalid keys + continue; + } + + items.Add(member.Key, Uri.EscapeDataString(member.Value)); + } + + return new BaggageHeader(items); + } + + public static BaggageHeader Merge(IEnumerable baggageHeaders) + { + throw new NotImplementedException(); + } + private static bool IsValidKey(string key) { if (key.Length == 0) diff --git a/src/Sentry/DynamicSamplingContext.cs b/src/Sentry/DynamicSamplingContext.cs index 2172c18118..e454c74b81 100644 --- a/src/Sentry/DynamicSamplingContext.cs +++ b/src/Sentry/DynamicSamplingContext.cs @@ -117,6 +117,8 @@ private DynamicSamplingContext( internal IReadOnlyDictionary GetItems() => _items; + internal BaggageHeader ToBaggageHeader() => BaggageHeader.Create(_items); + internal static DynamicSamplingContext? CreateFromBaggageHeader(BaggageHeader baggage) { var items = baggage.GetSentryMembers(); diff --git a/src/Sentry/Internal/Extensions/CollectionsExtensions.cs b/src/Sentry/Internal/Extensions/CollectionsExtensions.cs index 56c9a5a10f..5d5e2b6986 100644 --- a/src/Sentry/Internal/Extensions/CollectionsExtensions.cs +++ b/src/Sentry/Internal/Extensions/CollectionsExtensions.cs @@ -60,5 +60,8 @@ public static IReadOnlyList AsReadOnly(this IList list) => public static IReadOnlyDictionary AsReadOnly(this IDictionary dictionary) where TKey : notnull => new ReadOnlyDictionary(dictionary); + + public static IEnumerable ExceptNulls(this IEnumerable source) => + source.Where(x => x != null).Select(x => x!); } } diff --git a/src/Sentry/SentryHttpMessageHandler.cs b/src/Sentry/SentryHttpMessageHandler.cs index 7cb088e734..deee67f596 100644 --- a/src/Sentry/SentryHttpMessageHandler.cs +++ b/src/Sentry/SentryHttpMessageHandler.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Net.Http; using System.Threading; using System.Threading.Tasks; using Sentry.Extensibility; +using Sentry.Internal.Extensions; namespace Sentry { @@ -58,15 +60,24 @@ protected override async Task SendAsync( if (transaction is TransactionTracer {DynamicSamplingContext: {IsEmpty: false} dsc}) { + var baggage = dsc.ToBaggageHeader(); + // Set baggage header(s) if (request.Headers.TryGetValues(BaggageHeader.HttpHeaderName, out var baggageHeaders)) { - // TODO: merge baggage headers and add sentry's from the DSC - } - else - { - // TODO: Add Sentry's baggage header from DSC + // Merge baggage headers, including ours. + var allBaggage = baggageHeaders + .Select(s => BaggageHeader.TryParse(s)) + .ExceptNulls() + .Append(baggage); + baggage = BaggageHeader.Merge(allBaggage); + + // Remove the existing header so we can replace it. + request.Headers.Remove(BaggageHeader.HttpHeaderName); } + + // Set baggage header + request.Headers.Add(BaggageHeader.HttpHeaderName, baggage.ToString()); } // Prevent null reference exception in the following call From 39fc0135cd29c6876ec49e2e527953b598d56276 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 16:28:22 -0700 Subject: [PATCH 09/29] Simplify baggage header --- src/Sentry/BaggageHeader.cs | 190 +++--------------- .../ApiApprovalTests.Run.Core3_1.verified.txt | 14 -- ...piApprovalTests.Run.DotNet4_8.verified.txt | 14 -- ...piApprovalTests.Run.DotNet6_0.verified.txt | 14 -- ...HeaderTests.Parse_FromExample.verified.txt | 39 ++-- ...BaggageHeaderTests.Parse_Full.verified.txt | 71 ++++--- test/Sentry.Tests/BaggageHeaderTests.cs | 17 +- 7 files changed, 112 insertions(+), 247 deletions(-) diff --git a/src/Sentry/BaggageHeader.cs b/src/Sentry/BaggageHeader.cs index a7069e1211..c0009ada03 100644 --- a/src/Sentry/BaggageHeader.cs +++ b/src/Sentry/BaggageHeader.cs @@ -1,8 +1,6 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.Linq; -using Sentry.Internal.Extensions; namespace Sentry { @@ -12,143 +10,33 @@ namespace Sentry /// /// /// - public class BaggageHeader + internal class BaggageHeader { - private readonly IDictionary _members; internal const string HttpHeaderName = "baggage"; private const string SentryKeyPrefix = "sentry-"; - private BaggageHeader(IDictionary? members = null) - { - _members = members ?? new Dictionary(); - } + // https://www.w3.org/TR/baggage/#baggage-string + // "Uniqueness of keys between multiple list-members in a baggage-string is not guaranteed." + // "The order of duplicate entries SHOULD be preserved when mutating the list." + + public IReadOnlyList> Members { get; } - internal IReadOnlyDictionary GetRawMembers() => - _members.OrderBy(x => x.Key).ToDictionary(); + private BaggageHeader(IEnumerable> members) => + Members = members.ToList().AsReadOnly(); - internal IReadOnlyDictionary GetSentryMembers() => - _members - .Where(x => x.Key.StartsWith(SentryKeyPrefix)) - .OrderBy(x => x.Key) + // We can safely return a dictionary of Sentry members, as we are in control over the keys added. + // Just to be safe though, we'll group by key and only take the first of each one. + public IReadOnlyDictionary GetSentryMembers() => + Members + .Where(kvp => kvp.Key.StartsWith(SentryKeyPrefix)) + .GroupBy(kvp => kvp.Key, kvp => kvp.Value) .ToDictionary( #if NETCOREAPP || NETSTANDARD2_1 - _ => _.Key[SentryKeyPrefix.Length..], + g => g.Key[SentryKeyPrefix.Length..], #else - _ => _.Key.Substring(SentryKeyPrefix.Length), + g => g.Key.Substring(SentryKeyPrefix.Length), #endif - _ => Uri.UnescapeDataString(_.Value)); - - /// - /// Gets a value from the list members of the baggage header, if it exists. - /// - /// The key of the list member. - /// The value of the list member if found, or null otherwise. - public string? GetValue(string key) => _members.TryGetValue(key, out var value) - ? Uri.UnescapeDataString(value) - : null; - - /// - /// Sets a value for the a list member of the baggage header. - /// - /// The key of the list member. - /// The value of the list member. - /// - /// Only non-null members will be added to the list. - /// Attempting to set a null value will remove the member from the list if it exists. - /// Attempting to set a value that is already present in the list will overwrite the existing value. - /// - public void SetValue(string key, string? value) - { - if (IsValidKey(key)) - { - throw new ArgumentException("The provided key is invalid.", nameof(key)); - } - - SetValueInternal(key, value); - } - - private void SetValueInternal(string key, string? value) - { - if (value is null) - { - _members.Remove(key); - } - else - { - _members[key] = Uri.EscapeDataString(value); - } - } - - /// - /// Gets or sets the Sentry trace ID in the list members of the baggage header. - /// - public SentryId? SentryTraceId - { - get => _members.TryGetValue(SentryKeyPrefix + "trace_id", out var value) - ? Guid.TryParse(value, out var traceId) - ? new SentryId(traceId) - : null - : null; - set => SetValueInternal(SentryKeyPrefix + "trace_id", value?.ToString()); - } - - /// - /// Gets or sets the Sentry public key in the list members of the baggage header. - /// - public string? SentryPublicKey - { - get => GetValue(SentryKeyPrefix + "public_key"); - set => SetValueInternal(SentryKeyPrefix + "public_key", value); - } - - /// - /// Gets or sets the Sentry sample rate in the list members of the baggage header. - /// - public double? SentrySampleRate - { - get => _members.TryGetValue(SentryKeyPrefix + "sample_rate", out var value) - ? double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out var sampleRate) - ? sampleRate - : null - : null; - set => SetValueInternal(SentryKeyPrefix + "sample_rate", value?.ToString(CultureInfo.InvariantCulture)); - } - - /// - /// Gets or sets the Sentry release in the list members of the baggage header. - /// - public string? SentryRelease - { - get => GetValue(SentryKeyPrefix + "release"); - set => SetValueInternal(SentryKeyPrefix + "release", value); - } - - /// - /// Gets or sets the Sentry environment in the list members of the baggage header. - /// - public string? SentryEnvironment - { - get => GetValue(SentryKeyPrefix + "environment"); - set => SetValueInternal(SentryKeyPrefix + "environment", value); - } - - /// - /// Gets or sets the Sentry user segment in the list members of the baggage header. - /// - public string? SentryUserSegment - { - get => GetValue(SentryKeyPrefix + "user_segment"); - set => SetValueInternal(SentryKeyPrefix + "user_segment", value); - } - - /// - /// Gets or sets the Sentry transaction name in the list members of the baggage header. - /// - public string? SentryTransactionName - { - get => GetValue(SentryKeyPrefix + "transaction"); - set => SetValueInternal(SentryKeyPrefix + "transaction", value); - } + g => g.First()); /// /// Creates the baggage header string based on the members of this instance. @@ -156,12 +44,12 @@ public string? SentryTransactionName /// The baggage header string. public override string ToString() { - // the item keys do not require special encoding - // the item value are already encoded correctly, so we can just return them - var items = _members - .OrderBy(x => x.Key) - .Select(x => $"{x.Key}={x.Value}"); - return string.Join(", ", items); + // The keys do not require special encoding. The values are percent-encoded. + // The results should not be sorted, as the baggage spec says original ordering should be preserved. + var members = Members.Select(x => $"{x.Key}={Uri.EscapeDataString(x.Value)}"); + + // Whitespace after delimiter is optional by the spec, but typical by convention. + return string.Join(", ", members); } /// @@ -181,7 +69,7 @@ public override string ToString() // "key1=value1;property1;property2, key2 = value2, key3=value3; propertyKey=propertyValue" var items = baggage.Split(',', StringSplitOptions.RemoveEmptyEntries); - var resultItems = new Dictionary(items.Length); + var members = new List>(items.Length); foreach (var item in items) { @@ -203,35 +91,19 @@ public override string ToString() if (!onlySentry || key.StartsWith(SentryKeyPrefix)) { - resultItems.Add(key, value); + // Values are percent-encoded. Decode them before storing. + members.Add(new KeyValuePair(key, Uri.UnescapeDataString(value))); } } - return resultItems.Count == 0 ? null : new BaggageHeader(resultItems); + return members.Count == 0 ? null : new BaggageHeader(members); } - public static BaggageHeader Create(IReadOnlyCollection> members) - { - var items = new Dictionary(members.Count); + public static BaggageHeader Create(IEnumerable> members) => + new(members.Where(member => IsValidKey(member.Key))); - foreach (var member in members) - { - if (!IsValidKey(member.Key)) - { - // drop members with invalid keys - continue; - } - - items.Add(member.Key, Uri.EscapeDataString(member.Value)); - } - - return new BaggageHeader(items); - } - - public static BaggageHeader Merge(IEnumerable baggageHeaders) - { - throw new NotImplementedException(); - } + public static BaggageHeader Merge(IEnumerable baggageHeaders) => + new(baggageHeaders.SelectMany(x => x.Members)); private static bool IsValidKey(string key) { diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt index 5b7a97b90b..cd7d68de53 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt @@ -18,20 +18,6 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, } - public class BaggageHeader - { - public string? SentryEnvironment { get; set; } - public string? SentryPublicKey { get; set; } - public string? SentryRelease { get; set; } - public double? SentrySampleRate { get; set; } - public Sentry.SentryId? SentryTraceId { get; set; } - public string? SentryTransactionName { get; set; } - public string? SentryUserSegment { get; set; } - public string? GetValue(string key) { } - public void SetValue(string key, string? value) { } - public override string ToString() { } - public static Sentry.BaggageHeader? TryParse(string baggage, bool onlySentry = false) { } - } [System.Diagnostics.DebuggerDisplay("Message: {Message}, Type: {Type}")] public sealed class Breadcrumb : Sentry.IJsonSerializable { diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt index 5950436015..8a98a57bc5 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt @@ -18,20 +18,6 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, } - public class BaggageHeader - { - public string? SentryEnvironment { get; set; } - public string? SentryPublicKey { get; set; } - public string? SentryRelease { get; set; } - public double? SentrySampleRate { get; set; } - public Sentry.SentryId? SentryTraceId { get; set; } - public string? SentryTransactionName { get; set; } - public string? SentryUserSegment { get; set; } - public string? GetValue(string key) { } - public void SetValue(string key, string? value) { } - public override string ToString() { } - public static Sentry.BaggageHeader? TryParse(string baggage, bool onlySentry = false) { } - } [System.Diagnostics.DebuggerDisplay("Message: {Message}, Type: {Type}")] public sealed class Breadcrumb : Sentry.IJsonSerializable { diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index 5b7a97b90b..cd7d68de53 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt @@ -18,20 +18,6 @@ namespace Sentry UnrealContext = 3, UnrealLogs = 4, } - public class BaggageHeader - { - public string? SentryEnvironment { get; set; } - public string? SentryPublicKey { get; set; } - public string? SentryRelease { get; set; } - public double? SentrySampleRate { get; set; } - public Sentry.SentryId? SentryTraceId { get; set; } - public string? SentryTransactionName { get; set; } - public string? SentryUserSegment { get; set; } - public string? GetValue(string key) { } - public void SetValue(string key, string? value) { } - public override string ToString() { } - public static Sentry.BaggageHeader? TryParse(string baggage, bool onlySentry = false) { } - } [System.Diagnostics.DebuggerDisplay("Message: {Message}, Type: {Type}")] public sealed class Breadcrumb : Sentry.IJsonSerializable { diff --git a/test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt b/test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt index 95759dd0ff..555abb3f65 100644 --- a/test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt +++ b/test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt @@ -1,15 +1,26 @@ -{ - target: { - SentryTraceId: 771a43a4192642f0b136d5159a501700, - SentryPublicKey: 49d0f7386ad645858ae85020e393bef3, - SentrySampleRate: 0.01337 - }, - items: { - other-vendor-value-1: foo;bar;baz, - other-vendor-value-2: foo;bar;, - sentry-public_key: 49d0f7386ad645858ae85020e393bef3, - sentry-sample_rate: 0.01337, - sentry-trace_id: 771a43a4192642f0b136d5159a501700, - sentry-user_id: Am%C3%A9lie +[ + { + Key: other-vendor-value-1, + Value: foo;bar;baz + }, + { + Key: sentry-trace_id, + Value: 771a43a4192642f0b136d5159a501700 + }, + { + Key: sentry-public_key, + Value: 49d0f7386ad645858ae85020e393bef3 + }, + { + Key: sentry-sample_rate, + Value: 0.01337 + }, + { + Key: sentry-user_id, + Value: Amélie + }, + { + Key: other-vendor-value-2, + Value: foo;bar; } -} +] \ No newline at end of file diff --git a/test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt b/test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt index 1b757735a4..b671afa235 100644 --- a/test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt +++ b/test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt @@ -1,25 +1,46 @@ -{ - target: { - SentryTraceId: 771a43a4192642f0b136d5159a501700, - SentryPublicKey: 49d0f7386ad645858ae85020e393bef3, - SentrySampleRate: 0.01337, - SentryRelease: foo@abc+123, - SentryEnvironment: production, - SentryUserSegment: segment-a, - SentryTransactionName: something,I think - }, - items: { - other-vendor-value-1: foo, - other-vendor-value-2: foo;bar;, - sentry-environment: production, - sentry-other_value1: Am%C3%A9lie, - sentry-other_value2: Foo%20Bar%20Baz, - sentry-public_key: 49d0f7386ad645858ae85020e393bef3, - sentry-release: foo@abc+123, - sentry-sample_rate: 0.01337, - sentry-trace_id: 771a43a4192642f0b136d5159a501700, - sentry-transaction: something%2cI%20think, - sentry-user_segment: segment-a - }, - SentryOtherValue1: Amélie -} +[ + { + Key: sentry-trace_id, + Value: 771a43a4192642f0b136d5159a501700 + }, + { + Key: sentry-public_key, + Value: 49d0f7386ad645858ae85020e393bef3 + }, + { + Key: sentry-sample_rate, + Value: 0.01337 + }, + { + Key: sentry-release, + Value: foo@abc+123 + }, + { + Key: sentry-environment, + Value: production + }, + { + Key: sentry-user_segment, + Value: segment-a + }, + { + Key: sentry-transaction, + Value: something, I think + }, + { + Key: sentry-other_value1, + Value: Amélie + }, + { + Key: sentry-other_value2, + Value: Foo Bar Baz + }, + { + Key: other-vendor-value-1, + Value: foo + }, + { + Key: other-vendor-value-2, + Value: foo;bar; + } +] diff --git a/test/Sentry.Tests/BaggageHeaderTests.cs b/test/Sentry.Tests/BaggageHeaderTests.cs index 151f483e57..180b116c55 100644 --- a/test/Sentry.Tests/BaggageHeaderTests.cs +++ b/test/Sentry.Tests/BaggageHeaderTests.cs @@ -14,7 +14,7 @@ public Task Parse_Full() "sentry-release=foo@abc+123," + "sentry-environment=production," + "sentry-user_segment=segment-a," + - "sentry-transaction=something%2cI%20think," + + "sentry-transaction=something%2c%20I%20think," + "sentry-other_value1=Am%C3%A9lie," + "sentry-other_value2=Foo%20Bar%20Baz," + "other-vendor-value-1=foo," + @@ -22,8 +22,7 @@ public Task Parse_Full() Assert.NotNull(header); - return VerifyHeader(header) - .AppendValue("SentryOtherValue1", header.GetValue("sentry-other_value1")!); + return VerifyHeader(header); } [Fact] @@ -32,17 +31,21 @@ public Task Parse_FromExample() { // Taken from https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#baggage var header = BaggageHeader.TryParse( - "other-vendor-value-1=foo;bar;baz, sentry-trace_id=771a43a4192642f0b136d5159a501700, sentry-public_key=49d0f7386ad645858ae85020e393bef3, sentry-sample_rate=0.01337, sentry-user_id=Am%C3%A9lie, other-vendor-value-2=foo;bar;"); + "other-vendor-value-1=foo;bar;baz, " + + "sentry-trace_id=771a43a4192642f0b136d5159a501700, " + + "sentry-public_key=49d0f7386ad645858ae85020e393bef3, " + + "sentry-sample_rate=0.01337, " + + "sentry-user_id=Am%C3%A9lie, " + + "other-vendor-value-2=foo;bar;"); return VerifyHeader(header); } private static SettingsTask VerifyHeader(BaggageHeader header) { - return Verify(header) + return Verify(header.Members) .DontScrubGuids() - .AddExtraSettings(x => x.Converters.Add(new SentryIdConverter())) - .AppendValue("items", header.GetRawMembers()); + .AddExtraSettings(x => x.Converters.Add(new SentryIdConverter())); } private class SentryIdConverter : WriteOnlyJsonConverter From 382da982566dbef0f420dd5b421e3c007fb14f51 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 17:02:57 -0700 Subject: [PATCH 10/29] More baggage header tests --- src/Sentry/BaggageHeader.cs | 3 +- .../Internal/Extensions/MiscExtensions.cs | 6 ++ ...eHeader_TryParse_FromExample.verified.txt} | 0 ....BaggageHeader_TryParse_Full.verified.txt} | 10 +- ...ageHeader_TryParse_OnlySentry.verified.txt | 18 ++++ test/Sentry.Tests/BaggageHeaderTests.cs | 94 +++++++++++++++++-- 6 files changed, 121 insertions(+), 10 deletions(-) rename test/Sentry.Tests/{BaggageHeaderTests.Parse_FromExample.verified.txt => BaggageHeaderTests.BaggageHeader_TryParse_FromExample.verified.txt} (100%) rename test/Sentry.Tests/{BaggageHeaderTests.Parse_Full.verified.txt => BaggageHeaderTests.BaggageHeader_TryParse_Full.verified.txt} (87%) create mode 100644 test/Sentry.Tests/BaggageHeaderTests.BaggageHeader_TryParse_OnlySentry.verified.txt diff --git a/src/Sentry/BaggageHeader.cs b/src/Sentry/BaggageHeader.cs index c0009ada03..e35f7f577a 100644 --- a/src/Sentry/BaggageHeader.cs +++ b/src/Sentry/BaggageHeader.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Sentry.Internal.Extensions; namespace Sentry { @@ -92,7 +93,7 @@ public override string ToString() if (!onlySentry || key.StartsWith(SentryKeyPrefix)) { // Values are percent-encoded. Decode them before storing. - members.Add(new KeyValuePair(key, Uri.UnescapeDataString(value))); + members.Add(key, Uri.UnescapeDataString(value)); } } diff --git a/src/Sentry/Internal/Extensions/MiscExtensions.cs b/src/Sentry/Internal/Extensions/MiscExtensions.cs index e77dea0218..f7eea28910 100644 --- a/src/Sentry/Internal/Extensions/MiscExtensions.cs +++ b/src/Sentry/Internal/Extensions/MiscExtensions.cs @@ -57,5 +57,11 @@ public static void CancelAfterSafe(this CancellationTokenSource cts, TimeSpan ti public static string? GetStringProperty(this object obj, string name) => obj.GetProperty(name) as string; + + public static void Add( + this ICollection> collection, + TKey key, + TValue value) => + collection.Add(new KeyValuePair(key, value)); } } diff --git a/test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt b/test/Sentry.Tests/BaggageHeaderTests.BaggageHeader_TryParse_FromExample.verified.txt similarity index 100% rename from test/Sentry.Tests/BaggageHeaderTests.Parse_FromExample.verified.txt rename to test/Sentry.Tests/BaggageHeaderTests.BaggageHeader_TryParse_FromExample.verified.txt diff --git a/test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt b/test/Sentry.Tests/BaggageHeaderTests.BaggageHeader_TryParse_Full.verified.txt similarity index 87% rename from test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt rename to test/Sentry.Tests/BaggageHeaderTests.BaggageHeader_TryParse_Full.verified.txt index b671afa235..328e0a4c5a 100644 --- a/test/Sentry.Tests/BaggageHeaderTests.Parse_Full.verified.txt +++ b/test/Sentry.Tests/BaggageHeaderTests.BaggageHeader_TryParse_Full.verified.txt @@ -42,5 +42,13 @@ { Key: other-vendor-value-2, Value: foo;bar; + }, + { + Key: dup-value, + Value: something + }, + { + Key: dup-value, + Value: something } -] +] \ No newline at end of file diff --git a/test/Sentry.Tests/BaggageHeaderTests.BaggageHeader_TryParse_OnlySentry.verified.txt b/test/Sentry.Tests/BaggageHeaderTests.BaggageHeader_TryParse_OnlySentry.verified.txt new file mode 100644 index 0000000000..584a2af0dc --- /dev/null +++ b/test/Sentry.Tests/BaggageHeaderTests.BaggageHeader_TryParse_OnlySentry.verified.txt @@ -0,0 +1,18 @@ +[ + { + Key: sentry-trace_id, + Value: 771a43a4192642f0b136d5159a501700 + }, + { + Key: sentry-public_key, + Value: 49d0f7386ad645858ae85020e393bef3 + }, + { + Key: sentry-sample_rate, + Value: 0.01337 + }, + { + Key: sentry-user_id, + Value: Amélie + } +] \ No newline at end of file diff --git a/test/Sentry.Tests/BaggageHeaderTests.cs b/test/Sentry.Tests/BaggageHeaderTests.cs index 180b116c55..de8b4374d2 100644 --- a/test/Sentry.Tests/BaggageHeaderTests.cs +++ b/test/Sentry.Tests/BaggageHeaderTests.cs @@ -5,20 +5,23 @@ public class BaggageHeaderTests { [Fact] [Trait("Category", "Verify")] - public Task Parse_Full() + public Task BaggageHeader_TryParse_Full() { + // note: whitespace is intentionally varied as it should be ignored var header = BaggageHeader.TryParse( "sentry-trace_id=771a43a4192642f0b136d5159a501700," + - "sentry-public_key=49d0f7386ad645858ae85020e393bef3," + - "sentry-sample_rate=0.01337," + + "sentry-public_key = 49d0f7386ad645858ae85020e393bef3 , " + + "sentry-sample_rate=0.01337, " + "sentry-release=foo@abc+123," + "sentry-environment=production," + - "sentry-user_segment=segment-a," + + "sentry-user_segment =segment-a," + "sentry-transaction=something%2c%20I%20think," + - "sentry-other_value1=Am%C3%A9lie," + - "sentry-other_value2=Foo%20Bar%20Baz," + + "sentry-other_value1=Am%C3%A9lie, " + + "sentry-other_value2= Foo%20Bar%20Baz ," + "other-vendor-value-1=foo," + - "other-vendor-value-2=foo;bar;"); + "other-vendor-value-2=foo;bar;," + + "dup-value=something, " + + "dup-value=something,"); Assert.NotNull(header); @@ -27,7 +30,7 @@ public Task Parse_Full() [Fact] [Trait("Category", "Verify")] - public Task Parse_FromExample() + public Task BaggageHeader_TryParse_FromExample() { // Taken from https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#baggage var header = BaggageHeader.TryParse( @@ -41,6 +44,81 @@ public Task Parse_FromExample() return VerifyHeader(header); } + [Fact] + [Trait("Category", "Verify")] + public Task BaggageHeader_TryParse_OnlySentry() + { + // Taken from https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#baggage + var header = BaggageHeader.TryParse( + "other-vendor-value-1=foo;bar;baz, " + + "sentry-trace_id=771a43a4192642f0b136d5159a501700, " + + "sentry-public_key=49d0f7386ad645858ae85020e393bef3, " + + "sentry-sample_rate=0.01337, " + + "sentry-user_id=Am%C3%A9lie, " + + "other-vendor-value-2=foo;bar;", + onlySentry: true); + + return VerifyHeader(header); + } + + [Fact] + public void BaggageHeader_TryParse_Empty() + { + var header = BaggageHeader.TryParse(""); + Assert.Null(header); + } + + [Fact] + public void BaggageHeader_ToString() + { + var header = BaggageHeader.Create(new List> + { + {"test-bad-chars", @" ""(),/:;<=>?@[\]{}"}, + {"sentry-public_key", "49d0f7386ad645858ae85020e393bef3"}, + {"test-name", "Amélie"}, + {"test-name", "John"}, + }); + + Assert.Equal( + "test-bad-chars=%20%22%28%29%2C%2F%3A%3B%3C%3D%3E%3F%40%5B%5C%5D%7B%7D, " + + "sentry-public_key=49d0f7386ad645858ae85020e393bef3, test-name=Am%C3%A9lie, test-name=John", + header.ToString()); + } + + [Fact] + public void BaggageHeader_Merge() + { + var header1 = BaggageHeader.Create(new List> + { + {"foo", "123"}, + }); + + var header2 = BaggageHeader.Create(new List> + { + {"bar", "456"}, + {"baz", "789"}, + }); + + var header3 = BaggageHeader.Create(new List> + { + {"foo", "789"}, + {"baz", "000"}, + }); + + var merged = BaggageHeader.Merge(new[] {header1, header2, header3}); + + var expected = new List> + { + {"foo", "123"}, + {"bar", "456"}, + {"baz", "789"}, + {"foo", "789"}, + {"baz", "000"} + }; + + Assert.Equal(expected, merged.Members); + } + private static SettingsTask VerifyHeader(BaggageHeader header) { return Verify(header.Members) From 3b84f03babdafe4467796d964b9d9b0aa995300f Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 17:23:39 -0700 Subject: [PATCH 11/29] More baggage and DSC --- src/Sentry/BaggageHeader.cs | 20 +++++++++--- src/Sentry/DynamicSamplingContext.cs | 42 +++---------------------- test/Sentry.Tests/BaggageHeaderTests.cs | 36 +++++++++++++++++++++ 3 files changed, 55 insertions(+), 43 deletions(-) diff --git a/src/Sentry/BaggageHeader.cs b/src/Sentry/BaggageHeader.cs index e35f7f577a..1b2c809963 100644 --- a/src/Sentry/BaggageHeader.cs +++ b/src/Sentry/BaggageHeader.cs @@ -8,9 +8,8 @@ namespace Sentry /// /// Baggage Header for dynamic sampling. /// - /// - /// - /// + /// + /// internal class BaggageHeader { internal const string HttpHeaderName = "baggage"; @@ -100,8 +99,19 @@ public override string ToString() return members.Count == 0 ? null : new BaggageHeader(members); } - public static BaggageHeader Create(IEnumerable> members) => - new(members.Where(member => IsValidKey(member.Key))); + public static BaggageHeader Create( + IEnumerable> items, + bool useSentryPrefix = false) + { + var members = items.Where(member => IsValidKey(member.Key)); + + if (useSentryPrefix) + { + members = members.Select(kvp => new KeyValuePair(SentryKeyPrefix + kvp.Key, kvp.Value)); + } + + return new BaggageHeader(members); + } public static BaggageHeader Merge(IEnumerable baggageHeaders) => new(baggageHeaders.SelectMany(x => x.Members)); diff --git a/src/Sentry/DynamicSamplingContext.cs b/src/Sentry/DynamicSamplingContext.cs index e454c74b81..2056d3e842 100644 --- a/src/Sentry/DynamicSamplingContext.cs +++ b/src/Sentry/DynamicSamplingContext.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Globalization; +using System.Linq; using Sentry.Internal.Extensions; namespace Sentry @@ -33,7 +34,7 @@ private DynamicSamplingContext( string? userSegment = null, string? transactionName = null) { - // Validate and set required fields + // Validate and set required values if (traceId == SentryId.Empty) { throw new ArgumentOutOfRangeException(nameof(traceId)); @@ -56,7 +57,7 @@ private DynamicSamplingContext( ["sample_rate"] = sampleRate.ToString(CultureInfo.InvariantCulture) }; - // Set optional fields + // Set optional values if (!string.IsNullOrWhiteSpace(release)) { items.Add("release", release); @@ -80,44 +81,9 @@ private DynamicSamplingContext( _items = items; } - /// - /// Gets the trace ID of the Dynamic Sampling Context. - /// - public SentryId TraceId => SentryId.Parse(_items["trace_id"]); - - /// - /// Gets the public key of the Dynamic Sampling Context. - /// - public string PublicKey => _items["public_key"]; - - /// - /// Gets the sample rate of the Dynamic Sampling Context. - /// - public double SampleRate => double.Parse(_items["sample_rate"], CultureInfo.InvariantCulture); - - /// - /// Gets the release of the Dynamic Sampling Context, or null if none was set. - /// - public string? Release => _items.TryGetValue("release", out var value) ? value : null; - - /// - /// Gets the environment of the Dynamic Sampling Context, or null if none was set. - /// - public string? Environment => _items.TryGetValue("environment", out var value) ? value : null; - - /// - /// Gets the user segment of the Dynamic Sampling Context, or null if none was set. - /// - public string? UserSegment => _items.TryGetValue("user_segment", out var value) ? value : null; - - /// - /// Gets the transaction name of the Dynamic Sampling Context, or null if none was set. - /// - public string? TransactionName => _items.TryGetValue("transaction", out var value) ? value : null; - internal IReadOnlyDictionary GetItems() => _items; - internal BaggageHeader ToBaggageHeader() => BaggageHeader.Create(_items); + internal BaggageHeader ToBaggageHeader() => BaggageHeader.Create(_items, useSentryPrefix: true); internal static DynamicSamplingContext? CreateFromBaggageHeader(BaggageHeader baggage) { diff --git a/test/Sentry.Tests/BaggageHeaderTests.cs b/test/Sentry.Tests/BaggageHeaderTests.cs index de8b4374d2..dd770dc3ee 100644 --- a/test/Sentry.Tests/BaggageHeaderTests.cs +++ b/test/Sentry.Tests/BaggageHeaderTests.cs @@ -68,6 +68,42 @@ public void BaggageHeader_TryParse_Empty() Assert.Null(header); } + [Fact] + public void BaggageHeader_Create() + { + var header = BaggageHeader.Create(new List> + { + {"foo", "123"}, + {"bar", "456"} + }); + + var expected = new List> + { + {"foo", "123"}, + {"bar", "456"} + }; + + Assert.Equal(expected, header.Members); + } + + [Fact] + public void BaggageHeader_Create_WithSentryPrefix() + { + var header = BaggageHeader.Create(new List> + { + {"foo", "123"}, + {"bar", "456"} + }, useSentryPrefix: true); + + var expected = new List> + { + {"sentry-foo", "123"}, + {"sentry-bar", "456"} + }; + + Assert.Equal(expected, header.Members); + } + [Fact] public void BaggageHeader_ToString() { From 7d2594d8fa13c5201ef80f6df6399c8a32dc1347 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 17:31:38 -0700 Subject: [PATCH 12/29] Don't make DSC public --- src/Sentry/DynamicSamplingContext.cs | 29 ++++++++----------- src/Sentry/Protocol/Envelopes/Envelope.cs | 2 +- .../ApiApprovalTests.Run.Core3_1.verified.txt | 10 ------- ...piApprovalTests.Run.DotNet4_8.verified.txt | 10 ------- ...piApprovalTests.Run.DotNet6_0.verified.txt | 10 ------- 5 files changed, 13 insertions(+), 48 deletions(-) diff --git a/src/Sentry/DynamicSamplingContext.cs b/src/Sentry/DynamicSamplingContext.cs index 2056d3e842..5f035d767f 100644 --- a/src/Sentry/DynamicSamplingContext.cs +++ b/src/Sentry/DynamicSamplingContext.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Globalization; -using System.Linq; using Sentry.Internal.Extensions; namespace Sentry @@ -10,20 +9,18 @@ namespace Sentry /// Provides the Dynamic Sampling Context for a transaction. /// /// - public class DynamicSamplingContext + internal class DynamicSamplingContext { - // All values are stored in a dictionary, because it is possible to have other than those explicitly defined - // in this class. For example, the context can come from a baggage header with other Sentry- prefixed keys. - private readonly IReadOnlyDictionary _items; + public IReadOnlyDictionary Items { get; } - private DynamicSamplingContext(IReadOnlyDictionary items) => _items = items; + public bool IsEmpty => Items.Count == 0; + + private DynamicSamplingContext(IReadOnlyDictionary items) => Items = items; /// /// Gets an empty that can be used to "freeze" the DSC on a transaction. /// - internal static readonly DynamicSamplingContext Empty = new(new Dictionary().AsReadOnly()); - - internal bool IsEmpty => _items.Count == 0; + public static readonly DynamicSamplingContext Empty = new(new Dictionary().AsReadOnly()); private DynamicSamplingContext( SentryId traceId, @@ -78,14 +75,12 @@ private DynamicSamplingContext( items.Add("transaction", transactionName); } - _items = items; + Items = items; } - internal IReadOnlyDictionary GetItems() => _items; - - internal BaggageHeader ToBaggageHeader() => BaggageHeader.Create(_items, useSentryPrefix: true); + public BaggageHeader ToBaggageHeader() => BaggageHeader.Create(Items, useSentryPrefix: true); - internal static DynamicSamplingContext? CreateFromBaggageHeader(BaggageHeader baggage) + public static DynamicSamplingContext? CreateFromBaggageHeader(BaggageHeader baggage) { var items = baggage.GetSentryMembers(); @@ -113,7 +108,7 @@ private DynamicSamplingContext( return new DynamicSamplingContext(items); } - internal static DynamicSamplingContext CreateFromTransaction(TransactionTracer transaction, SentryOptions options) + public static DynamicSamplingContext CreateFromTransaction(TransactionTracer transaction, SentryOptions options) { // These should already be set on the transaction. var publicKey = Dsn.Parse(options.Dsn!).PublicKey; @@ -139,10 +134,10 @@ internal static DynamicSamplingContext CreateFromTransaction(TransactionTracer t internal static class DynamicSamplingContextExtensions { - internal static DynamicSamplingContext? CreateDynamicSamplingContext(this BaggageHeader baggage) + public static DynamicSamplingContext? CreateDynamicSamplingContext(this BaggageHeader baggage) => DynamicSamplingContext.CreateFromBaggageHeader(baggage); - internal static DynamicSamplingContext CreateDynamicSamplingContext(this TransactionTracer transaction, SentryOptions options) + public static DynamicSamplingContext CreateDynamicSamplingContext(this TransactionTracer transaction, SentryOptions options) => DynamicSamplingContext.CreateFromTransaction(transaction, options); } } diff --git a/src/Sentry/Protocol/Envelopes/Envelope.cs b/src/Sentry/Protocol/Envelopes/Envelope.cs index bd746f6bcb..f4c2630c8e 100644 --- a/src/Sentry/Protocol/Envelopes/Envelope.cs +++ b/src/Sentry/Protocol/Envelopes/Envelope.cs @@ -228,7 +228,7 @@ public static Envelope FromTransaction(Transaction transaction) if (transaction.DynamicSamplingContext is { } dsc) { header = CreateHeader(transaction.EventId, extraCapacity: 1); - header["trace"] = dsc.GetItems(); + header["trace"] = dsc.Items; } else { diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt index cd7d68de53..e72485e063 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt @@ -109,16 +109,6 @@ namespace Sentry public DsnAttribute(string dsn) { } public string Dsn { get; } } - public class DynamicSamplingContext - { - public string? Environment { get; } - public string PublicKey { get; } - public string? Release { get; } - public double SampleRate { get; } - public Sentry.SentryId TraceId { get; } - public string? TransactionName { get; } - public string? UserSegment { get; } - } public static class EventLikeExtensions { public static bool HasUser(this Sentry.IEventLike eventLike) { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt index 8a98a57bc5..7004476547 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt @@ -109,16 +109,6 @@ namespace Sentry public DsnAttribute(string dsn) { } public string Dsn { get; } } - public class DynamicSamplingContext - { - public string? Environment { get; } - public string PublicKey { get; } - public string? Release { get; } - public double SampleRate { get; } - public Sentry.SentryId TraceId { get; } - public string? TransactionName { get; } - public string? UserSegment { get; } - } public static class EventLikeExtensions { public static bool HasUser(this Sentry.IEventLike eventLike) { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index cd7d68de53..e72485e063 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt @@ -109,16 +109,6 @@ namespace Sentry public DsnAttribute(string dsn) { } public string Dsn { get; } } - public class DynamicSamplingContext - { - public string? Environment { get; } - public string PublicKey { get; } - public string? Release { get; } - public double SampleRate { get; } - public Sentry.SentryId TraceId { get; } - public string? TransactionName { get; } - public string? UserSegment { get; } - } public static class EventLikeExtensions { public static bool HasUser(this Sentry.IEventLike eventLike) { } From fa363f82be47ee24d88ae3ac2dcc20162a703010 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 17:46:25 -0700 Subject: [PATCH 13/29] Refactor --- src/Sentry/BaggageHeader.cs | 2 +- src/Sentry/SentryHttpMessageHandler.cs | 59 +++++++++++++++----------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/Sentry/BaggageHeader.cs b/src/Sentry/BaggageHeader.cs index 1b2c809963..224109eadd 100644 --- a/src/Sentry/BaggageHeader.cs +++ b/src/Sentry/BaggageHeader.cs @@ -13,7 +13,7 @@ namespace Sentry internal class BaggageHeader { internal const string HttpHeaderName = "baggage"; - private const string SentryKeyPrefix = "sentry-"; + internal const string SentryKeyPrefix = "sentry-"; // https://www.w3.org/TR/baggage/#baggage-string // "Uniqueness of keys between multiple list-members in a baggage-string is not guaranteed." diff --git a/src/Sentry/SentryHttpMessageHandler.cs b/src/Sentry/SentryHttpMessageHandler.cs index deee67f596..660d8745bf 100644 --- a/src/Sentry/SentryHttpMessageHandler.cs +++ b/src/Sentry/SentryHttpMessageHandler.cs @@ -56,29 +56,7 @@ protected override async Task SendAsync( request.Headers.Add(SentryTraceHeader.HttpHeaderName, traceHeader.ToString()); } - var transaction = _hub.GetSpan(); - - if (transaction is TransactionTracer {DynamicSamplingContext: {IsEmpty: false} dsc}) - { - var baggage = dsc.ToBaggageHeader(); - - // Set baggage header(s) - if (request.Headers.TryGetValues(BaggageHeader.HttpHeaderName, out var baggageHeaders)) - { - // Merge baggage headers, including ours. - var allBaggage = baggageHeaders - .Select(s => BaggageHeader.TryParse(s)) - .ExceptNulls() - .Append(baggage); - baggage = BaggageHeader.Merge(allBaggage); - - // Remove the existing header so we can replace it. - request.Headers.Remove(BaggageHeader.HttpHeaderName); - } - - // Set baggage header - request.Headers.Add(BaggageHeader.HttpHeaderName, baggage.ToString()); - } + AddBaggageHeader(request); // Prevent null reference exception in the following call // in case the user didn't set an inner handler. @@ -89,7 +67,7 @@ protected override async Task SendAsync( // Start a span that tracks this request // (may be null if transaction is not set on the scope) - var span = transaction?.StartChild( + var span = _hub.GetSpan()?.StartChild( "http.client", // e.g. "GET https://example.com" $"{requestMethod} {url}"); @@ -117,5 +95,38 @@ protected override async Task SendAsync( throw; } } + + private void AddBaggageHeader(HttpRequestMessage request) + { + var transaction = _hub.GetSpan(); + if (transaction is not TransactionTracer {DynamicSamplingContext: {IsEmpty: false} dsc}) + { + return; + } + + var baggage = dsc.ToBaggageHeader(); + + if (request.Headers.TryGetValues(BaggageHeader.HttpHeaderName, out var baggageHeaders)) + { + var headers = baggageHeaders.ToList(); + if (headers.Any(h => h.StartsWith(BaggageHeader.SentryKeyPrefix))) + { + // The Sentry headers have already been added to this request. Do nothing. + return; + } + + // Merge existing baggage headers with ours. + var allBaggage = headers + .Select(s => BaggageHeader.TryParse(s)).ExceptNulls() + .Append(baggage); + baggage = BaggageHeader.Merge(allBaggage); + + // Remove the existing header so we can replace it with the merged one. + request.Headers.Remove(BaggageHeader.HttpHeaderName); + } + + // Set the baggage header + request.Headers.Add(BaggageHeader.HttpHeaderName, baggage.ToString()); + } } } From 5c9300cb7b0c90672287c6513026715511b4d53b Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Wed, 28 Sep 2022 21:45:58 -0700 Subject: [PATCH 14/29] DSC tests --- .../DynamicSamplingContextTests.cs | 253 ++++++++++++++++++ 1 file changed, 253 insertions(+) create mode 100644 test/Sentry.Tests/DynamicSamplingContextTests.cs diff --git a/test/Sentry.Tests/DynamicSamplingContextTests.cs b/test/Sentry.Tests/DynamicSamplingContextTests.cs new file mode 100644 index 0000000000..df14242d21 --- /dev/null +++ b/test/Sentry.Tests/DynamicSamplingContextTests.cs @@ -0,0 +1,253 @@ +namespace Sentry.Tests; + +public class DynamicSamplingContextTests +{ + [Fact] + public void EmptyContext() + { + var dsc = DynamicSamplingContext.Empty; + + Assert.True(dsc.IsEmpty); + } + + [Fact] + public void CreateFromBaggage_TraceId_Missing() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.0"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_TraceId_EmptyGuid() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "00000000000000000000000000000000"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.0"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_TraceId_Invalid() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "not-a-guid"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.0"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_PublicKey_Missing() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-sample_rate", "1.0"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_PublicKey_Blank() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", " "}, + {"sentry-sample_rate", "1.0"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_SampleRate_Missing() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_SampleRate_Invalid() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "not-a-number"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_SampleRate_TooLow() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "-0.1"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_SampleRate_TooHigh() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.1"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.Null(dsc); + } + + [Fact] + public void CreateFromBaggage_Valid_Minimum() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.0"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.NotNull(dsc); + Assert.Equal(3, dsc.Items.Count); + Assert.Equal("43365712692146d08ee11a729dfbcaca", Assert.Contains("trace_id", dsc.Items)); + Assert.Equal("d4d82fc1c2c4032a83f3a29aa3a3aff", Assert.Contains("public_key", dsc.Items)); + Assert.Equal("1.0", Assert.Contains("sample_rate", dsc.Items)); + } + + [Fact] + public void CreateFromBaggage_Valid_Complete() + { + var baggage = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.0"}, + {"sentry-release", "test@1.0.0+abc"}, + {"sentry-environment", "production"}, + {"sentry-user_segment", "Group B"}, + {"sentry-transaction", "GET /person/{id}"} + }); + + var dsc = baggage.CreateDynamicSamplingContext(); + + Assert.NotNull(dsc); + Assert.Equal(7, dsc.Items.Count); + Assert.Equal("43365712692146d08ee11a729dfbcaca", Assert.Contains("trace_id", dsc.Items)); + Assert.Equal("d4d82fc1c2c4032a83f3a29aa3a3aff", Assert.Contains("public_key", dsc.Items)); + Assert.Equal("1.0", Assert.Contains("sample_rate", dsc.Items)); + Assert.Equal("test@1.0.0+abc", Assert.Contains("release", dsc.Items)); + Assert.Equal("production", Assert.Contains("environment", dsc.Items)); + Assert.Equal("Group B", Assert.Contains("user_segment", dsc.Items)); + Assert.Equal("GET /person/{id}", Assert.Contains("transaction", dsc.Items)); + } + + [Fact] + public void ToBaggageHeader() + { + var original = BaggageHeader.Create(new List> + { + {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, + {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, + {"sentry-sample_rate", "1.0"}, + {"sentry-release", "test@1.0.0+abc"}, + {"sentry-environment", "production"}, + {"sentry-user_segment", "Group B"}, + {"sentry-transaction", "GET /person/{id}"} + }); + + var dsc = original.CreateDynamicSamplingContext(); + + var result = dsc?.ToBaggageHeader(); + + Assert.NotNull(dsc); + Assert.Equal(original.Members, result.Members); + } + + [Fact] + public void CreateFromTransaction() + { + var options = new SentryOptions + { + Dsn = ValidDsn, + Release = "foo@2.4.5", + Environment = "staging" + }; + + var hub = Substitute.For(); + var ctx = Substitute.For(); + + var traceId = SentryId.Create(); + ctx.TraceId.Returns(traceId); + + var transaction = new TransactionTracer(hub, ctx) + { + Name = "GET /person/{id}", + NameSource = TransactionNameSource.Route, + IsSampled = true, + SampleRate = 0.5, + User = + { + Segment = "Group A" + }, + }; + + var dsc = transaction.CreateDynamicSamplingContext(options); + + Assert.NotNull(dsc); + Assert.Equal(7, dsc.Items.Count); + Assert.Equal(traceId.ToString(), Assert.Contains("trace_id", dsc.Items)); + Assert.Equal("d4d82fc1c2c4032a83f3a29aa3a3aff", Assert.Contains("public_key", dsc.Items)); + Assert.Equal("0.5", Assert.Contains("sample_rate", dsc.Items)); + Assert.Equal("foo@2.4.5", Assert.Contains("release", dsc.Items)); + Assert.Equal("staging", Assert.Contains("environment", dsc.Items)); + Assert.Equal("Group A", Assert.Contains("user_segment", dsc.Items)); + Assert.Equal("GET /person/{id}", Assert.Contains("transaction", dsc.Items)); + } +} From 9b422741b6faeed5d3de96ca01ab9e7f325a6c1d Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Thu, 29 Sep 2022 11:12:46 -0700 Subject: [PATCH 15/29] Add ASP.NET support --- src/Sentry.AspNet/HttpContextExtensions.cs | 66 ++++++++++++++++--- .../SentryTracingMiddleware.cs | 3 + src/Sentry/SentrySdk.cs | 10 +++ 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/Sentry.AspNet/HttpContextExtensions.cs b/src/Sentry.AspNet/HttpContextExtensions.cs index 7f143a318d..8234b62963 100644 --- a/src/Sentry.AspNet/HttpContextExtensions.cs +++ b/src/Sentry.AspNet/HttpContextExtensions.cs @@ -1,6 +1,6 @@ -using System.Collections.Specialized; using System.ComponentModel; using System.Web; +using Sentry.Extensibility; namespace Sentry.AspNet; @@ -12,20 +12,47 @@ public static class HttpContextExtensions { private const string HttpContextTransactionItemName = "__SentryTransaction"; - private static SentryTraceHeader? TryGetTraceHeader(NameValueCollection headers) + private static SentryTraceHeader? TryGetSentryTraceHeader(HttpContext context, SentryOptions? options) { - var traceHeader = headers.Get(SentryTraceHeader.HttpHeaderName); - if (traceHeader == null) + var value = context.Request.Headers.Get(SentryTraceHeader.HttpHeaderName); + if (string.IsNullOrWhiteSpace(value)) { return null; } + options?.LogDebug("Received Sentry trace header '{0}'.", value); + + try + { + return SentryTraceHeader.Parse(value); + } + catch (Exception ex) + { + options?.LogError("Invalid Sentry trace header '{0}'.", ex, value); + return null; + } + } + + private static BaggageHeader? TryGetBaggageHeader(HttpContext context, SentryOptions? options) + { + var value = context.Request.Headers.Get(SentryTraceHeader.HttpHeaderName); + if (string.IsNullOrWhiteSpace(value)) + { + return null; + } + + // Note: If there are multiple baggage headers, they will be joined with comma delimiters, + // and can thus be treated as a single baggage header. + + options?.LogDebug("Received baggage header '{0}'.", value); + try { - return SentryTraceHeader.Parse(traceHeader); + return BaggageHeader.TryParse(value, onlySentry: true); } - catch + catch (Exception ex) { + options?.LogError("Invalid baggage header '{0}'.", ex, value); return null; } } @@ -37,8 +64,9 @@ public static ITransaction StartSentryTransaction(this HttpContext httpContext) { var method = httpContext.Request.HttpMethod; var path = httpContext.Request.Path; + var options = SentrySdk.CurrentOptions; - var traceHeader = TryGetTraceHeader(httpContext.Request.Headers); + var traceHeader = TryGetSentryTraceHeader(httpContext, options); var transactionName = $"{method} {path}"; const string transactionOperation = "http.server"; @@ -47,7 +75,29 @@ public static ITransaction StartSentryTransaction(this HttpContext httpContext) ? new TransactionContext(transactionName, transactionOperation, traceHeader, TransactionNameSource.Url) : new TransactionContext(transactionName, transactionOperation, TransactionNameSource.Url); - var transaction = SentrySdk.StartTransaction(transactionContext); + var customSamplingContext = new Dictionary(3, StringComparer.Ordinal) + { + ["__HttpMethod"] = method, + ["__HttpPath"] = path, + ["__HttpContext"] = httpContext, + }; + + // Set the Dynamic Sampling Context from the baggage header, if it exists. + var baggageHeader = TryGetBaggageHeader(httpContext, options); + var dynamicSamplingContext = baggageHeader?.CreateDynamicSamplingContext(); + + if (traceHeader is { } && baggageHeader is null) + { + // We received a sentry-trace header without a baggage header, which indicates the request + // originated from an older SDK that doesn't support dynamic sampling. + // Set DynamicSamplingContext.Empty to "freeze" the DSC on the transaction. + // See: + // https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#freezing-dynamic-sampling-context + // https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/#unified-propagation-mechanism + dynamicSamplingContext = DynamicSamplingContext.Empty; + } + + var transaction = SentrySdk.StartTransaction(transactionContext, customSamplingContext, dynamicSamplingContext); SentrySdk.ConfigureScope(scope => scope.Transaction = transaction); httpContext.Items[HttpContextTransactionItemName] = transaction; diff --git a/src/Sentry.AspNetCore/SentryTracingMiddleware.cs b/src/Sentry.AspNetCore/SentryTracingMiddleware.cs index cc312f8923..e73e7b7e2f 100644 --- a/src/Sentry.AspNetCore/SentryTracingMiddleware.cs +++ b/src/Sentry.AspNetCore/SentryTracingMiddleware.cs @@ -59,6 +59,9 @@ public SentryTracingMiddleware( return null; } + // Note: If there are multiple baggage headers, they will be joined with comma delimiters, + // and can thus be treated as a single baggage header. + _options.LogDebug("Received baggage header '{0}'.", value); try diff --git a/src/Sentry/SentrySdk.cs b/src/Sentry/SentrySdk.cs index cd7b5b616b..38977c9274 100644 --- a/src/Sentry/SentrySdk.cs +++ b/src/Sentry/SentrySdk.cs @@ -395,6 +395,16 @@ public static ITransaction StartTransaction( IReadOnlyDictionary customSamplingContext) => CurrentHub.StartTransaction(context, customSamplingContext); + /// + /// Starts a transaction. + /// + [DebuggerStepThrough] + internal static ITransaction StartTransaction( + ITransactionContext context, + IReadOnlyDictionary customSamplingContext, + DynamicSamplingContext? dynamicSamplingContext) + => CurrentHub.StartTransaction(context, customSamplingContext, dynamicSamplingContext); + /// /// Starts a transaction. /// From 0f457b22171aa04be721bd851fd40e34b0fb76ec Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Thu, 29 Sep 2022 13:46:37 -0700 Subject: [PATCH 16/29] Add baggage propagation test --- .../SentryTracingMiddlewareTests.cs | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs index 57d82882c0..e3399464ed 100644 --- a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs @@ -213,6 +213,85 @@ public async Task TraceID_from_trace_header_propagates_to_outbound_requests() h.Value.First().StartsWith("75302ac48a024bde9a3b3734a82e36c8-")); } + [Fact] + public async Task Baggage_header_propagates_to_outbound_requests() + { + // incoming baggage header + const string incomingBaggage = + "sentry-trace_id=75302ac48a024bde9a3b3734a82e36c8, " + + "sentry-public_key=d4d82fc1c2c4032a83f3a29aa3a3aff, " + + "sentry-sample_rate=0.5, " + + "foo-bar=abc123"; + + // other baggage already on the outbound request (manually in this test, but in theory by some other middleware) + const string existingOutboundBaggage = "other-value=abc123"; + + // we expect this to be the result on outbound requests + const string expectedOutboundBaggage = + "other-value=abc123, " + + "sentry-trace_id=75302ac48a024bde9a3b3734a82e36c8, " + + "sentry-public_key=d4d82fc1c2c4032a83f3a29aa3a3aff, " + + "sentry-sample_rate=0.5"; + + // Note that we "play nice" with existing headers on the outbound request, but we do not propagate other + // non-Sentry headers on the inbound request. The expectation is that the other vendor would add their + // own middleware to do that. + + // Arrange + var sentryClient = Substitute.For(); + + var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); + + HttpRequestHeaders outboundRequestHeaders = null; + + var server = new TestServer(new WebHostBuilder() + .UseDefaultServiceProvider(di => di.EnableValidation()) + .UseSentry() + .ConfigureServices(services => + { + services.AddRouting(); + + services.RemoveAll(typeof(Func)); + services.AddSingleton>(() => hub); + }) + .Configure(app => + { + app.UseRouting(); + app.UseSentryTracing(); + + app.UseEndpoints(routes => routes.Map("/person/{id}", async _ => + { + // simulate an outbound request and capture the request headers + using var innerHandler = new RecordingHttpMessageHandler(new FakeHttpMessageHandler()); + using var sentryHandler = new SentryHttpMessageHandler(innerHandler, hub); + using var client = new HttpClient(sentryHandler); + client.DefaultRequestHeaders.Add("baggage", existingOutboundBaggage); + await client.GetAsync("https://localhost/"); + using var request = innerHandler.GetRequests().Single(); + outboundRequestHeaders = request.Headers; + })); + })); + + var client = server.CreateClient(); + + // Act + using var request = new HttpRequestMessage(HttpMethod.Get, "/person/13") + { + Headers = + { + {"baggage", incomingBaggage} + } + }; + + await client.SendAsync(request); + + // Assert + Assert.NotNull(outboundRequestHeaders); + outboundRequestHeaders.Should().Contain(h => + h.Key == "baggage" && + h.Value.First() == expectedOutboundBaggage); + } + [Fact] public async Task Transaction_is_automatically_populated_with_request_data() { From 21884380cfacf7d2d0f77a7f892617a69359f036 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Thu, 29 Sep 2022 15:11:49 -0700 Subject: [PATCH 17/29] DSC end-to-end test --- .../SentryTracingMiddlewareTests.cs | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs index e3399464ed..0d8f6a1b59 100644 --- a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs @@ -292,6 +292,66 @@ public async Task Baggage_header_propagates_to_outbound_requests() h.Value.First() == expectedOutboundBaggage); } + [Fact] + public async Task Baggage_header_sets_dynamic_sampling_context() + { + // incoming baggage header + const string baggage = + "sentry-trace_id=75302ac48a024bde9a3b3734a82e36c8, " + + "sentry-public_key=d4d82fc1c2c4032a83f3a29aa3a3aff, " + + "sentry-sample_rate=0.5"; + + // Arrange + TransactionTracer transaction = null; + + var sentryClient = Substitute.For(); + + var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); + + var server = new TestServer(new WebHostBuilder() + .UseDefaultServiceProvider(di => di.EnableValidation()) + .UseSentry() + .ConfigureServices(services => + { + services.AddRouting(); + + services.RemoveAll(typeof(Func)); + services.AddSingleton>(() => hub); + }) + .Configure(app => + { + app.UseRouting(); + app.UseSentryTracing(); + + app.UseEndpoints(routes => + { + routes.Map("/person/{id}", _ => + { + transaction = hub.GetSpan() as TransactionTracer; + return Task.CompletedTask; + }); + }); + })); + + var client = server.CreateClient(); + + // Act + using var request = new HttpRequestMessage(HttpMethod.Get, "/person/13") + { + Headers = + { + {"baggage", baggage} + } + }; + + await client.SendAsync(request); + + // Assert + var dsc = transaction?.DynamicSamplingContext; + Assert.NotNull(dsc); + Assert.Equal(baggage, dsc.ToBaggageHeader().ToString()); + } + [Fact] public async Task Transaction_is_automatically_populated_with_request_data() { From abe26e53ee5dcdc701690023c0b66cc94f1c3f89 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Fri, 30 Sep 2022 17:52:12 -0700 Subject: [PATCH 18/29] Add some logging --- src/Sentry/BaggageHeader.cs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/Sentry/BaggageHeader.cs b/src/Sentry/BaggageHeader.cs index 224109eadd..8267433745 100644 --- a/src/Sentry/BaggageHeader.cs +++ b/src/Sentry/BaggageHeader.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Sentry.Extensibility; using Sentry.Internal.Extensions; namespace Sentry @@ -22,7 +23,7 @@ internal class BaggageHeader public IReadOnlyList> Members { get; } private BaggageHeader(IEnumerable> members) => - Members = members.ToList().AsReadOnly(); + Members = members.ToList(); // We can safely return a dictionary of Sentry members, as we are in control over the keys added. // Just to be safe though, we'll group by key and only take the first of each one. @@ -71,21 +72,35 @@ public override string ToString() var items = baggage.Split(',', StringSplitOptions.RemoveEmptyEntries); var members = new List>(items.Length); + var logger = SentrySdk.CurrentOptions?.DiagnosticLogger; + foreach (var item in items) { // Per baggage spec, the value may contain = characters, so limit the split to 2 parts. var parts = item.Split('=', 2); if (parts.Length != 2) { - // malformed, missing separator, key, or value + logger?.LogWarning( + "The baggage header has an item without a '=' separator, and it will be discarded. " + + "The item is: \"{0}\"", item); continue; } var key = parts[0].Trim(); + if (key.Length == 0) + { + logger?.LogWarning( + "The baggage header has an item with an empty key, and it will be discarded. " + + "The item is: \"{0}\"", item); + continue; + } + var value = parts[1].Trim(); - if (key.Length == 0 || value.Length == 0) + if (value.Length == 0) { - // malformed, key or value found empty + logger?.LogWarning( + "The baggage header has an item with an empty value, and it will be discarded. " + + "The item is: \"{0}\"", item); continue; } From 34ddb3c2766a46d81a616d66b2a8107517fe08fc Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Fri, 30 Sep 2022 18:08:12 -0700 Subject: [PATCH 19/29] misc --- src/Sentry.AspNetCore/SentryTracingMiddleware.cs | 1 - .../Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Sentry.AspNetCore/SentryTracingMiddleware.cs b/src/Sentry.AspNetCore/SentryTracingMiddleware.cs index e73e7b7e2f..57d2bb3baf 100644 --- a/src/Sentry.AspNetCore/SentryTracingMiddleware.cs +++ b/src/Sentry.AspNetCore/SentryTracingMiddleware.cs @@ -5,7 +5,6 @@ using Microsoft.Extensions.Options; using Sentry.AspNetCore.Extensions; using Sentry.Extensibility; -using Sentry.Internal; namespace Sentry.AspNetCore { diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs index 0d8f6a1b59..2b0db494c8 100644 --- a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs @@ -90,7 +90,7 @@ public async Task Transaction_is_bound_on_the_scope_automatically() { routes.Map("/person/{id}", _ => { - transaction = hub.GetSpan() as ITransactionData; + transaction = (ITransactionData) hub.GetSpan(); return Task.CompletedTask; }); }); @@ -327,7 +327,7 @@ public async Task Baggage_header_sets_dynamic_sampling_context() { routes.Map("/person/{id}", _ => { - transaction = hub.GetSpan() as TransactionTracer; + transaction = (TransactionTracer) hub.GetSpan(); return Task.CompletedTask; }); }); @@ -381,7 +381,7 @@ public async Task Transaction_is_automatically_populated_with_request_data() { routes.Map("/person/{id}", _ => { - transaction = hub.GetSpan() as ITransactionData; + transaction = (ITransactionData) hub.GetSpan(); return Task.CompletedTask; }); }); From 34f997dcacd0b959fa662db687123c004bf386bf Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Mon, 3 Oct 2022 16:41:31 -0700 Subject: [PATCH 20/29] Get logger once in static initialization --- src/Sentry/BaggageHeader.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Sentry/BaggageHeader.cs b/src/Sentry/BaggageHeader.cs index 8267433745..57fcb60209 100644 --- a/src/Sentry/BaggageHeader.cs +++ b/src/Sentry/BaggageHeader.cs @@ -16,6 +16,8 @@ internal class BaggageHeader internal const string HttpHeaderName = "baggage"; internal const string SentryKeyPrefix = "sentry-"; + internal static IDiagnosticLogger? Logger { get; set; } = SentrySdk.CurrentOptions?.DiagnosticLogger; + // https://www.w3.org/TR/baggage/#baggage-string // "Uniqueness of keys between multiple list-members in a baggage-string is not guaranteed." // "The order of duplicate entries SHOULD be preserved when mutating the list." @@ -72,15 +74,13 @@ public override string ToString() var items = baggage.Split(',', StringSplitOptions.RemoveEmptyEntries); var members = new List>(items.Length); - var logger = SentrySdk.CurrentOptions?.DiagnosticLogger; - foreach (var item in items) { // Per baggage spec, the value may contain = characters, so limit the split to 2 parts. var parts = item.Split('=', 2); if (parts.Length != 2) { - logger?.LogWarning( + Logger?.LogWarning( "The baggage header has an item without a '=' separator, and it will be discarded. " + "The item is: \"{0}\"", item); continue; @@ -89,7 +89,7 @@ public override string ToString() var key = parts[0].Trim(); if (key.Length == 0) { - logger?.LogWarning( + Logger?.LogWarning( "The baggage header has an item with an empty key, and it will be discarded. " + "The item is: \"{0}\"", item); continue; @@ -98,7 +98,7 @@ public override string ToString() var value = parts[1].Trim(); if (value.Length == 0) { - logger?.LogWarning( + Logger?.LogWarning( "The baggage header has an item with an empty value, and it will be discarded. " + "The item is: \"{0}\"", item); continue; From cec62714b0e71b77c07727f08ab6cc12cdf97969 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Mon, 3 Oct 2022 16:42:36 -0700 Subject: [PATCH 21/29] Add extension method for getting options --- src/Sentry/SentryClient.cs | 2 +- src/Sentry/SentryClientExtensions.cs | 11 +++++++++++ src/Sentry/SentrySdk.cs | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Sentry/SentryClient.cs b/src/Sentry/SentryClient.cs index 3ce545b653..5d0c7d7016 100644 --- a/src/Sentry/SentryClient.cs +++ b/src/Sentry/SentryClient.cs @@ -23,8 +23,8 @@ public class SentryClient : ISentryClient, IDisposable private readonly SentryOptions _options; private readonly RandomValuesFactory _randomValuesFactory; - // Internal for testing. internal IBackgroundWorker Worker { get; } + internal SentryOptions Options => _options; /// /// Whether the client is enabled. diff --git a/src/Sentry/SentryClientExtensions.cs b/src/Sentry/SentryClientExtensions.cs index 5af7b57807..a224b8816f 100644 --- a/src/Sentry/SentryClientExtensions.cs +++ b/src/Sentry/SentryClientExtensions.cs @@ -1,5 +1,7 @@ using System; using System.ComponentModel; +using Sentry.Extensibility; +using Sentry.Internal; namespace Sentry { @@ -59,5 +61,14 @@ public static void CaptureUserFeedback(this ISentryClient client, SentryId event client.CaptureUserFeedback(new UserFeedback(eventId, name, email, comments)); } } + + internal static SentryOptions? GetSentryOptions(this ISentryClient clientOrHub) => + clientOrHub switch + { + SentryClient client => client.Options, + Hub hub => hub.Options, + HubAdapter => SentrySdk.CurrentOptions, + _ => null + }; } } diff --git a/src/Sentry/SentrySdk.cs b/src/Sentry/SentrySdk.cs index 38977c9274..4d6aa8e061 100644 --- a/src/Sentry/SentrySdk.cs +++ b/src/Sentry/SentrySdk.cs @@ -26,7 +26,7 @@ public static class SentrySdk { private static IHub CurrentHub = DisabledHub.Instance; - internal static SentryOptions? CurrentOptions => CurrentHub is Hub hub ? hub.Options : null; + internal static SentryOptions? CurrentOptions => CurrentHub.GetSentryOptions(); /// /// Last event id recorded in the current scope. From 60287d473ba22b9e723e4b672e8b33306810f389 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Mon, 3 Oct 2022 16:43:07 -0700 Subject: [PATCH 22/29] Add TracePropogationTargets --- src/Sentry/Internal/Polyfills.cs | 3 + src/Sentry/SentryHttpMessageHandler.cs | 31 +++++++--- src/Sentry/SentryOptions.cs | 10 ++++ src/Sentry/TracePropagationTarget.cs | 83 ++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 8 deletions(-) create mode 100644 src/Sentry/TracePropagationTarget.cs diff --git a/src/Sentry/Internal/Polyfills.cs b/src/Sentry/Internal/Polyfills.cs index 61055454ad..a8d0fb675d 100644 --- a/src/Sentry/Internal/Polyfills.cs +++ b/src/Sentry/Internal/Polyfills.cs @@ -27,6 +27,9 @@ public static string[] Split(this string str, char c, int count, StringSplitOpti public static bool Contains(this string str, char c) => str.IndexOf(c) >= 0; + public static bool Contains(this string str, string value, StringComparison comparisonType) => + str.IndexOf(value, comparisonType) >= 0; + public static Task CopyToAsync(this Stream stream, Stream destination, CancellationToken cancellationToken) => stream.CopyToAsync(destination, 81920, cancellationToken); diff --git a/src/Sentry/SentryHttpMessageHandler.cs b/src/Sentry/SentryHttpMessageHandler.cs index 660d8745bf..7229b31304 100644 --- a/src/Sentry/SentryHttpMessageHandler.cs +++ b/src/Sentry/SentryHttpMessageHandler.cs @@ -15,6 +15,7 @@ namespace Sentry public class SentryHttpMessageHandler : DelegatingHandler { private readonly IHub _hub; + private readonly SentryOptions? _options; /// /// Initializes an instance of . @@ -22,6 +23,7 @@ public class SentryHttpMessageHandler : DelegatingHandler public SentryHttpMessageHandler(IHub hub) { _hub = hub; + _options = hub.GetSentryOptions(); } /// @@ -50,14 +52,6 @@ protected override async Task SendAsync( HttpRequestMessage request, CancellationToken cancellationToken) { - // Set trace header if it hasn't already been set - if (!request.Headers.Contains(SentryTraceHeader.HttpHeaderName) && _hub.GetTraceHeader() is { } traceHeader) - { - request.Headers.Add(SentryTraceHeader.HttpHeaderName, traceHeader.ToString()); - } - - AddBaggageHeader(request); - // Prevent null reference exception in the following call // in case the user didn't set an inner handler. InnerHandler ??= new HttpClientHandler(); @@ -65,6 +59,12 @@ protected override async Task SendAsync( var requestMethod = request.Method.Method.ToUpperInvariant(); var url = request.RequestUri?.ToString() ?? string.Empty; + if (ShouldPropagateTrace(url)) + { + AddSentryTraceHeader(request); + AddBaggageHeader(request); + } + // Start a span that tracks this request // (may be null if transaction is not set on the scope) var span = _hub.GetSpan()?.StartChild( @@ -96,6 +96,21 @@ protected override async Task SendAsync( } } + private bool ShouldPropagateTrace(string url) + { + var targets = _options?.TracePropagationTargets; + return targets?.Any(t => t.IsMatch(url)) is null or true; + } + + private void AddSentryTraceHeader(HttpRequestMessage request) + { + // Set trace header if it hasn't already been set + if (!request.Headers.Contains(SentryTraceHeader.HttpHeaderName) && _hub.GetTraceHeader() is { } traceHeader) + { + request.Headers.Add(SentryTraceHeader.HttpHeaderName, traceHeader.ToString()); + } + } + private void AddBaggageHeader(HttpRequestMessage request) { var transaction = _hub.GetSpan(); diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index 6164f8f78d..f4486c366a 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -574,6 +574,16 @@ public double TracesSampleRate /// public Func? TracesSampler { get; set; } + /// + /// A customizable list of objects, each containing either a + /// substring or regular expression pattern that can be used to control which outgoing HTTP requests + /// will have the sentry-trace and baggage headers propagated, for purposes of distributed tracing. + /// The default value is null, which indicates that the headers will be propagated to ALL targets. + /// To disable propagation completely, set this value to an empty collection. + /// + /// + public ICollection? TracePropagationTargets { get; set; } = null; + private StackTraceMode? _stackTraceMode; /// diff --git a/src/Sentry/TracePropagationTarget.cs b/src/Sentry/TracePropagationTarget.cs new file mode 100644 index 0000000000..304602ee31 --- /dev/null +++ b/src/Sentry/TracePropagationTarget.cs @@ -0,0 +1,83 @@ +using System; +using System.Text.RegularExpressions; + +namespace Sentry +{ + /// + /// Provides a pattern that can be used to identify which destinations will have sentry-trace and + /// baggage headers propagated to, for purposes of distributed tracing. + /// The pattern can either be a substring or a regular expression. + /// + /// + public class TracePropagationTarget + { + private readonly Regex? _regex; + private readonly string? _substring; + private readonly StringComparison _stringComparison; + + private TracePropagationTarget(string substring, StringComparison comparison) + { + _substring = substring; + _stringComparison = comparison; + } + + private TracePropagationTarget(Regex regex) => _regex = regex; + + /// + /// Creates a instance that will match when the provided + /// is contained within the outgoing request URL. + /// + /// The substring to match. + /// + /// Whether the matching is case sensitive. Defaults to false (case insensitive). + /// + /// The constructed instance. + public static TracePropagationTarget CreateFromSubstring(string substring, bool caseSensitive = false) => + new(substring, caseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase); + + /// + /// Creates a instance that will match when the provided + /// is a regular expression pattern that matches the outgoing request URL. + /// + /// The regular expression pattern to match. + /// + /// Whether the matching is case sensitive. Defaults to false (case insensitive). + /// + /// The constructed instance. + /// + /// Sets and always. + /// Sets when is true. + /// + public static TracePropagationTarget CreateFromRegex(string pattern, bool caseSensitive = false) + { + var regexOptions = + RegexOptions.Compiled | + RegexOptions.CultureInvariant | + (caseSensitive ? RegexOptions.IgnoreCase : RegexOptions.None); + + var regex = new Regex(pattern, regexOptions); + return new TracePropagationTarget(regex); + } + + /// + /// Creates a instance that will match when the provided + /// is a regular expression object that matches the outgoing request URL. + /// + /// The regular expression object to match. + /// The constructed instance. + /// + /// Use this overload when you need to control the regular expression matching options. + /// We recommend setting at least for performance, and + /// (unless you have culture-specific matching needs). + /// The overload sets these by default. + /// + public static TracePropagationTarget CreateFromRegex(Regex regex) => new(regex); + + /// + public override string ToString() => _substring ?? _regex?.ToString() ?? ""; + + internal bool IsMatch(string url) => + _regex?.IsMatch(url) == true || + (_substring != null && url.Contains(_substring, _stringComparison)); + } +} From 3a7e9192add427b697db6ed8d0cb41e13139842a Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Mon, 3 Oct 2022 16:47:07 -0700 Subject: [PATCH 23/29] Update API verifications --- .../ApiApprovalTests.Run.Core3_1.verified.txt | 8 ++++++++ .../ApiApprovalTests.Run.DotNet4_8.verified.txt | 8 ++++++++ .../ApiApprovalTests.Run.DotNet6_0.verified.txt | 8 ++++++++ 3 files changed, 24 insertions(+) diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt index e72485e063..d06fa98259 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt @@ -517,6 +517,7 @@ namespace Sentry public string? ServerName { get; set; } public System.TimeSpan ShutdownTimeout { get; set; } public Sentry.StackTraceMode StackTraceMode { get; set; } + public System.Collections.Generic.ICollection? TracePropagationTargets { get; set; } public double TracesSampleRate { get; set; } public System.Func? TracesSampler { get; set; } public Sentry.Extensibility.ITransport? Transport { get; set; } @@ -815,6 +816,13 @@ namespace Sentry public StreamAttachmentContent(System.IO.Stream stream) { } public System.IO.Stream GetStream() { } } + public class TracePropagationTarget + { + public override string ToString() { } + public static Sentry.TracePropagationTarget CreateFromRegex(System.Text.RegularExpressions.Regex regex) { } + public static Sentry.TracePropagationTarget CreateFromRegex(string pattern, bool caseSensitive = false) { } + public static Sentry.TracePropagationTarget CreateFromSubstring(string substring, bool caseSensitive = false) { } + } public class Transaction : Sentry.IEventLike, Sentry.IHasBreadcrumbs, Sentry.IHasExtra, Sentry.IHasTags, Sentry.IHasTransactionNameSource, Sentry.IJsonSerializable, Sentry.ISpanContext, Sentry.ISpanData, Sentry.ITransactionContext, Sentry.ITransactionData, Sentry.Protocol.ITraceContext { public Transaction(Sentry.ITransaction tracer) { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt index 7004476547..e423f7f242 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt @@ -516,6 +516,7 @@ namespace Sentry public string? ServerName { get; set; } public System.TimeSpan ShutdownTimeout { get; set; } public Sentry.StackTraceMode StackTraceMode { get; set; } + public System.Collections.Generic.ICollection? TracePropagationTargets { get; set; } public double TracesSampleRate { get; set; } public System.Func? TracesSampler { get; set; } public Sentry.Extensibility.ITransport? Transport { get; set; } @@ -814,6 +815,13 @@ namespace Sentry public StreamAttachmentContent(System.IO.Stream stream) { } public System.IO.Stream GetStream() { } } + public class TracePropagationTarget + { + public override string ToString() { } + public static Sentry.TracePropagationTarget CreateFromRegex(System.Text.RegularExpressions.Regex regex) { } + public static Sentry.TracePropagationTarget CreateFromRegex(string pattern, bool caseSensitive = false) { } + public static Sentry.TracePropagationTarget CreateFromSubstring(string substring, bool caseSensitive = false) { } + } public class Transaction : Sentry.IEventLike, Sentry.IHasBreadcrumbs, Sentry.IHasExtra, Sentry.IHasTags, Sentry.IHasTransactionNameSource, Sentry.IJsonSerializable, Sentry.ISpanContext, Sentry.ISpanData, Sentry.ITransactionContext, Sentry.ITransactionData, Sentry.Protocol.ITraceContext { public Transaction(Sentry.ITransaction tracer) { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index e72485e063..d06fa98259 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt @@ -517,6 +517,7 @@ namespace Sentry public string? ServerName { get; set; } public System.TimeSpan ShutdownTimeout { get; set; } public Sentry.StackTraceMode StackTraceMode { get; set; } + public System.Collections.Generic.ICollection? TracePropagationTargets { get; set; } public double TracesSampleRate { get; set; } public System.Func? TracesSampler { get; set; } public Sentry.Extensibility.ITransport? Transport { get; set; } @@ -815,6 +816,13 @@ namespace Sentry public StreamAttachmentContent(System.IO.Stream stream) { } public System.IO.Stream GetStream() { } } + public class TracePropagationTarget + { + public override string ToString() { } + public static Sentry.TracePropagationTarget CreateFromRegex(System.Text.RegularExpressions.Regex regex) { } + public static Sentry.TracePropagationTarget CreateFromRegex(string pattern, bool caseSensitive = false) { } + public static Sentry.TracePropagationTarget CreateFromSubstring(string substring, bool caseSensitive = false) { } + } public class Transaction : Sentry.IEventLike, Sentry.IHasBreadcrumbs, Sentry.IHasExtra, Sentry.IHasTags, Sentry.IHasTransactionNameSource, Sentry.IJsonSerializable, Sentry.ISpanContext, Sentry.ISpanData, Sentry.ITransactionContext, Sentry.ITransactionData, Sentry.Protocol.ITraceContext { public Transaction(Sentry.ITransaction tracer) { } From 9bf673b36045a43310a1fb9f44bbcebcbd3ac3fe Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Mon, 3 Oct 2022 17:24:40 -0700 Subject: [PATCH 24/29] Refactor and add tests --- src/Sentry/SentryHttpMessageHandler.cs | 8 +- src/Sentry/TracePropagationTarget.cs | 10 +- .../TracePropagationTargetTests.cs | 185 ++++++++++++++++++ 3 files changed, 195 insertions(+), 8 deletions(-) create mode 100644 test/Sentry.Tests/TracePropagationTargetTests.cs diff --git a/src/Sentry/SentryHttpMessageHandler.cs b/src/Sentry/SentryHttpMessageHandler.cs index 7229b31304..38bb1615dd 100644 --- a/src/Sentry/SentryHttpMessageHandler.cs +++ b/src/Sentry/SentryHttpMessageHandler.cs @@ -59,7 +59,7 @@ protected override async Task SendAsync( var requestMethod = request.Method.Method.ToUpperInvariant(); var url = request.RequestUri?.ToString() ?? string.Empty; - if (ShouldPropagateTrace(url)) + if ((_options?.TracePropagationTargets).ShouldPropagateTrace(url)) { AddSentryTraceHeader(request); AddBaggageHeader(request); @@ -96,12 +96,6 @@ protected override async Task SendAsync( } } - private bool ShouldPropagateTrace(string url) - { - var targets = _options?.TracePropagationTargets; - return targets?.Any(t => t.IsMatch(url)) is null or true; - } - private void AddSentryTraceHeader(HttpRequestMessage request) { // Set trace header if it hasn't already been set diff --git a/src/Sentry/TracePropagationTarget.cs b/src/Sentry/TracePropagationTarget.cs index 304602ee31..548f118ad8 100644 --- a/src/Sentry/TracePropagationTarget.cs +++ b/src/Sentry/TracePropagationTarget.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Linq; using System.Text.RegularExpressions; namespace Sentry @@ -53,7 +55,7 @@ public static TracePropagationTarget CreateFromRegex(string pattern, bool caseSe var regexOptions = RegexOptions.Compiled | RegexOptions.CultureInvariant | - (caseSensitive ? RegexOptions.IgnoreCase : RegexOptions.None); + (caseSensitive ? RegexOptions.None : RegexOptions.IgnoreCase); var regex = new Regex(pattern, regexOptions); return new TracePropagationTarget(regex); @@ -80,4 +82,10 @@ internal bool IsMatch(string url) => _regex?.IsMatch(url) == true || (_substring != null && url.Contains(_substring, _stringComparison)); } + + internal static class TracePropagationTargetExtensions + { + public static bool ShouldPropagateTrace(this IEnumerable? targets, string url) => + targets?.Any(t => t.IsMatch(url)) is null or true; + } } diff --git a/test/Sentry.Tests/TracePropagationTargetTests.cs b/test/Sentry.Tests/TracePropagationTargetTests.cs new file mode 100644 index 0000000000..799cd0750e --- /dev/null +++ b/test/Sentry.Tests/TracePropagationTargetTests.cs @@ -0,0 +1,185 @@ +using System.Text.RegularExpressions; + +namespace Sentry.Tests; + +public class TracePropagationTargetTests +{ + [Fact] + public void Substring_Matches() + { + var target = TracePropagationTarget.CreateFromSubstring("cde"); + var isMatch = target.IsMatch("abcdef"); + Assert.True(isMatch); + } + + [Fact] + public void Substring_Doesnt_Match() + { + var target = TracePropagationTarget.CreateFromSubstring("xyz"); + var isMatch = target.IsMatch("abcdef"); + Assert.False(isMatch); + } + + [Fact] + public void Substring_Matches_CaseInsensitive_ByDefault() + { + var target = TracePropagationTarget.CreateFromSubstring("cDe"); + var isMatch = target.IsMatch("ABCdEF"); + Assert.True(isMatch); + } + + [Fact] + public void Substring_Matches_CaseSensitive() + { + var target = TracePropagationTarget.CreateFromSubstring("CdE", caseSensitive: true); + var isMatch = target.IsMatch("ABCdEF"); + Assert.True(isMatch); + } + + [Fact] + public void Substring_Doesnt_Match_WhenCaseSensitive() + { + var target = TracePropagationTarget.CreateFromSubstring("cDe", caseSensitive: true); + var isMatch = target.IsMatch("ABCdEF"); + Assert.False(isMatch); + } + + [Fact] + public void Regex_Object_Matches() + { + var regex = new Regex("^abc.*ghi$"); + var target = TracePropagationTarget.CreateFromRegex(regex); + var isMatch = target.IsMatch("abcdefghi"); + Assert.True(isMatch); + } + + [Fact] + public void Regex_Object_Doesnt_Match() + { + var regex = new Regex("^abc.*ghi$"); + var target = TracePropagationTarget.CreateFromRegex(regex); + var isMatch = target.IsMatch("abcdef"); + Assert.False(isMatch); + } + + [Fact] + public void Regex_Pattern_Matches() + { + var target = TracePropagationTarget.CreateFromRegex("^abc.*ghi$"); + var isMatch = target.IsMatch("abcdefghi"); + Assert.True(isMatch); + } + + [Fact] + public void Regex_Pattern_Matches_CaseInsensitive_ByDefault() + { + var target = TracePropagationTarget.CreateFromRegex("^abc.*ghi$"); + var isMatch = target.IsMatch("aBcDeFgHi"); + Assert.True(isMatch); + } + + [Fact] + public void Regex_Pattern_Matches_CaseSensitive() + { + var target = TracePropagationTarget.CreateFromRegex("^aBc.*gHi$", caseSensitive: true); + var isMatch = target.IsMatch("aBcDeFgHi"); + Assert.True(isMatch); + } + + [Fact] + public void Regex_Pattern_Doesnt_Match_WhenCaseSensitive() + { + var target = TracePropagationTarget.CreateFromRegex("^abc.*ghi$", caseSensitive: true); + var isMatch = target.IsMatch("aBcDeFgHi"); + Assert.False(isMatch); + } + + [Fact] + public void SentryOptions_TracePropagationTargets_DefaultNull() + { + var options = new SentryOptions(); + Assert.Null(options.TracePropagationTargets); + } + + [Fact] + public void SentryOptions_TracePropagationTargets_DefaultPropagatesAll() + { + var options = new SentryOptions(); + + var result1 = options.TracePropagationTargets.ShouldPropagateTrace("foo"); + var result2 = options.TracePropagationTargets.ShouldPropagateTrace(""); + var result3 = options.TracePropagationTargets.ShouldPropagateTrace(null!); + + Assert.True(result1); + Assert.True(result2); + Assert.True(result3); + } + + [Fact] + public void SentryOptions_TracePropagationTargets_EmptyPropagatesNone() + { + var options = new SentryOptions + { + TracePropagationTargets = new List() + }; + + var result1 = options.TracePropagationTargets.ShouldPropagateTrace("foo"); + var result2 = options.TracePropagationTargets.ShouldPropagateTrace(""); + var result3 = options.TracePropagationTargets.ShouldPropagateTrace(null!); + + Assert.False(result1); + Assert.False(result2); + Assert.False(result3); + } + + [Fact] + public void SentryOptions_TracePropagationTargets_OneMatchPropagates() + { + var options = new SentryOptions + { + TracePropagationTargets = new List + { + TracePropagationTarget.CreateFromSubstring("foo"), + TracePropagationTarget.CreateFromSubstring("localhost"), + TracePropagationTarget.CreateFromSubstring("bar") + } + }; + + var result = options.TracePropagationTargets.ShouldPropagateTrace("http://localhost/abc/123"); + Assert.True(result); + } + + [Fact] + public void SentryOptions_TracePropagationTargets_MultipleMatchesPropagates() + { + var options = new SentryOptions + { + TracePropagationTargets = new List + { + TracePropagationTarget.CreateFromSubstring("foo"), + TracePropagationTarget.CreateFromSubstring("localhost"), + TracePropagationTarget.CreateFromSubstring("bar") + } + }; + + var result = options.TracePropagationTargets.ShouldPropagateTrace("http://localhost/foo/123"); + Assert.True(result); + } + + [Fact] + public void SentryOptions_TracePropagationTargets_NoMatchesDoesntPropagates() + { + var options = new SentryOptions + { + TracePropagationTargets = new List + { + TracePropagationTarget.CreateFromSubstring("foo"), + TracePropagationTarget.CreateFromSubstring("localhost"), + TracePropagationTarget.CreateFromSubstring("bar") + } + }; + + var result = options.TracePropagationTargets.ShouldPropagateTrace("https://sentry.io/abc/123"); + Assert.False(result); + } +} From 9940846b74d61b4a7c65ab4491ea7fe764395279 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Mon, 3 Oct 2022 17:46:56 -0700 Subject: [PATCH 25/29] Add tests --- src/Sentry/SentryHttpMessageHandler.cs | 12 ++++ .../SentryHttpMessageHandlerTests.cs | 62 ++++++++++++++++++- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/Sentry/SentryHttpMessageHandler.cs b/src/Sentry/SentryHttpMessageHandler.cs index 38bb1615dd..38dc39d4d0 100644 --- a/src/Sentry/SentryHttpMessageHandler.cs +++ b/src/Sentry/SentryHttpMessageHandler.cs @@ -26,6 +26,12 @@ public SentryHttpMessageHandler(IHub hub) _options = hub.GetSentryOptions(); } + internal SentryHttpMessageHandler(IHub hub, SentryOptions options) + { + _hub = hub; + _options = options; + } + /// /// Initializes an instance of . /// @@ -35,6 +41,12 @@ public SentryHttpMessageHandler(HttpMessageHandler innerHandler, IHub hub) InnerHandler = innerHandler; } + internal SentryHttpMessageHandler(HttpMessageHandler innerHandler, IHub hub, SentryOptions options) + : this(hub, options) + { + InnerHandler = innerHandler; + } + /// /// Initializes an instance of . /// diff --git a/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs b/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs index 46e1f9ae92..d3ffc8d16f 100644 --- a/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs +++ b/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs @@ -6,7 +6,7 @@ namespace Sentry.Tests; public class SentryHttpMessageHandlerTests { [Fact] - public async Task SendAsync_SentryTraceHeaderNotSet_SetsHeader() + public async Task SendAsync_SentryTraceHeaderNotSet_SetsHeader_ByDefault() { // Arrange var hub = Substitute.For(); @@ -29,6 +29,66 @@ public async Task SendAsync_SentryTraceHeaderNotSet_SetsHeader() string.Concat(h.Value) == "75302ac48a024bde9a3b3734a82e36c8-1000000000000000-0"); } + [Fact] + public async Task SendAsync_SentryTraceHeaderNotSet_SetsHeader_WhenUrlMatchesPropagationOptions() + { + // Arrange + var hub = Substitute.For(); + var options = new SentryOptions + { + TracePropagationTargets = new List + { + TracePropagationTarget.CreateFromSubstring("localhost") + } + }; + + hub.GetTraceHeader().ReturnsForAnyArgs( + SentryTraceHeader.Parse("75302ac48a024bde9a3b3734a82e36c8-1000000000000000-0")); + + using var innerHandler = new RecordingHttpMessageHandler(new FakeHttpMessageHandler()); + using var sentryHandler = new SentryHttpMessageHandler(innerHandler, hub, options); + using var client = new HttpClient(sentryHandler); + + // Act + await client.GetAsync("https://localhost/"); + + using var request = innerHandler.GetRequests().Single(); + + // Assert + request.Headers.Should().Contain(h => + h.Key == "sentry-trace" && + string.Concat(h.Value) == "75302ac48a024bde9a3b3734a82e36c8-1000000000000000-0"); + } + + [Fact] + public async Task SendAsync_SentryTraceHeaderNotSet_DoesntSetHeader_WhenUrlDoesntMatchesPropagationOptions() + { + // Arrange + var hub = Substitute.For(); + var options = new SentryOptions + { + TracePropagationTargets = new List + { + TracePropagationTarget.CreateFromSubstring("foo") + } + }; + + hub.GetTraceHeader().ReturnsForAnyArgs( + SentryTraceHeader.Parse("75302ac48a024bde9a3b3734a82e36c8-1000000000000000-0")); + + using var innerHandler = new RecordingHttpMessageHandler(new FakeHttpMessageHandler()); + using var sentryHandler = new SentryHttpMessageHandler(innerHandler, hub, options); + using var client = new HttpClient(sentryHandler); + + // Act + await client.GetAsync("https://localhost/"); + + using var request = innerHandler.GetRequests().Single(); + + // Assert + request.Headers.Should().NotContain(h => h.Key == "sentry-trace"); + } + [Fact] public async Task SendAsync_SentryTraceHeaderAlreadySet_NotOverwritten() { From 77812f4b1af90aee98c9d35f9cd20c1136728d13 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Mon, 3 Oct 2022 17:55:35 -0700 Subject: [PATCH 26/29] Add tests --- .../SentryTracingMiddlewareTests.cs | 69 +++++++++++++++---- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs index 2b0db494c8..96645543f6 100644 --- a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs @@ -156,13 +156,26 @@ public async Task Transaction_is_started_automatically_from_incoming_trace_heade )); } - [Fact] - public async Task TraceID_from_trace_header_propagates_to_outbound_requests() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task TraceID_from_trace_header_propagates_to_outbound_requests(bool shouldPropagate) { // Arrange var sentryClient = Substitute.For(); - var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); + var options = new SentryOptions + { + Dsn = ValidDsn, + TracesSampleRate = 1 + }; + + if (!shouldPropagate) + { + options.TracePropagationTargets = Array.Empty(); + } + + var hub = new Hub(options, sentryClient); HttpRequestHeaders outboundRequestHeaders = null; @@ -208,13 +221,22 @@ public async Task TraceID_from_trace_header_propagates_to_outbound_requests() // Assert Assert.NotNull(outboundRequestHeaders); - outboundRequestHeaders.Should().Contain(h => - h.Key == "sentry-trace" && - h.Value.First().StartsWith("75302ac48a024bde9a3b3734a82e36c8-")); + if (shouldPropagate) + { + outboundRequestHeaders.Should().Contain(h => + h.Key == "sentry-trace" && + h.Value.First().StartsWith("75302ac48a024bde9a3b3734a82e36c8-")); + } + else + { + outboundRequestHeaders.Should().NotContain(h => h.Key == "sentry-trace"); + } } - [Fact] - public async Task Baggage_header_propagates_to_outbound_requests() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Baggage_header_propagates_to_outbound_requests(bool shouldPropagate) { // incoming baggage header const string incomingBaggage = @@ -227,11 +249,19 @@ public async Task Baggage_header_propagates_to_outbound_requests() const string existingOutboundBaggage = "other-value=abc123"; // we expect this to be the result on outbound requests - const string expectedOutboundBaggage = - "other-value=abc123, " + - "sentry-trace_id=75302ac48a024bde9a3b3734a82e36c8, " + - "sentry-public_key=d4d82fc1c2c4032a83f3a29aa3a3aff, " + - "sentry-sample_rate=0.5"; + string expectedOutboundBaggage; + if (shouldPropagate) + { + expectedOutboundBaggage = + "other-value=abc123, " + + "sentry-trace_id=75302ac48a024bde9a3b3734a82e36c8, " + + "sentry-public_key=d4d82fc1c2c4032a83f3a29aa3a3aff, " + + "sentry-sample_rate=0.5"; + } + else + { + expectedOutboundBaggage = "other-value=abc123"; + } // Note that we "play nice" with existing headers on the outbound request, but we do not propagate other // non-Sentry headers on the inbound request. The expectation is that the other vendor would add their @@ -240,7 +270,18 @@ public async Task Baggage_header_propagates_to_outbound_requests() // Arrange var sentryClient = Substitute.For(); - var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient); + var options = new SentryOptions + { + Dsn = ValidDsn, + TracesSampleRate = 1 + }; + + if (!shouldPropagate) + { + options.TracePropagationTargets = Array.Empty(); + } + + var hub = new Hub(options, sentryClient); HttpRequestHeaders outboundRequestHeaders = null; From c7e92ad05c842f24f4a10fb64007d52064e79c7c Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Mon, 3 Oct 2022 18:31:21 -0700 Subject: [PATCH 27/29] Support setting option via json config file --- src/Sentry/TracePropagationTarget.cs | 21 +++++++++++++++++++ .../IntegrationMockedBackgroundWorker.cs | 4 ++++ test/Sentry.AspNetCore.Tests/allsettings.json | 10 +++++++-- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/Sentry/TracePropagationTarget.cs b/src/Sentry/TracePropagationTarget.cs index 548f118ad8..048c59198c 100644 --- a/src/Sentry/TracePropagationTarget.cs +++ b/src/Sentry/TracePropagationTarget.cs @@ -1,5 +1,7 @@ using System; using System.Collections.Generic; +using System.ComponentModel; +using System.Globalization; using System.Linq; using System.Text.RegularExpressions; @@ -11,6 +13,7 @@ namespace Sentry /// The pattern can either be a substring or a regular expression. /// /// + [TypeConverter(typeof(TracePropagationTargetTypeConverter))] public class TracePropagationTarget { private readonly Regex? _regex; @@ -88,4 +91,22 @@ internal static class TracePropagationTargetExtensions public static bool ShouldPropagateTrace(this IEnumerable? targets, string url) => targets?.Any(t => t.IsMatch(url)) is null or true; } + + internal class TracePropagationTargetTypeConverter : TypeConverter + { + // This class allows the TracePropagationTargets option to be set from config, such as appSettings.json + + public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) + { + return sourceType == typeof(string); + } + + public override object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value) + { + var s = (string)value; + return s.StartsWith("regex:") + ? TracePropagationTarget.CreateFromRegex(s.Substring(6)) + : TracePropagationTarget.CreateFromSubstring(s); + } + } } diff --git a/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs b/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs index f7bb5f82ff..36ce6a8f23 100644 --- a/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs +++ b/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs @@ -268,6 +268,10 @@ public void AllSettingsViaJson() Assert.Equal(1, options.SampleRate); Assert.Equal("7f5d9a1", options.Release); Assert.Equal("Staging", options.Environment); + Assert.Equal(1, options.TracesSampleRate); + + var targets = options.TracePropagationTargets?.Select(t => t.ToString()); + Assert.Equal(new[] {"foo", "bar", "^abc.*ghi$"}, targets); } [Fact] diff --git a/test/Sentry.AspNetCore.Tests/allsettings.json b/test/Sentry.AspNetCore.Tests/allsettings.json index 4be2835072..acbb5fdd17 100644 --- a/test/Sentry.AspNetCore.Tests/allsettings.json +++ b/test/Sentry.AspNetCore.Tests/allsettings.json @@ -8,8 +8,14 @@ "MinimumEventLevel": "Critical", "InitializeSdk": "false", "MaxBreadcrumbs": "999", - "SampleRate": "1", + "SampleRate": 1.0, "Release": "7f5d9a1", - "Environment": "Staging" + "Environment": "Staging", + "TracesSampleRate": 1.0, + "TracePropagationTargets": [ + "foo", + "bar", + "regex:^abc.*ghi$" + ] } } From 3409cdf2ddf1d898e4cd4a164e223e0fb5ace1e6 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Mon, 3 Oct 2022 18:33:52 -0700 Subject: [PATCH 28/29] Update API verifications --- test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt | 1 + test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt | 1 + test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt | 1 + 3 files changed, 3 insertions(+) diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt index d06fa98259..3b2dc33978 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt @@ -816,6 +816,7 @@ namespace Sentry public StreamAttachmentContent(System.IO.Stream stream) { } public System.IO.Stream GetStream() { } } + [System.ComponentModel.TypeConverter(typeof(Sentry.TracePropagationTargetTypeConverter))] public class TracePropagationTarget { public override string ToString() { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt index e423f7f242..fa5766c030 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt @@ -815,6 +815,7 @@ namespace Sentry public StreamAttachmentContent(System.IO.Stream stream) { } public System.IO.Stream GetStream() { } } + [System.ComponentModel.TypeConverter(typeof(Sentry.TracePropagationTargetTypeConverter))] public class TracePropagationTarget { public override string ToString() { } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index d06fa98259..3b2dc33978 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt @@ -816,6 +816,7 @@ namespace Sentry public StreamAttachmentContent(System.IO.Stream stream) { } public System.IO.Stream GetStream() { } } + [System.ComponentModel.TypeConverter(typeof(Sentry.TracePropagationTargetTypeConverter))] public class TracePropagationTarget { public override string ToString() { } From 6474cccc23012f003cc6bf57055ec6c9b747a4dc Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Tue, 4 Oct 2022 11:04:00 -0700 Subject: [PATCH 29/29] Update TracePropagationTargets implementation --- src/Sentry/Internal/AutoClearingList.cs | 60 +++++++++ src/Sentry/SentryHttpMessageHandler.cs | 2 +- src/Sentry/SentryOptions.cs | 13 +- src/Sentry/TracePropagationTarget.cs | 119 +++++++++--------- .../IntegrationMockedBackgroundWorker.cs | 2 +- .../SentryTracingMiddlewareTests.cs | 4 +- test/Sentry.AspNetCore.Tests/allsettings.json | 2 +- .../ApiApprovalTests.Run.Core3_1.verified.txt | 7 +- ...piApprovalTests.Run.DotNet4_8.verified.txt | 7 +- ...piApprovalTests.Run.DotNet6_0.verified.txt | 7 +- .../SentryHttpMessageHandlerTests.cs | 4 +- .../TracePropagationTargetTests.cs | 57 +++++---- 12 files changed, 179 insertions(+), 105 deletions(-) create mode 100644 src/Sentry/Internal/AutoClearingList.cs diff --git a/src/Sentry/Internal/AutoClearingList.cs b/src/Sentry/Internal/AutoClearingList.cs new file mode 100644 index 0000000000..4a7d2f6ed2 --- /dev/null +++ b/src/Sentry/Internal/AutoClearingList.cs @@ -0,0 +1,60 @@ +using System.Collections; +using System.Collections.Generic; + +namespace Sentry.Internal +{ + // Workaround for the fact that setting a list in config options appends instead of replaces. + // See https://github.com/dotnet/runtime/issues/36569 + + internal class AutoClearingList : IList + { + private readonly IList _list = new List(); + + private bool _clearOnNextAdd; + + public void Add(T item) + { + if (_clearOnNextAdd) + { + Clear(); + _clearOnNextAdd = false; + } + + _list.Add(item); + } + + public AutoClearingList ClearOnNextAdd() + { + _clearOnNextAdd = true; + return this; + } + + public IEnumerator GetEnumerator() => _list.GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable)_list).GetEnumerator(); + + public void Clear() => _list.Clear(); + + public bool Contains(T item) => _list.Contains(item); + + public void CopyTo(T[] array, int arrayIndex) => _list.CopyTo(array, arrayIndex); + + public bool Remove(T item) => _list.Remove(item); + + public int Count => _list.Count; + + public bool IsReadOnly => _list.IsReadOnly; + + public int IndexOf(T item) => _list.IndexOf(item); + + public void Insert(int index, T item) => _list.Insert(index, item); + + public void RemoveAt(int index) => _list.RemoveAt(index); + + public T this[int index] + { + get => _list[index]; + set => _list[index] = value; + } + } +} diff --git a/src/Sentry/SentryHttpMessageHandler.cs b/src/Sentry/SentryHttpMessageHandler.cs index 38dc39d4d0..d234ac0eb7 100644 --- a/src/Sentry/SentryHttpMessageHandler.cs +++ b/src/Sentry/SentryHttpMessageHandler.cs @@ -71,7 +71,7 @@ protected override async Task SendAsync( var requestMethod = request.Method.Method.ToUpperInvariant(); var url = request.RequestUri?.ToString() ?? string.Empty; - if ((_options?.TracePropagationTargets).ShouldPropagateTrace(url)) + if (_options?.TracePropagationTargets.ShouldPropagateTrace(url) is true or null) { AddSentryTraceHeader(request); AddBaggageHeader(request); diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index f4486c366a..d4cd180ed7 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -578,11 +578,18 @@ public double TracesSampleRate /// A customizable list of objects, each containing either a /// substring or regular expression pattern that can be used to control which outgoing HTTP requests /// will have the sentry-trace and baggage headers propagated, for purposes of distributed tracing. - /// The default value is null, which indicates that the headers will be propagated to ALL targets. - /// To disable propagation completely, set this value to an empty collection. + /// The default value contains a single value of .*, which matches everything. + /// To disable propagation completely, clear this collection or set it to an empty collection. /// /// - public ICollection? TracePropagationTargets { get; set; } = null; + /// + /// Adding an item to the default list will clear the .* value automatically. + /// + public IList TracePropagationTargets { get; set; } = + new AutoClearingList + { + new(".*") + }.ClearOnNextAdd(); private StackTraceMode? _stackTraceMode; diff --git a/src/Sentry/TracePropagationTarget.cs b/src/Sentry/TracePropagationTarget.cs index 048c59198c..fd090a0cb1 100644 --- a/src/Sentry/TracePropagationTarget.cs +++ b/src/Sentry/TracePropagationTarget.cs @@ -20,93 +20,90 @@ public class TracePropagationTarget private readonly string? _substring; private readonly StringComparison _stringComparison; - private TracePropagationTarget(string substring, StringComparison comparison) - { - _substring = substring; - _stringComparison = comparison; - } - - private TracePropagationTarget(Regex regex) => _regex = regex; - /// - /// Creates a instance that will match when the provided - /// is contained within the outgoing request URL. + /// Constructs a instance that will match when the provided + /// is either found as a substring within the outgoing request URL, + /// or matches as a regular expression pattern against the outgoing request URL. /// - /// The substring to match. - /// - /// Whether the matching is case sensitive. Defaults to false (case insensitive). - /// - /// The constructed instance. - public static TracePropagationTarget CreateFromSubstring(string substring, bool caseSensitive = false) => - new(substring, caseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase); - - /// - /// Creates a instance that will match when the provided - /// is a regular expression pattern that matches the outgoing request URL. - /// - /// The regular expression pattern to match. - /// - /// Whether the matching is case sensitive. Defaults to false (case insensitive). - /// - /// The constructed instance. - /// - /// Sets and always. - /// Sets when is true. - /// - public static TracePropagationTarget CreateFromRegex(string pattern, bool caseSensitive = false) + /// The substring or regular expression pattern to match on. + /// The string comparison type to use when matching. + public TracePropagationTarget( + string substringOrRegexPattern, + StringComparison comparison = StringComparison.OrdinalIgnoreCase) { - var regexOptions = - RegexOptions.Compiled | - RegexOptions.CultureInvariant | - (caseSensitive ? RegexOptions.None : RegexOptions.IgnoreCase); - - var regex = new Regex(pattern, regexOptions); - return new TracePropagationTarget(regex); + _substring = substringOrRegexPattern; + _stringComparison = comparison; + _regex = TryParseRegex(substringOrRegexPattern, comparison); } /// - /// Creates a instance that will match when the provided - /// is a regular expression object that matches the outgoing request URL. + /// Constructs a instance that will match when the provided + /// object matches the outgoing request URL. /// - /// The regular expression object to match. - /// The constructed instance. + /// /// - /// Use this overload when you need to control the regular expression matching options. + /// Use this constructor when you need to control the regular expression matching options. /// We recommend setting at least for performance, and /// (unless you have culture-specific matching needs). - /// The overload sets these by default. + /// The constructor sets these by default. /// - public static TracePropagationTarget CreateFromRegex(Regex regex) => new(regex); + public TracePropagationTarget(Regex regex) => _regex = regex; /// public override string ToString() => _substring ?? _regex?.ToString() ?? ""; internal bool IsMatch(string url) => - _regex?.IsMatch(url) == true || - (_substring != null && url.Contains(_substring, _stringComparison)); + _substring == ".*" || // perf shortcut + (_substring != null && url.Contains(_substring, _stringComparison)) || + _regex?.IsMatch(url) == true; + + private static Regex? TryParseRegex(string pattern, StringComparison comparison) + { + try + { + var regexOptions = RegexOptions.Compiled; + + if (comparison is + StringComparison.InvariantCulture or + StringComparison.InvariantCultureIgnoreCase or + StringComparison.Ordinal or + StringComparison.OrdinalIgnoreCase) + { + regexOptions |= RegexOptions.CultureInvariant; + } + + if (comparison is + StringComparison.CurrentCultureIgnoreCase or + StringComparison.InvariantCultureIgnoreCase or + StringComparison.OrdinalIgnoreCase) + { + regexOptions |= RegexOptions.IgnoreCase; + } + + return new Regex(pattern, regexOptions); + } + catch + { + // not a valid regex + return null; + } + } } internal static class TracePropagationTargetExtensions { - public static bool ShouldPropagateTrace(this IEnumerable? targets, string url) => - targets?.Any(t => t.IsMatch(url)) is null or true; + public static bool ShouldPropagateTrace(this IEnumerable targets, string url) => + targets.Any(t => t.IsMatch(url)); } internal class TracePropagationTargetTypeConverter : TypeConverter { // This class allows the TracePropagationTargets option to be set from config, such as appSettings.json - public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) - { - return sourceType == typeof(string); - } + public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) => + sourceType == typeof(string); - public override object? ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value) - { - var s = (string)value; - return s.StartsWith("regex:") - ? TracePropagationTarget.CreateFromRegex(s.Substring(6)) - : TracePropagationTarget.CreateFromSubstring(s); - } + public override object ConvertFrom(ITypeDescriptorContext? context, CultureInfo? culture, object value) => + new TracePropagationTarget((string)value); } } diff --git a/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs b/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs index 36ce6a8f23..f8f1aa49ff 100644 --- a/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs +++ b/test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs @@ -270,7 +270,7 @@ public void AllSettingsViaJson() Assert.Equal("Staging", options.Environment); Assert.Equal(1, options.TracesSampleRate); - var targets = options.TracePropagationTargets?.Select(t => t.ToString()); + var targets = options.TracePropagationTargets.Select(t => t.ToString()); Assert.Equal(new[] {"foo", "bar", "^abc.*ghi$"}, targets); } diff --git a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs index 96645543f6..163d39266d 100644 --- a/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs @@ -172,7 +172,7 @@ public async Task TraceID_from_trace_header_propagates_to_outbound_requests(bool if (!shouldPropagate) { - options.TracePropagationTargets = Array.Empty(); + options.TracePropagationTargets.Clear(); } var hub = new Hub(options, sentryClient); @@ -278,7 +278,7 @@ public async Task Baggage_header_propagates_to_outbound_requests(bool shouldProp if (!shouldPropagate) { - options.TracePropagationTargets = Array.Empty(); + options.TracePropagationTargets.Clear(); } var hub = new Hub(options, sentryClient); diff --git a/test/Sentry.AspNetCore.Tests/allsettings.json b/test/Sentry.AspNetCore.Tests/allsettings.json index acbb5fdd17..427091798c 100644 --- a/test/Sentry.AspNetCore.Tests/allsettings.json +++ b/test/Sentry.AspNetCore.Tests/allsettings.json @@ -15,7 +15,7 @@ "TracePropagationTargets": [ "foo", "bar", - "regex:^abc.*ghi$" + "^abc.*ghi$" ] } } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt index 3b2dc33978..7a5eb85da0 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt @@ -517,7 +517,7 @@ namespace Sentry public string? ServerName { get; set; } public System.TimeSpan ShutdownTimeout { get; set; } public Sentry.StackTraceMode StackTraceMode { get; set; } - public System.Collections.Generic.ICollection? TracePropagationTargets { get; set; } + public System.Collections.Generic.IList TracePropagationTargets { get; set; } public double TracesSampleRate { get; set; } public System.Func? TracesSampler { get; set; } public Sentry.Extensibility.ITransport? Transport { get; set; } @@ -819,10 +819,9 @@ namespace Sentry [System.ComponentModel.TypeConverter(typeof(Sentry.TracePropagationTargetTypeConverter))] public class TracePropagationTarget { + public TracePropagationTarget(System.Text.RegularExpressions.Regex regex) { } + public TracePropagationTarget(string substringOrRegexPattern, System.StringComparison comparison = 5) { } public override string ToString() { } - public static Sentry.TracePropagationTarget CreateFromRegex(System.Text.RegularExpressions.Regex regex) { } - public static Sentry.TracePropagationTarget CreateFromRegex(string pattern, bool caseSensitive = false) { } - public static Sentry.TracePropagationTarget CreateFromSubstring(string substring, bool caseSensitive = false) { } } public class Transaction : Sentry.IEventLike, Sentry.IHasBreadcrumbs, Sentry.IHasExtra, Sentry.IHasTags, Sentry.IHasTransactionNameSource, Sentry.IJsonSerializable, Sentry.ISpanContext, Sentry.ISpanData, Sentry.ITransactionContext, Sentry.ITransactionData, Sentry.Protocol.ITraceContext { diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt index fa5766c030..2376f3aec1 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet4_8.verified.txt @@ -516,7 +516,7 @@ namespace Sentry public string? ServerName { get; set; } public System.TimeSpan ShutdownTimeout { get; set; } public Sentry.StackTraceMode StackTraceMode { get; set; } - public System.Collections.Generic.ICollection? TracePropagationTargets { get; set; } + public System.Collections.Generic.IList TracePropagationTargets { get; set; } public double TracesSampleRate { get; set; } public System.Func? TracesSampler { get; set; } public Sentry.Extensibility.ITransport? Transport { get; set; } @@ -818,10 +818,9 @@ namespace Sentry [System.ComponentModel.TypeConverter(typeof(Sentry.TracePropagationTargetTypeConverter))] public class TracePropagationTarget { + public TracePropagationTarget(System.Text.RegularExpressions.Regex regex) { } + public TracePropagationTarget(string substringOrRegexPattern, System.StringComparison comparison = 5) { } public override string ToString() { } - public static Sentry.TracePropagationTarget CreateFromRegex(System.Text.RegularExpressions.Regex regex) { } - public static Sentry.TracePropagationTarget CreateFromRegex(string pattern, bool caseSensitive = false) { } - public static Sentry.TracePropagationTarget CreateFromSubstring(string substring, bool caseSensitive = false) { } } public class Transaction : Sentry.IEventLike, Sentry.IHasBreadcrumbs, Sentry.IHasExtra, Sentry.IHasTags, Sentry.IHasTransactionNameSource, Sentry.IJsonSerializable, Sentry.ISpanContext, Sentry.ISpanData, Sentry.ITransactionContext, Sentry.ITransactionData, Sentry.Protocol.ITraceContext { diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt index 3b2dc33978..7a5eb85da0 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt @@ -517,7 +517,7 @@ namespace Sentry public string? ServerName { get; set; } public System.TimeSpan ShutdownTimeout { get; set; } public Sentry.StackTraceMode StackTraceMode { get; set; } - public System.Collections.Generic.ICollection? TracePropagationTargets { get; set; } + public System.Collections.Generic.IList TracePropagationTargets { get; set; } public double TracesSampleRate { get; set; } public System.Func? TracesSampler { get; set; } public Sentry.Extensibility.ITransport? Transport { get; set; } @@ -819,10 +819,9 @@ namespace Sentry [System.ComponentModel.TypeConverter(typeof(Sentry.TracePropagationTargetTypeConverter))] public class TracePropagationTarget { + public TracePropagationTarget(System.Text.RegularExpressions.Regex regex) { } + public TracePropagationTarget(string substringOrRegexPattern, System.StringComparison comparison = 5) { } public override string ToString() { } - public static Sentry.TracePropagationTarget CreateFromRegex(System.Text.RegularExpressions.Regex regex) { } - public static Sentry.TracePropagationTarget CreateFromRegex(string pattern, bool caseSensitive = false) { } - public static Sentry.TracePropagationTarget CreateFromSubstring(string substring, bool caseSensitive = false) { } } public class Transaction : Sentry.IEventLike, Sentry.IHasBreadcrumbs, Sentry.IHasExtra, Sentry.IHasTags, Sentry.IHasTransactionNameSource, Sentry.IJsonSerializable, Sentry.ISpanContext, Sentry.ISpanData, Sentry.ITransactionContext, Sentry.ITransactionData, Sentry.Protocol.ITraceContext { diff --git a/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs b/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs index d3ffc8d16f..32137cedc3 100644 --- a/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs +++ b/test/Sentry.Tests/SentryHttpMessageHandlerTests.cs @@ -38,7 +38,7 @@ public async Task SendAsync_SentryTraceHeaderNotSet_SetsHeader_WhenUrlMatchesPro { TracePropagationTargets = new List { - TracePropagationTarget.CreateFromSubstring("localhost") + new("localhost") } }; @@ -69,7 +69,7 @@ public async Task SendAsync_SentryTraceHeaderNotSet_DoesntSetHeader_WhenUrlDoesn { TracePropagationTargets = new List { - TracePropagationTarget.CreateFromSubstring("foo") + new("foo") } }; diff --git a/test/Sentry.Tests/TracePropagationTargetTests.cs b/test/Sentry.Tests/TracePropagationTargetTests.cs index 799cd0750e..371b47f092 100644 --- a/test/Sentry.Tests/TracePropagationTargetTests.cs +++ b/test/Sentry.Tests/TracePropagationTargetTests.cs @@ -7,7 +7,7 @@ public class TracePropagationTargetTests [Fact] public void Substring_Matches() { - var target = TracePropagationTarget.CreateFromSubstring("cde"); + var target = new TracePropagationTarget("cde"); var isMatch = target.IsMatch("abcdef"); Assert.True(isMatch); } @@ -15,7 +15,7 @@ public void Substring_Matches() [Fact] public void Substring_Doesnt_Match() { - var target = TracePropagationTarget.CreateFromSubstring("xyz"); + var target = new TracePropagationTarget("xyz"); var isMatch = target.IsMatch("abcdef"); Assert.False(isMatch); } @@ -23,7 +23,7 @@ public void Substring_Doesnt_Match() [Fact] public void Substring_Matches_CaseInsensitive_ByDefault() { - var target = TracePropagationTarget.CreateFromSubstring("cDe"); + var target = new TracePropagationTarget("cDe"); var isMatch = target.IsMatch("ABCdEF"); Assert.True(isMatch); } @@ -31,7 +31,7 @@ public void Substring_Matches_CaseInsensitive_ByDefault() [Fact] public void Substring_Matches_CaseSensitive() { - var target = TracePropagationTarget.CreateFromSubstring("CdE", caseSensitive: true); + var target = new TracePropagationTarget("CdE", StringComparison.Ordinal); var isMatch = target.IsMatch("ABCdEF"); Assert.True(isMatch); } @@ -39,7 +39,7 @@ public void Substring_Matches_CaseSensitive() [Fact] public void Substring_Doesnt_Match_WhenCaseSensitive() { - var target = TracePropagationTarget.CreateFromSubstring("cDe", caseSensitive: true); + var target = new TracePropagationTarget("cDe", StringComparison.Ordinal); var isMatch = target.IsMatch("ABCdEF"); Assert.False(isMatch); } @@ -48,7 +48,7 @@ public void Substring_Doesnt_Match_WhenCaseSensitive() public void Regex_Object_Matches() { var regex = new Regex("^abc.*ghi$"); - var target = TracePropagationTarget.CreateFromRegex(regex); + var target = new TracePropagationTarget(regex); var isMatch = target.IsMatch("abcdefghi"); Assert.True(isMatch); } @@ -57,7 +57,7 @@ public void Regex_Object_Matches() public void Regex_Object_Doesnt_Match() { var regex = new Regex("^abc.*ghi$"); - var target = TracePropagationTarget.CreateFromRegex(regex); + var target = new TracePropagationTarget(regex); var isMatch = target.IsMatch("abcdef"); Assert.False(isMatch); } @@ -65,7 +65,7 @@ public void Regex_Object_Doesnt_Match() [Fact] public void Regex_Pattern_Matches() { - var target = TracePropagationTarget.CreateFromRegex("^abc.*ghi$"); + var target = new TracePropagationTarget("^abc.*ghi$"); var isMatch = target.IsMatch("abcdefghi"); Assert.True(isMatch); } @@ -73,7 +73,7 @@ public void Regex_Pattern_Matches() [Fact] public void Regex_Pattern_Matches_CaseInsensitive_ByDefault() { - var target = TracePropagationTarget.CreateFromRegex("^abc.*ghi$"); + var target = new TracePropagationTarget("^abc.*ghi$"); var isMatch = target.IsMatch("aBcDeFgHi"); Assert.True(isMatch); } @@ -81,7 +81,7 @@ public void Regex_Pattern_Matches_CaseInsensitive_ByDefault() [Fact] public void Regex_Pattern_Matches_CaseSensitive() { - var target = TracePropagationTarget.CreateFromRegex("^aBc.*gHi$", caseSensitive: true); + var target = new TracePropagationTarget("^aBc.*gHi$", StringComparison.Ordinal); var isMatch = target.IsMatch("aBcDeFgHi"); Assert.True(isMatch); } @@ -89,16 +89,29 @@ public void Regex_Pattern_Matches_CaseSensitive() [Fact] public void Regex_Pattern_Doesnt_Match_WhenCaseSensitive() { - var target = TracePropagationTarget.CreateFromRegex("^abc.*ghi$", caseSensitive: true); + var target = new TracePropagationTarget("^abc.*ghi$", StringComparison.Ordinal); var isMatch = target.IsMatch("aBcDeFgHi"); Assert.False(isMatch); } [Fact] - public void SentryOptions_TracePropagationTargets_DefaultNull() + public void SentryOptions_TracePropagationTargets_DefaultAll() { var options = new SentryOptions(); - Assert.Null(options.TracePropagationTargets); + Assert.Equal(1, options.TracePropagationTargets.Count); + Assert.Equal(".*", options.TracePropagationTargets[0].ToString()); + } + + [Fact] + public void SentryOptions_TracePropagationTargets_AddRemovesDefault() + { + var options = new SentryOptions(); + options.TracePropagationTargets.Add(new TracePropagationTarget("foo")); + options.TracePropagationTargets.Add(new TracePropagationTarget("bar")); + + Assert.Equal(2, options.TracePropagationTargets.Count); + Assert.Equal("foo", options.TracePropagationTargets[0].ToString()); + Assert.Equal("bar", options.TracePropagationTargets[1].ToString()); } [Fact] @@ -139,9 +152,9 @@ public void SentryOptions_TracePropagationTargets_OneMatchPropagates() { TracePropagationTargets = new List { - TracePropagationTarget.CreateFromSubstring("foo"), - TracePropagationTarget.CreateFromSubstring("localhost"), - TracePropagationTarget.CreateFromSubstring("bar") + new("foo"), + new("localhost"), + new("bar") } }; @@ -156,9 +169,9 @@ public void SentryOptions_TracePropagationTargets_MultipleMatchesPropagates() { TracePropagationTargets = new List { - TracePropagationTarget.CreateFromSubstring("foo"), - TracePropagationTarget.CreateFromSubstring("localhost"), - TracePropagationTarget.CreateFromSubstring("bar") + new("foo"), + new("localhost"), + new("bar") } }; @@ -173,9 +186,9 @@ public void SentryOptions_TracePropagationTargets_NoMatchesDoesntPropagates() { TracePropagationTargets = new List { - TracePropagationTarget.CreateFromSubstring("foo"), - TracePropagationTarget.CreateFromSubstring("localhost"), - TracePropagationTarget.CreateFromSubstring("bar") + new("foo"), + new("localhost"), + new("bar") } };