Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Enrich transactions #2838

Merged
merged 9 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixes

- Don't add WinUI exception integration on mobile platforms ([#2821](https://github.com/getsentry/sentry-dotnet/pull/2821))
- `Transactions` are now getting enriched by the client instead of the hub ([#2838](https://github.com/getsentry/sentry-dotnet/pull/2838))

### API breaking Changes

Expand Down
2 changes: 1 addition & 1 deletion src/Sentry/Extensibility/DisabledHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public void CaptureTransaction(Transaction transaction)
/// <summary>
/// No-Op.
/// </summary>
public void CaptureTransaction(Transaction transaction, Hint? hint)
public void CaptureTransaction(Transaction transaction, Scope? scope, Hint? hint)
{
}

Expand Down
4 changes: 2 additions & 2 deletions src/Sentry/Extensibility/HubAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ public void CaptureTransaction(Transaction transaction)
/// </summary>
[DebuggerStepThrough]
[EditorBrowsable(EditorBrowsableState.Never)]
public void CaptureTransaction(Transaction transaction, Hint? hint)
=> SentrySdk.CaptureTransaction(transaction, hint);
public void CaptureTransaction(Transaction transaction, Scope? scope, Hint? hint)
=> SentrySdk.CaptureTransaction(transaction, scope, hint);

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
Expand Down
3 changes: 2 additions & 1 deletion src/Sentry/ISentryClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,13 @@ public interface ISentryClient
/// Instead, call <see cref="ISpanTracer.Finish()"/> on the transaction.
/// </remarks>
/// <param name="transaction">The transaction.</param>
/// <param name="scope">The scope to be applied to the transaction</param>
/// <param name="hint">
/// A hint providing extra context.
/// This will be available in callbacks prior to processing the transaction.
/// </param>
[EditorBrowsable(EditorBrowsableState.Never)]
void CaptureTransaction(Transaction transaction, Hint? hint);
void CaptureTransaction(Transaction transaction, Scope? scope, Hint? hint);

/// <summary>
/// Captures a session update.
Expand Down
37 changes: 5 additions & 32 deletions src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ internal class Hub : IHub, IDisposable
private readonly ISessionManager _sessionManager;
private readonly SentryOptions _options;
private readonly RandomValuesFactory _randomValuesFactory;
private readonly Enricher _enricher;

private int _isPersistedSessionRecovered;

Expand Down Expand Up @@ -59,8 +58,6 @@ internal class Hub : IHub, IDisposable
PushScope();
}

_enricher = new Enricher(options);

foreach (var integration in options.Integrations)
{
options.LogDebug("Registering integration: '{0}'.", integration.GetType().Name);
Expand Down Expand Up @@ -458,9 +455,9 @@ public void CaptureUserFeedback(UserFeedback userFeedback)
}
}

public void CaptureTransaction(Transaction transaction) => CaptureTransaction(transaction, null);
public void CaptureTransaction(Transaction transaction) => CaptureTransaction(transaction, null, null);

public void CaptureTransaction(Transaction transaction, Hint? hint)
public void CaptureTransaction(Transaction transaction, Scope? scope, Hint? hint)
{
// Note: The hub should capture transactions even if it is disabled.
// This allows transactions to be reported as failed when they encountered an unhandled exception,
Expand All @@ -473,34 +470,10 @@ public void CaptureTransaction(Transaction transaction, Hint? hint)

try
{
// Apply scope data
var (scope, client) = ScopeManager.GetCurrent();
scope.Evaluate();
scope.Apply(transaction);

// Apply enricher
_enricher.Apply(transaction);

// Add attachments to the hint for processors and callbacks
hint ??= new Hint();
hint.AddAttachmentsFromScope(scope);

var processedTransaction = transaction;
if (transaction.IsSampled != false)
{
foreach (var processor in scope.GetAllTransactionProcessors())
{
processedTransaction = processor.DoProcessTransaction(transaction, hint);
if (processedTransaction == null)
{
_options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Transaction);
_options.LogInfo("Event dropped by processor {0}", processor.GetType().Name);
return;
}
}
}
var (currentScope, client) = ScopeManager.GetCurrent();
scope ??= currentScope;

client.CaptureTransaction(processedTransaction, hint);
client.CaptureTransaction(transaction, scope, hint);
}
catch (Exception e)
{
Expand Down
34 changes: 29 additions & 5 deletions src/Sentry/SentryClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public class SentryClient : ISentryClient, IDisposable
private readonly SentryOptions _options;
private readonly ISessionManager _sessionManager;
private readonly RandomValuesFactory _randomValuesFactory;
private readonly Enricher _enricher;

internal IBackgroundWorker Worker { get; }
internal SentryOptions Options => _options;
Expand All @@ -44,6 +45,7 @@ public SentryClient(SentryOptions options)
_options = options ?? throw new ArgumentNullException(nameof(options));
_randomValuesFactory = randomValuesFactory ?? new SynchronizedRandomValuesFactory();
_sessionManager = sessionManager ?? new GlobalSessionManager(options);
_enricher = new Enricher(options);

options.SetupLogging(); // Only relevant if this client wasn't created as a result of calling Init

Expand Down Expand Up @@ -98,10 +100,10 @@ public void CaptureUserFeedback(UserFeedback userFeedback)
}

/// <inheritdoc />
public void CaptureTransaction(Transaction transaction) => CaptureTransaction(transaction, null);
public void CaptureTransaction(Transaction transaction) => CaptureTransaction(transaction, null, null);

/// <inheritdoc />
public void CaptureTransaction(Transaction transaction, Hint? hint)
public void CaptureTransaction(Transaction transaction, Scope? scope, Hint? hint)
{
if (transaction.SpanId.Equals(SpanId.Empty))
{
Expand All @@ -127,8 +129,7 @@ public void CaptureTransaction(Transaction transaction, Hint? hint)
}

// Sampling decision MUST have been made at this point
Debug.Assert(transaction.IsSampled is not null,
"Attempt to capture transaction without sampling decision.");
Debug.Assert(transaction.IsSampled is not null, "Attempt to capture transaction without sampling decision.");

if (transaction.IsSampled is false)
{
Expand All @@ -137,7 +138,30 @@ public void CaptureTransaction(Transaction transaction, Hint? hint)
return;
}

var processedTransaction = BeforeSendTransaction(transaction, hint ?? new Hint());
scope ??= new Scope(_options);
hint ??= new Hint();
hint.AddAttachmentsFromScope(scope);

_options.LogInfo("Capturing transaction.");

scope.Evaluate();
scope.Apply(transaction);

_enricher.Apply(transaction);

var processedTransaction = transaction;
foreach (var processor in scope.GetAllTransactionProcessors())
{
processedTransaction = processor.DoProcessTransaction(transaction, hint);
if (processedTransaction == null) // Rejected transaction
{
_options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Transaction);
_options.LogInfo("Event dropped by processor {0}", processor.GetType().Name);
return;
}
}

processedTransaction = BeforeSendTransaction(processedTransaction, hint);
if (processedTransaction is null) // Rejected transaction
{
_options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Transaction);
Expand Down
4 changes: 2 additions & 2 deletions src/Sentry/SentrySdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,8 @@ public static void CaptureTransaction(Transaction transaction)
/// </remarks>
[DebuggerStepThrough]
[EditorBrowsable(EditorBrowsableState.Never)]
public static void CaptureTransaction(Transaction transaction, Hint? hint)
=> CurrentHub.CaptureTransaction(transaction, hint);
public static void CaptureTransaction(Transaction transaction, Scope? scope, Hint? hint)
=> CurrentHub.CaptureTransaction(transaction, scope, hint);

/// <summary>
/// Captures a session update.
Expand Down
11 changes: 6 additions & 5 deletions test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public async Task Transactions_are_grouped_by_route()
Arg.Is<Transaction>(transaction =>
transaction.Name == "GET /person/{id}" &&
transaction.NameSource == TransactionNameSource.Route),
Arg.Any<Scope>(),
Arg.Any<Hint>()
);
}
Expand Down Expand Up @@ -151,6 +152,7 @@ public async Task Transaction_is_started_automatically_from_incoming_trace_heade
t.ParentSpanId == SpanId.Parse("1000000000000000") &&
t.IsSampled == false
),
Arg.Any<Scope>(),
Arg.Any<Hint>()
);
}
Expand Down Expand Up @@ -184,6 +186,7 @@ public async Task Transaction_name_includes_slash_prefix()
Transaction transaction = null;
sentryClient.CaptureTransaction(
Arg.Do<Transaction>(t => transaction = t),
Arg.Any<Scope>(),
Arg.Any<Hint>()
);

Expand Down Expand Up @@ -437,9 +440,7 @@ public async Task Transaction_is_automatically_populated_with_request_data()
// Arrange
ITransactionData transaction = null;

var sentryClient = Substitute.For<ISentryClient>();

var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 }, sentryClient);
var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1 });

var server = new TestServer(new WebHostBuilder()
.UseDefaultServiceProvider(di => di.EnableValidation())
Expand Down Expand Up @@ -598,7 +599,7 @@ public async Task Transaction_TransactionNameProviderSetSet_TransactionNameSet()
var expectedName = "My custom name";

var sentryClient = Substitute.For<ISentryClient>();
sentryClient.When(x => x.CaptureTransaction(Arg.Any<Transaction>(), Arg.Any<Hint>()))
sentryClient.When(x => x.CaptureTransaction(Arg.Any<Transaction>(), Arg.Any<Scope>(), Arg.Any<Hint>()))
.Do(callback => transaction = callback.Arg<Transaction>());
var options = new SentryAspNetCoreOptions
{
Expand Down Expand Up @@ -640,7 +641,7 @@ public async Task Transaction_TransactionNameProviderSetUnset_TransactionNameSet
Transaction transaction = null;

var sentryClient = Substitute.For<ISentryClient>();
sentryClient.When(x => x.CaptureTransaction(Arg.Any<Transaction>(), Arg.Any<Hint>()))
sentryClient.When(x => x.CaptureTransaction(Arg.Any<Transaction>(), Arg.Any<Scope>(), Arg.Any<Hint>()))
.Do(callback => transaction = callback.Arg<Transaction>());
var options = new SentryAspNetCoreOptions
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public Fixture()
var client = Substitute.For<ISentryClient>();
var sessionManager = Substitute.For<ISessionManager>();

client.When(x => x.CaptureTransaction(Arg.Any<Transaction>(), Arg.Any<Hint>()))
client.When(x => x.CaptureTransaction(Arg.Any<Transaction>(), Arg.Any<Scope>(), Arg.Any<Hint>()))
.Do(callback => Transaction = callback.Arg<Transaction>());

ScopeManager = new SentryScopeManager(options, client);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void Configure_BindsConfigurationToOptions()
["ServerName"] = expected.ServerName,
["AttachStacktrace"] = expected.AttachStacktrace.ToString(),
["MaxBreadcrumbs"] = expected.MaxBreadcrumbs.ToString(),
["SampleRate"] = expected.SampleRate.ToString(),
["SampleRate"] = expected.SampleRate.Value.ToString(CultureInfo.InvariantCulture),
["Release"] = expected.Release,
["Distribution"] = expected.Distribution,
["Environment"] = expected.Environment,
Expand All @@ -95,7 +95,7 @@ public void Configure_BindsConfigurationToOptions()
["InitCacheFlushTimeout"] = expected.InitCacheFlushTimeout.ToString(),
["DefaultTags"] = expected.DefaultTags.ToString(),
["EnableTracing"] = expected.EnableTracing.ToString(),
["TracesSampleRate"] = expected.TracesSampleRate.ToString(),
["TracesSampleRate"] = expected.TracesSampleRate.Value.ToString(CultureInfo.InvariantCulture),
["TracePropagationTargets:0"] = expected.TracePropagationTargets.First().ToString(),
["TracePropagationTargets:1"] = expected.TracePropagationTargets.Last().ToString(),
["StackTraceMode"] = expected.StackTraceMode.ToString(),
Expand Down
10 changes: 5 additions & 5 deletions test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ namespace Sentry
Sentry.SentryId CaptureEvent(Sentry.SentryEvent evt, Sentry.Scope? scope = null, Sentry.Hint? hint = null);
void CaptureSession(Sentry.SessionUpdate sessionUpdate);
void CaptureTransaction(Sentry.Transaction transaction);
void CaptureTransaction(Sentry.Transaction transaction, Sentry.Hint? hint);
void CaptureTransaction(Sentry.Transaction transaction, Sentry.Scope? scope, Sentry.Hint? hint);
void CaptureUserFeedback(Sentry.UserFeedback userFeedback);
System.Threading.Tasks.Task FlushAsync(System.TimeSpan timeout);
}
Expand Down Expand Up @@ -478,7 +478,7 @@ namespace Sentry
public Sentry.SentryId CaptureEvent(Sentry.SentryEvent? @event, Sentry.Scope? scope = null, Sentry.Hint? hint = null) { }
public void CaptureSession(Sentry.SessionUpdate sessionUpdate) { }
public void CaptureTransaction(Sentry.Transaction transaction) { }
public void CaptureTransaction(Sentry.Transaction transaction, Sentry.Hint? hint) { }
public void CaptureTransaction(Sentry.Transaction transaction, Sentry.Scope? scope, Sentry.Hint? hint) { }
public void CaptureUserFeedback(Sentry.UserFeedback userFeedback) { }
public void Dispose() { }
public System.Threading.Tasks.Task FlushAsync(System.TimeSpan timeout) { }
Expand Down Expand Up @@ -716,7 +716,7 @@ namespace Sentry
public static Sentry.SentryId CaptureMessage(string message, System.Action<Sentry.Scope> configureScope, Sentry.SentryLevel level = 1) { }
public static void CaptureSession(Sentry.SessionUpdate sessionUpdate) { }
public static void CaptureTransaction(Sentry.Transaction transaction) { }
public static void CaptureTransaction(Sentry.Transaction transaction, Sentry.Hint? hint) { }
public static void CaptureTransaction(Sentry.Transaction transaction, Sentry.Scope? scope, Sentry.Hint? hint) { }
public static void CaptureUserFeedback(Sentry.UserFeedback userFeedback) { }
public static void CaptureUserFeedback(Sentry.SentryId eventId, string email, string comments, string? name = null) { }
[System.Obsolete("WARNING: This method deliberately causes a crash, and should not be used in a rea" +
Expand Down Expand Up @@ -1187,7 +1187,7 @@ namespace Sentry.Extensibility
public Sentry.SentryId CaptureEvent(Sentry.SentryEvent evt, Sentry.Scope? scope = null, Sentry.Hint? hint = null) { }
public void CaptureSession(Sentry.SessionUpdate sessionUpdate) { }
public void CaptureTransaction(Sentry.Transaction transaction) { }
public void CaptureTransaction(Sentry.Transaction transaction, Sentry.Hint? hint) { }
public void CaptureTransaction(Sentry.Transaction transaction, Sentry.Scope? scope, Sentry.Hint? hint) { }
public void CaptureUserFeedback(Sentry.UserFeedback userFeedback) { }
public void ConfigureScope(System.Action<Sentry.Scope> configureScope) { }
public System.Threading.Tasks.Task ConfigureScopeAsync(System.Func<Sentry.Scope, System.Threading.Tasks.Task> configureScope) { }
Expand Down Expand Up @@ -1229,7 +1229,7 @@ namespace Sentry.Extensibility
public Sentry.SentryId CaptureException(System.Exception exception) { }
public void CaptureSession(Sentry.SessionUpdate sessionUpdate) { }
public void CaptureTransaction(Sentry.Transaction transaction) { }
public void CaptureTransaction(Sentry.Transaction transaction, Sentry.Hint? hint) { }
public void CaptureTransaction(Sentry.Transaction transaction, Sentry.Scope? scope, Sentry.Hint? hint) { }
public void CaptureUserFeedback(Sentry.UserFeedback sentryUserFeedback) { }
public void ConfigureScope(System.Action<Sentry.Scope> configureScope) { }
public System.Threading.Tasks.Task ConfigureScopeAsync(System.Func<Sentry.Scope, System.Threading.Tasks.Task> configureScope) { }
Expand Down