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: Tracing without performance #2493

Merged
merged 49 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
d486409
wip
bitsandfoxes Jul 18, 2023
3ec17d7
added propagation context
bitsandfoxes Jul 18, 2023
60dabd0
baggage fetching
bitsandfoxes Jul 19, 2023
8c90704
scope creates propagationcontext
bitsandfoxes Jul 19, 2023
3f88d89
http handler retrieves baggage from hub
bitsandfoxes Jul 19, 2023
56f14d9
keeping things internal
bitsandfoxes Jul 19, 2023
6fecb32
properly propagating things with tracing disabled
bitsandfoxes Jul 19, 2023
84971ed
wip
bitsandfoxes Jul 20, 2023
6c7aff1
simplifying
bitsandfoxes Jul 20, 2023
717b37e
caching parsed DSN
bitsandfoxes Jul 24, 2023
16494c6
reviewpoints
bitsandfoxes Jul 25, 2023
0a37731
continuing trace in ASP.NET Core
bitsandfoxes Jul 25, 2023
c6d632c
simplified
bitsandfoxes Jul 25, 2023
9f94934
simplified 2
bitsandfoxes Jul 25, 2023
b421c88
merged main
bitsandfoxes Jul 25, 2023
f4a6b04
cleanup
bitsandfoxes Jul 25, 2023
848af4e
fixed tracing middleware
bitsandfoxes Jul 25, 2023
dc0e022
wip
bitsandfoxes Jul 26, 2023
b973357
tests
bitsandfoxes Jul 28, 2023
522a752
Apply suggestions from code review
bitsandfoxes Jul 31, 2023
cb54916
objects as keys
bitsandfoxes Jul 31, 2023
7b54760
merge
bitsandfoxes Jul 31, 2023
d87e4d5
fixed using
bitsandfoxes Jul 31, 2023
cd02bce
logging the name of the missing key
bitsandfoxes Jul 31, 2023
d27622d
cleanup
bitsandfoxes Jul 31, 2023
6bfd515
comments
bitsandfoxes Jul 31, 2023
ffc633a
merged main
bitsandfoxes Jul 31, 2023
91b3bd1
renamed internal 'IsTracingEnabled' to 'IsPerformanceMonitoringEnabled'
bitsandfoxes Jul 31, 2023
3ed5a8f
test
bitsandfoxes Jul 31, 2023
dfc4274
istracingenabled
bitsandfoxes Jul 31, 2023
b812630
removed unused method
bitsandfoxes Jul 31, 2023
9640278
fixed parsing dsn test
bitsandfoxes Jul 31, 2023
57ff2f5
last one? maybe?
bitsandfoxes Aug 1, 2023
dd1ed93
added release to options in verify
bitsandfoxes Aug 2, 2023
48ba10a
updated verify
bitsandfoxes Aug 2, 2023
75916f0
simplified
bitsandfoxes Aug 2, 2023
fde8865
reverted submodule bump
bitsandfoxes Aug 2, 2023
ee2a0c8
disabled hub
bitsandfoxes Aug 2, 2023
ced245b
Updated CHANGELOG.md
bitsandfoxes Aug 2, 2023
5c3b22e
setting release
bitsandfoxes Aug 2, 2023
cc049ad
missed a release
bitsandfoxes Aug 2, 2023
f072aad
trying to fix the verified
bitsandfoxes Aug 2, 2023
3cb0af4
line numbers
bitsandfoxes Aug 2, 2023
3dd2265
last round of verified?
bitsandfoxes Aug 2, 2023
60d1f1c
reset
bitsandfoxes Aug 2, 2023
b5afd16
dear lord.. please accept these verified update
bitsandfoxes Aug 2, 2023
88ba20c
added missing mono
bitsandfoxes Aug 2, 2023
bde03c6
whitespace?
bitsandfoxes Aug 2, 2023
139579a
Apply suggestions from code review
bitsandfoxes Aug 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/Sentry/BaggageHeader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Sentry;
/// </summary>
/// <seealso href="https://develop.sentry.dev/sdk/performance/dynamic-sampling-context"/>
/// <seealso href="https://www.w3.org/TR/baggage"/>
internal class BaggageHeader
public class BaggageHeader
jamescrosswell marked this conversation as resolved.
Show resolved Hide resolved
{
internal const string HttpHeaderName = "baggage";
internal const string SentryKeyPrefix = "sentry-";
Expand All @@ -19,14 +19,14 @@ internal class BaggageHeader
// "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<KeyValuePair<string, string>> Members { get; }
internal IReadOnlyList<KeyValuePair<string, string>> Members { get; }

private BaggageHeader(IEnumerable<KeyValuePair<string, string>> members) =>
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.
public IReadOnlyDictionary<string, string> GetSentryMembers() =>
internal IReadOnlyDictionary<string, string> GetSentryMembers() =>
Members
.Where(kvp => kvp.Key.StartsWith(SentryKeyPrefix))
.GroupBy(kvp => kvp.Key, kvp => kvp.Value)
Expand Down Expand Up @@ -107,7 +107,7 @@ public override string ToString()
return members.Count == 0 ? null : new BaggageHeader(members);
}

public static BaggageHeader Create(
internal static BaggageHeader Create(
IEnumerable<KeyValuePair<string, string>> items,
bool useSentryPrefix = false)
{
Expand All @@ -121,7 +121,7 @@ public static BaggageHeader Create(
return new BaggageHeader(members);
}

public static BaggageHeader Merge(IEnumerable<BaggageHeader> baggageHeaders) =>
internal static BaggageHeader Merge(IEnumerable<BaggageHeader> baggageHeaders) =>
new(baggageHeaders.SelectMany(x => x.Members));

private static bool IsValidKey(string? key)
Expand Down
15 changes: 15 additions & 0 deletions src/Sentry/Extensibility/DisabledHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,21 @@ public void BindException(Exception exception, ISpan span)
/// </summary>
public SentryTraceHeader? GetTraceHeader() => null;

/// <summary>
/// Returns null.
/// </summary>
public SentryTraceHeader? GetTraceParent() => null;

/// <summary>
/// Returns null.
/// </summary>
public BaggageHeader? GetBaggage() => null;

/// <summary>
/// Returns null.
/// </summary>
public TransactionContext? ContinueTrace(string? sentryTrace, string? baggageHeaders) => null;

/// <summary>
/// No-Op.
/// </summary>
Expand Down
21 changes: 21 additions & 0 deletions src/Sentry/Extensibility/HubAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,27 @@ public void BindException(Exception exception, ISpan span) =>
public SentryTraceHeader? GetTraceHeader()
=> SentrySdk.GetTraceHeader();

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
[DebuggerStepThrough]
public SentryTraceHeader? GetTraceParent()
=> SentrySdk.GetTraceParent();

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
[DebuggerStepThrough]
public BaggageHeader? GetBaggage()
=> SentrySdk.GetBaggage();

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
[DebuggerStepThrough]
public TransactionContext? ContinueTrace(string? sentryTrace, string? baggageHeaders)
=> SentrySdk.ContinueTrace(sentryTrace, baggageHeaders);

/// <summary>
/// Forwards the call to <see cref="SentrySdk"/>.
/// </summary>
Expand Down
18 changes: 18 additions & 0 deletions src/Sentry/IHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,24 @@ ITransaction StartTransaction(
/// </summary>
SentryTraceHeader? GetTraceHeader();

/// <summary>
/// Gets the Sentry trace header of the parent that allows tracing across services
/// </summary>
SentryTraceHeader? GetTraceParent();

/// <summary>
/// Gets the Sentry "baggage" header that allows tracing across services
/// </summary>
BaggageHeader? GetBaggage();

/// <summary>
/// Continues a trace based on HTTP header values.
/// </summary>
/// /// <remarks>
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved
/// If no "sentry-trace" header is provided a random trace ID and span ID is created.
/// </remarks>
TransactionContext? ContinueTrace(string? sentryTrace, string? baggageHeaders);

/// <summary>
/// Starts a new session.
/// </summary>
Expand Down
39 changes: 38 additions & 1 deletion src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,44 @@ public void BindException(Exception exception, ISpan span)

public ISpan? GetSpan() => ScopeManager.GetCurrent().Key.Span;

public SentryTraceHeader? GetTraceHeader() => GetSpan()?.GetTraceHeader();
public SentryTraceHeader? GetTraceHeader() => GetTraceParent();
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved

public SentryTraceHeader? GetTraceParent()
{
var traceHeader = GetSpan()?.GetTraceHeader();
if (traceHeader is not null)
{
return traceHeader;
}
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved

var propagationContext = ScopeManager.GetCurrent().Key.PropagationContext;
if (propagationContext is not null)
{
return new SentryTraceHeader(propagationContext.TraceId, propagationContext.SpanId, null);
}

return null;
Copy link
Member

Choose a reason for hiding this comment

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

I havent' spotted any usage of Logging. There are a few conditions happening here:

  • tracing enabled
  • transation of type X with prop Y having value Z
  • DSC not being null and returning the result of another method call (that could also be null?

Is this easy to track down what's going on in debug mode?

}

public BaggageHeader? GetBaggage()
{
throw new NotImplementedException();
}

public TransactionContext? ContinueTrace(string? sentryTrace, string? baggageHeaders)
{
var propagationContext = SentryPropagationContext.CreateFromHeaders(sentryTrace, baggageHeaders);
ConfigureScope(scope => scope.PropagationContext = propagationContext);
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved

if (_options.IsTracingEnabled)
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved
{
return new TransactionContext(propagationContext.SpanId,propagationContext.ParentSpanId, propagationContext.TraceId, "", "", "", null, null, null);
}
else
{
return null;
}
}

public void StartSession()
{
Expand Down
2 changes: 2 additions & 0 deletions src/Sentry/Scope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ public ITransaction? Transaction
set => _transaction = value;
}

internal SentryPropagationContext? PropagationContext { get; set; }

internal SessionUpdate? SessionUpdate { get; set; }

/// <inheritdoc />
Expand Down
45 changes: 45 additions & 0 deletions src/Sentry/SentryPropagationContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
namespace Sentry;

internal class SentryPropagationContext
{
public SentryId TraceId { get; }
public SpanId SpanId { get; }
public SpanId? ParentSpanId { get; }
public DynamicSamplingContext? DynamicSamplingContext { get; }

private SentryPropagationContext(
SentryTraceHeader traceHeader,
SpanId? parentSpanId,
DynamicSamplingContext? dynamicSamplingContext)
{
TraceId = traceHeader.TraceId;
SpanId = traceHeader.SpanId;
ParentSpanId = parentSpanId;
DynamicSamplingContext = dynamicSamplingContext;
}

private SentryPropagationContext()
{
TraceId = new SentryId();
SpanId = new SpanId();
}

public static SentryPropagationContext CreateFromHeaders(string? sentryTraceHeader, string? baggageHeadersString)
{
if (sentryTraceHeader == null)
{
return new SentryPropagationContext();
}

var traceHeader = SentryTraceHeader.Parse(sentryTraceHeader);

DynamicSamplingContext? dynamicSamplingContext = null;
if (baggageHeadersString is not null)
{
var baggageHeader = BaggageHeader.TryParse(baggageHeadersString);
dynamicSamplingContext = baggageHeader?.CreateDynamicSamplingContext();
}

return new SentryPropagationContext(traceHeader, null, dynamicSamplingContext);
}
}
25 changes: 25 additions & 0 deletions src/Sentry/SentrySdk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -614,9 +614,34 @@ public static void BindException(Exception exception, ISpan span)
/// Gets the Sentry trace header.
/// </summary>
[DebuggerStepThrough]
// [Obsolete("This method will be removed in a future major version. Use the `GetTraceParent` instead.")]
bitsandfoxes marked this conversation as resolved.
Show resolved Hide resolved
public static SentryTraceHeader? GetTraceHeader()
=> CurrentHub.GetTraceHeader();

/// <summary>
/// Gets the Sentry trace header of the parent that allows tracing across services
/// </summary>
[DebuggerStepThrough]
public static SentryTraceHeader? GetTraceParent()
=> CurrentHub.GetTraceParent();

/// <summary>
/// Gets the Sentry "baggage" header that allows tracing across services
/// </summary>
[DebuggerStepThrough]
public static BaggageHeader? GetBaggage()
=> CurrentHub.GetBaggage();

/// <summary>
/// Continues a trace based on HTTP header values.
/// </summary>
/// <remarks>
/// If no "sentry-trace" header is provided a random trace ID and span ID is created.
/// </remarks>
[DebuggerStepThrough]
public static TransactionContext? ContinueTrace(string? sentryTrace, string? baggageHeaders)
=> CurrentHub.ContinueTrace(sentryTrace, baggageHeaders);

/// <inheritdoc cref="IHub.StartSession"/>
[DebuggerStepThrough]
public static void StartSession()
Expand Down
Loading