Skip to content

Commit

Permalink
Reordered parameters to TransactionContext and SpanContext constructo…
Browse files Browse the repository at this point in the history 
…rs (#2696)

* Replaced multiple TransactionContext constructors to a single constructor having default properties
* Made most of the SpanContext constructor parameters optional
  • Loading branch information
@jamescrosswell @vaind
jamescrosswell authored and vaind committed Oct 14, 2023
1 parent e75bcfa commit 4ed996a
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 134 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -28,6 +28,7 @@ API Changes:
- Enable `CaptureFailedRequests` by default ([2688](https://github.com/getsentry/sentry-dotnet/pull/2688))
- Additional constructors removed from `TransactionTracer`. ([#2694](https://github.com/getsentry/sentry-dotnet/pull/2694))
- Removed the `Scope.Platform` property as it was never applied ([#2695](https://github.com/getsentry/sentry-dotnet/pull/2695))
- Reordered parameters for ther TransactionContext and SpanContext constructors. If you're constructing instances of these classes, you will need to adjust the order in which you pass parameters to these. ([#2696](https://github.com/getsentry/sentry-dotnet/pull/2696))

## Unreleased

Expand Down
12 changes: 4 additions & 8 deletions src/Sentry.OpenTelemetry/SentrySpanProcessor.cs
View file
Expand Up @@ -66,10 +66,10 @@ public override void OnStart(Activity data)
{
// We can find the parent span - start a child span.
var context = new SpanContext(
data.OperationName,
data.SpanId.AsSentrySpanId(),
data.ParentSpanId.AsSentrySpanId(),
data.TraceId.AsSentryId(),
data.OperationName,
data.DisplayName,
null,
null)
Expand All @@ -87,16 +87,12 @@ public override void OnStart(Activity data)
bool? isSampled = data.HasRemoteParent ? data.Recorded : null;

// No parent span found - start a new transaction
var transactionContext = new TransactionContext(
var transactionContext = new TransactionContext(data.DisplayName,
data.OperationName,
data.SpanId.AsSentrySpanId(),
data.ParentSpanId.AsSentrySpanId(),
data.TraceId.AsSentryId(),
data.DisplayName,
data.OperationName,
data.DisplayName,
null,
isSampled,
isSampled)
data.DisplayName, null, isSampled, isSampled)
{
Instrumenter = Instrumenter.OpenTelemetry
};
Expand Down
4 changes: 2 additions & 2 deletions src/Sentry/Extensibility/DisabledHub.cs
View file
Expand Up @@ -89,7 +89,7 @@ public void BindException(Exception exception, ISpan span)
string? operation = null)
{
// Transactions from DisabledHub are always sampled out
return new TransactionContext( name ?? string.Empty, operation ?? string.Empty, false);
return new TransactionContext( name ?? string.Empty, operation ?? string.Empty, isSampled: false);
}

/// <summary>
Expand All @@ -102,7 +102,7 @@ public void BindException(Exception exception, ISpan span)
string? operation = null)
{
// Transactions from DisabledHub are always sampled out
return new TransactionContext( name ?? string.Empty, operation ?? string.Empty, false);
return new TransactionContext( name ?? string.Empty, operation ?? string.Empty, isSampled: false);
}

/// <summary>
Expand Down
16 changes: 8 additions & 8 deletions src/Sentry/SpanContext.cs
View file
Expand Up @@ -37,17 +37,17 @@ public class SpanContext : ITraceContext
/// Initializes an instance of <see cref="SpanContext"/>.
/// </summary>
public SpanContext(
SpanId spanId,
SpanId? parentSpanId,
SentryId traceId,
string operation,
string? description,
SpanStatus? status,
bool? isSampled)
SpanId? spanId = null,
SpanId? parentSpanId = null,
SentryId? traceId = null,
string? description = null,
SpanStatus? status = null,
bool? isSampled = null)
{
SpanId = spanId;
SpanId = spanId ?? SpanId.Create();
ParentSpanId = parentSpanId;
TraceId = traceId;
TraceId = traceId ?? SentryId.Create();
Operation = operation;
Description = description;
Status = status;
Expand Down
95 changes: 12 additions & 83 deletions src/Sentry/TransactionContext.cs
View file
Expand Up @@ -20,17 +20,18 @@ public class TransactionContext : SpanContext, ITransactionContext
/// Initializes an instance of <see cref="TransactionContext"/>.
/// </summary>
public TransactionContext(
SpanId spanId,
SpanId? parentSpanId,
SentryId traceId,
string name,
string operation,
string? description,
SpanStatus? status,
bool? isSampled,
bool? isParentSampled,
TransactionNameSource nameSource)
: base(spanId, parentSpanId, traceId, operation, description, status, isSampled)
SpanId? spanId = null,
SpanId? parentSpanId = null,
SentryId? traceId = null,
string? description = "",
SpanStatus? status = null,
bool? isSampled = null,
bool? isParentSampled = null,
TransactionNameSource nameSource = TransactionNameSource.Custom
)
: base(operation, spanId, parentSpanId, traceId, description, status, isSampled)
{
Name = name;
IsParentSampled = isParentSampled;
Expand All @@ -40,83 +41,11 @@ public class TransactionContext : SpanContext, ITransactionContext
/// <summary>
/// Initializes an instance of <see cref="TransactionContext"/>.
/// </summary>
public TransactionContext(
SpanId spanId,
SpanId? parentSpanId,
SentryId traceId,
string name,
string operation,
string? description,
SpanStatus? status,
bool? isSampled,
bool? isParentSampled)
: base(spanId, parentSpanId, traceId, operation, description, status, isSampled)
{
Name = name;
IsParentSampled = isParentSampled;
NameSource = TransactionNameSource.Custom;
}

/// <summary>
/// Initializes an instance of <see cref="TransactionContext"/>.
/// </summary>
public TransactionContext(
SpanId? parentSpanId,
SentryId traceId,
string name,
string operation,
bool? isParentSampled)
: this(SpanId.Create(), parentSpanId, traceId, name, operation, "", null, isParentSampled, isParentSampled)
{
}

/// <summary>
/// Initializes an instance of <see cref="TransactionContext"/>.
/// </summary>
public TransactionContext(
string name,
string operation,
bool? isSampled)
: this(SpanId.Create(), null, SentryId.Create(), name, operation, "", null, isSampled, null)
{
}

/// <summary>
/// Initializes an instance of <see cref="TransactionContext"/>.
/// </summary>
public TransactionContext(
internal TransactionContext(
string name,
string operation,
SentryTraceHeader traceHeader)
: this(SpanId.Create(), traceHeader.SpanId, traceHeader.TraceId, name, operation, "", null, traceHeader.IsSampled, traceHeader.IsSampled)
{
}

/// <summary>
/// Initializes an instance of <see cref="TransactionContext"/>.
/// </summary>
public TransactionContext(
string name,
string operation,
SentryTraceHeader traceHeader,
TransactionNameSource nameSource)
: this(SpanId.Create(), traceHeader.SpanId, traceHeader.TraceId, name, operation, "", null, traceHeader.IsSampled, traceHeader.IsSampled, nameSource)
{
}

/// <summary>
/// Initializes an instance of <see cref="TransactionContext"/>.
/// </summary>
public TransactionContext(string name, string operation)
: this(SpanId.Create(), null, SentryId.Create(), name, operation, "", null, null, null)
{
}

/// <summary>
/// Initializes an instance of <see cref="TransactionContext"/>.
/// </summary>
public TransactionContext(string name, string operation, TransactionNameSource nameSource)
: this(SpanId.Create(), null, SentryId.Create(), name, operation, "", null, null, null, nameSource)
: this(name, operation, SpanId.Create(), parentSpanId: traceHeader.SpanId, traceId: traceHeader.TraceId, "", null, isSampled: traceHeader.IsSampled, isParentSampled: traceHeader.IsSampled)
{
}
}
47 changes: 14 additions & 33 deletions test/Sentry.Tests/Protocol/TransactionTests.cs
View file
Expand Up @@ -38,18 +38,13 @@ public async Task NewTransactionTracer_IdleTimeoutProvided_AutomaticallyFinishes
Debug = true
};
var hub = new Hub(options, client);
var context = new TransactionContext(
var context = new TransactionContext("my name",
"my operation",
SpanId.Create(),
SpanId.Create(),
SentryId.Create(),
"my name",
"my operation",
"description",
SpanStatus.Ok,
null,
true,
TransactionNameSource.Component
);
SpanStatus.Ok, null, true, TransactionNameSource.Component);

var transaction = new TransactionTracer(hub, context, TimeSpan.FromMilliseconds(2));

Expand All @@ -75,18 +70,13 @@ public void Redact_Redacts_Urls()
var breadcrumbMessage = "message https://user@sentry.io"; // should be redacted
var breadcrumbDataValue = "data-value https://user@sentry.io"; // should be redacted
var tagValue = "tag_value https://user@not.redacted";
var context = new TransactionContext(
var context = new TransactionContext(name,
operation,
SpanId.Create(),
SpanId.Create(),
SentryId.Create(),
name,
operation,
description,
SpanStatus.AlreadyExists,
null,
true,
TransactionNameSource.Component
);
SpanStatus.AlreadyExists, null, true, TransactionNameSource.Component);

var txTracer = new TransactionTracer(DisabledHub.Instance, context)
{
Expand Down Expand Up @@ -172,18 +162,14 @@ public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject()
{
// Arrange
var timestamp = DateTimeOffset.MaxValue;
var context = new TransactionContext(
var context = new TransactionContext("name123",
"op123",
SpanId.Create(),
SpanId.Create(),
SentryId.Create(),
"name123",
"op123",
"desc",
SpanStatus.AlreadyExists,
null, // sampling isn't serialized and getting FluentAssertions
SentryId.Create(), // sampling isn't serialized and getting FluentAssertions
// to ignore that on Spans and contexts isn't really straight forward
true,
TransactionNameSource.Component);
"desc",
SpanStatus.AlreadyExists, null, true, TransactionNameSource.Component);

var transaction = new TransactionTracer(DisabledHub.Instance, context)
{
Expand Down Expand Up @@ -449,18 +435,13 @@ public async Task Finish_SentryRequestTransactionGetsIgnored()
Dsn = ValidDsn,
};
var hub = new Hub(options, client);
var context = new TransactionContext(
var context = new TransactionContext("my name",
"my operation",
SpanId.Create(),
SpanId.Create(),
SentryId.Create(),
"my name",
"my operation",
"description",
SpanStatus.Ok,
null,
true,
TransactionNameSource.Component
);
SpanStatus.Ok, null, true, TransactionNameSource.Component);

var transaction = new TransactionTracer(hub, context, TimeSpan.FromMilliseconds(2))
{
Expand Down

0 comments on commit 4ed996a

Please sign in to comment.