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

Trace sampling #753

Merged
merged 21 commits into from
Jan 22, 2021
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## vNext

- Add support for dynamic transaction sampling. (#753) @Tyrrrz

## 3.0.0-beta.0

- Add instruction_addr to SentryStackFrame. (#744) @lucas-zimerman
Expand Down Expand Up @@ -717,4 +721,4 @@ Also available via NuGet:

[Sentry](https://www.nuget.org/packages/Sentry/0.0.1-preview1)
[Sentry.AspNetCore](https://www.nuget.org/packages/Sentry.AspNetCore/0.0.1-preview1)
[Sentry.Extensions.Logging](https://www.nuget.org/packages/Sentry.Extensions.Logging/0.0.1-preview1)
[Sentry.Extensions.Logging](https://www.nuget.org/packages/Sentry.Extensions.Logging/0.0.1-preview1)
2 changes: 1 addition & 1 deletion modules/Ben.Demystifier
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Sentry;
using Sentry.Extensibility;
using Sentry.Protocol;

namespace Samples.AspNetCore.Mvc
{
Expand Down
1 change: 1 addition & 0 deletions src/Sentry.AspNetCore/ScopeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.Net.Http.Headers;
using Sentry.AspNetCore.Extensions;
using Sentry.Extensibility;
using Sentry.Protocol;

namespace Sentry.AspNetCore
{
Expand Down
10 changes: 6 additions & 4 deletions src/Sentry/Extensibility/DisabledHub.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Sentry.Protocol;

Expand Down Expand Up @@ -55,10 +56,11 @@ public void WithScope(Action<Scope> scopeCallback)
/// <summary>
/// Returns a dummy transaction.
/// </summary>
public ITransaction StartTransaction(string name, string operation) => new Transaction(this, name, operation)
{
IsSampled = false
};
public ITransaction StartTransaction(
ITransactionContext context,
IReadOnlyDictionary<string, object?> customSamplingContext) =>
Tyrrrz marked this conversation as resolved.
Show resolved Hide resolved
// Transactions from DisabledHub are always sampled out
new Transaction(this, context) {IsSampled = false};

/// <summary>
/// Returns null.
Expand Down
6 changes: 4 additions & 2 deletions src/Sentry/Extensibility/HubAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@ public void WithScope(Action<Scope> scopeCallback)
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
[DebuggerStepThrough]
public ITransaction StartTransaction(string name, string operation)
=> SentrySdk.StartTransaction(name, operation);
public ITransaction StartTransaction(
ITransactionContext context,
IReadOnlyDictionary<string, object?> customSamplingContext)
=> SentrySdk.StartTransaction(context, customSamplingContext);

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
Expand Down
15 changes: 15 additions & 0 deletions src/Sentry/HubExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,21 @@ namespace Sentry
[EditorBrowsable(EditorBrowsableState.Never)]
public static class HubExtensions
{
/// <summary>
/// Starts a transaction.
/// </summary>
public static ITransaction StartTransaction(this IHub hub, ITransactionContext context) =>
hub.StartTransaction(context, new Dictionary<string, object?>());

/// <summary>
/// Starts a transaction.
/// </summary>
public static ITransaction StartTransaction(
this IHub hub,
string name,
string operation) =>
hub.StartTransaction(new TransactionContext(name, operation));

/// <summary>
/// Starts a transaction.
/// </summary>
Expand Down
6 changes: 5 additions & 1 deletion src/Sentry/IHub.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Collections.Generic;
using Sentry.Protocol;

namespace Sentry
Expand All @@ -24,7 +25,10 @@ public interface IHub :
/// <summary>
/// Starts a transaction.
/// </summary>
ITransaction StartTransaction(string name, string operation);
ITransaction StartTransaction(
ITransactionContext context,
IReadOnlyDictionary<string, object?> customSamplingContext
);

/// <summary>
/// Gets the sentry trace header.
Expand Down
13 changes: 0 additions & 13 deletions src/Sentry/ISentryTraceSampler.cs

This file was deleted.

36 changes: 34 additions & 2 deletions src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading.Tasks;
using Sentry.Extensibility;
using Sentry.Integrations;
Expand Down Expand Up @@ -103,9 +105,11 @@ public void WithScope(Action<Scope> scopeCallback)

public void BindClient(ISentryClient client) => ScopeManager.BindClient(client);

public ITransaction StartTransaction(string name, string operation)
public ITransaction StartTransaction(
ITransactionContext context,
IReadOnlyDictionary<string, object?> customSamplingContext)
{
var transaction = new Transaction(this, name, operation);
var transaction = new Transaction(this, context);

var nameAndVersion = MainSentryEventProcessor.NameAndVersion;
var protocolPackageName = MainSentryEventProcessor.ProtocolPackageName;
Expand All @@ -121,6 +125,34 @@ public ITransaction StartTransaction(string name, string operation)
transaction.Sdk.AddPackage(protocolPackageName, nameAndVersion.Version);
}

// Make a sampling decision
var samplingContext = new TransactionSamplingContext(
context,
customSamplingContext
);

var sampleRate =
// Custom sampler may not exist or may return null, in which case we fallback
// to the static sample rate.
_options.TracesSampler?.Invoke(samplingContext)
Comment on lines +135 to +137
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is different from Java. I decided to allow the dynamic sampler to return null to fallback to the configured sample rate. I think it would be very useful, in case when you only want special sampling rules for a subset of transactions and default rules for everything else.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to me and I don't have a problem with that.
Maybe @marandaneto has opinions though.

Please make a note on the spec then that this is an alternative approach.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that sounds a good use case, lets talk about this today in the meeting

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll make this on Java too

?? _options.TracesSampleRate;

transaction.IsSampled = sampleRate switch
{
// Sample rate >= 1 means always sampled *in*
>= 1 => true,
// Sample rate <= 0 means always sampled *out*
<= 0 => false,
// Otherwise roll the dice
_ => SynchronizedRandom.NextDouble() > sampleRate
};

// 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.

// Sampling decision must have been made at this point
Debug.Assert(transaction.IsSampled != null, "Started transaction without a sampling decision.");

return transaction;
}

Expand Down
17 changes: 17 additions & 0 deletions src/Sentry/Internal/SynchronizedRandom.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System;

namespace Sentry.Internal
{
internal static class SynchronizedRandom
{
private static readonly Random Random = new();

public static double NextDouble()
{
lock (Random)
{
return Random.NextDouble();
}
}
}
}
42 changes: 42 additions & 0 deletions src/Sentry/Protocol/Context/ITraceContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
namespace Sentry.Protocol.Context
{
/// <summary>
/// Trace metadata stored in 'contexts.trace' on a n event or transaction.
/// </summary>
public interface ITraceContext
{
/// <summary>
/// Span ID.
/// </summary>
SpanId SpanId { get; }

/// <summary>
/// Parent ID.
/// </summary>
SpanId? ParentSpanId { get; }

/// <summary>
/// Trace ID.
/// </summary>
SentryId TraceId { get; }

/// <summary>
/// Operation.
/// </summary>
string Operation { get; }

/// <summary>
/// Status.
/// </summary>
SpanStatus? Status { get; }

// Note: this may need to be mutated internally,
// but the user should never be able to change it
// on their own.

/// <summary>
/// Whether the span or transaction is sampled in (i.e. eligible for sending to Sentry).
/// </summary>
bool? IsSampled { get; }
}
}
12 changes: 8 additions & 4 deletions src/Sentry/Protocol/Context/Trace.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
using System.Text.Json;
using Sentry.Internal.Extensions;
using Sentry.Protocol.Context;

namespace Sentry.Protocol
{
/// <summary>
/// Trace context data.
/// </summary>
public class Trace : ISpanContext, IJsonSerializable
public class Trace : ITraceContext, IJsonSerializable
{
/// <summary>
/// Tells Sentry which type of context this is.
Expand All @@ -29,7 +30,7 @@ public class Trace : ISpanContext, IJsonSerializable
public SpanStatus? Status { get; set; }

/// <inheritdoc />
public bool IsSampled { get; internal set; } = true;
public bool? IsSampled { get; internal set; }

/// <summary>
/// Clones this instance.
Expand Down Expand Up @@ -76,7 +77,10 @@ public void WriteTo(Utf8JsonWriter writer)
writer.WriteString("status", status.ToString().ToSnakeCase());
}

writer.WriteBoolean("sampled", IsSampled);
if (IsSampled is {} isSampled)
{
writer.WriteBoolean("sampled", isSampled);
}

writer.WriteEndObject();
}
Expand All @@ -91,7 +95,7 @@ public static Trace FromJson(JsonElement json)
var traceId = json.GetPropertyOrNull("trace_id")?.Pipe(SentryId.FromJson) ?? SentryId.Empty;
var operation = json.GetPropertyOrNull("op")?.GetString() ?? "";
var status = json.GetPropertyOrNull("status")?.GetString()?.Pipe(s => s.Replace("_", "").ParseEnum<SpanStatus>());
var isSampled = json.GetPropertyOrNull("sampled")?.GetBoolean() ?? true;
var isSampled = json.GetPropertyOrNull("sampled")?.GetBoolean();

return new Trace
{
Expand Down
24 changes: 18 additions & 6 deletions src/Sentry/Protocol/IEventLike.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
using System.Collections.Generic;
using System.Linq;

namespace Sentry.Protocol
{
/// <summary>
/// Models members common between types that represent event-like data.
/// </summary>
public interface IEventLike : IHasTags, IHasExtra
public interface IEventLike : IHasBreadcrumbs, IHasTags, IHasExtra
{
/// <summary>
/// Sentry level.
Expand Down Expand Up @@ -70,16 +71,27 @@ public interface IEventLike : IHasTags, IHasExtra
/// <example> { "fingerprint": ["myrpc", "POST", "/foo.bar"] } </example>
/// <example> { "fingerprint": ["{{ default }}", "http://example.com/my.url"] } </example>
IReadOnlyList<string> Fingerprint { get; set; }
}

/// <summary>
/// Extensions for <see cref="IEventLike"/>.
/// </summary>
public static class EventLikeExtensions
{
/// <summary>
/// A trail of events which happened prior to an issue.
/// Whether a <see cref="Protocol.User"/> has been set to the object with any of its fields non null.
/// </summary>
/// <seealso href="https://docs.sentry.io/platforms/dotnet/enriching-events/breadcrumbs/"/>
IReadOnlyCollection<Breadcrumb> Breadcrumbs { get; }
public static bool HasUser(this IEventLike eventLike)
=> eventLike.User.Email is not null
|| eventLike.User.Id is not null
|| eventLike.User.Username is not null
|| eventLike.User.InternalOther?.Count > 0
|| eventLike.User.IpAddress is not null;

/// <summary>
/// Adds a breadcrumb.
/// Sets the fingerprint to the object.
/// </summary>
void AddBreadcrumb(Breadcrumb breadcrumb);
public static void SetFingerprint(this IEventLike eventLike, IEnumerable<string> fingerprint)
=> eventLike.Fingerprint = fingerprint as IReadOnlyList<string> ?? fingerprint.ToArray();
}
}
Loading