Skip to content

Commit 6f8312d

Browse files
Added some new tests... found some new issues
1 parent 907850b commit 6f8312d

10 files changed

Lines changed: 298 additions & 30 deletions

File tree

samples/Sentry.Samples.OpenTelemetry.AspNetCore/Program.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
.AddAspNetCoreInstrumentation()
3030
.AddHttpClientInstrumentation()
3131
// Finally, we configure OpenTelemetry over OTLP to send traces to Sentry
32-
.AddSentryOTLP(dsn)
32+
.AddSentryOtlp(dsn)
3333
);
3434

3535
builder.WebHost.UseSentry(options =>
@@ -43,7 +43,7 @@
4343
options.Debug = builder.Environment.IsDevelopment();
4444
options.SendDefaultPii = true;
4545
options.TracesSampleRate = 1.0;
46-
options.UseOTLP(); // <-- Configure Sentry to use OpenTelemetry trace information
46+
options.UseOtlp(); // <-- Configure Sentry to use OpenTelemetry trace information
4747
});
4848

4949
builder.Services

src/Sentry.OpenTelemetry/OtelPropagationContext.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ internal class OtelPropagationContext : IPropagationContext
1111
public SpanId SpanId => Activity.Current?.SpanId.AsSentrySpanId() ?? default;
1212
public SpanId? ParentSpanId => Activity.Current?.ParentSpanId.AsSentrySpanId();
1313

14+
/// <summary>
15+
/// Warning: this method may throw an exception if Activity.Current is null.
16+
/// This method should not be used when instrumenting with OTEL.
17+
/// </summary>
1418
public DynamicSamplingContext GetOrCreateDynamicSamplingContext(SentryOptions options, IReplaySession replaySession)
1519
{
1620
if (DynamicSamplingContext is null)

src/Sentry.OpenTelemetry/SentryOptionsExtensions.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public static class SentryOptionsExtensions
3030
/// </param>
3131
/// <remarks>
3232
/// This method of initialising the Sentry OpenTelemetry integration will be depricated in a future major release.
33-
/// We recommend you use <see cref="UseOTLP(SentryOptions, TracerProviderBuilder, TextMapPropagator?)"/> instead.
33+
/// We recommend you use <see cref="UseOtlp(Sentry.SentryOptions,TracerProviderBuilder,TextMapPropagator?)"/> instead.
3434
/// </remarks>
3535
public static void UseOpenTelemetry(this SentryOptions options, TracerProviderBuilder builder,
3636
TextMapPropagator? textMapPropagator = null, bool disableSentryTracing = false)
@@ -55,7 +55,7 @@ public static void UseOpenTelemetry(this SentryOptions options, TracerProviderBu
5555
/// </param>
5656
/// <remarks>
5757
/// This method of initialising the Sentry OpenTelemetry integration will be depricated in a future major release.
58-
/// We recommend you use <see cref="UseOTLP(SentryOptions, TracerProviderBuilder, TextMapPropagator?)"/> instead.
58+
/// We recommend you use <see cref="UseOtlp(Sentry.SentryOptions,TracerProviderBuilder,TextMapPropagator?)"/> instead.
5959
/// </remarks>
6060
public static void UseOpenTelemetry(this SentryOptions options, bool disableSentryTracing = false)
6161
{
@@ -88,14 +88,14 @@ public static void UseOpenTelemetry(this SentryOptions options, bool disableSent
8888
/// could wrap this in a <see cref="CompositeTextMapPropagator"/> if you needed other propagators as well.
8989
/// </para>
9090
/// </param>
91-
public static void UseOTLP(this SentryOptions options, TracerProviderBuilder builder, TextMapPropagator? textMapPropagator = null)
91+
public static void UseOtlp(this SentryOptions options, TracerProviderBuilder builder, TextMapPropagator? textMapPropagator = null)
9292
{
9393
if (string.IsNullOrWhiteSpace(options.Dsn))
9494
{
9595
throw new ArgumentException("Sentry DSN must be set before calling `SentryOptions.UseOTLP`", nameof(options.Dsn));
9696
}
97-
builder.AddSentryOTLP(options.Dsn, textMapPropagator);
98-
options.UseOTLP();
97+
builder.AddSentryOtlp(options.Dsn, textMapPropagator);
98+
options.UseOtlp();
9999
}
100100

101101
/// <summary>
@@ -108,11 +108,11 @@ public static void UseOTLP(this SentryOptions options, TracerProviderBuilder bui
108108
/// </summary>
109109
/// <remarks>
110110
/// Note: if you are using this overload to configure Sentry to work with OpenTelemetry, you will also have to call
111-
/// <see cref="TracerProviderBuilderExtensions.AddSentryOTLP"/>, when building your <see cref="TracerProviderBuilder"/>
111+
/// <see cref="TracerProviderBuilderExtensions.AddSentryOtlp"/>, when building your <see cref="TracerProviderBuilder"/>
112112
/// to ensure OpenTelemetry sends trace information to Sentry.
113113
/// </remarks>
114114
/// <param name="options">The <see cref="SentryOptions"/> instance.</param>
115-
public static void UseOTLP(this SentryOptions options)
115+
public static void UseOtlp(this SentryOptions options)
116116
{
117117
options.Instrumenter = Instrumenter.OpenTelemetry;
118118
options.DisableSentryTracing = true;

src/Sentry.OpenTelemetry/TracerProviderBuilderExtensions.cs

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@ namespace Sentry.OpenTelemetry;
1212
/// </summary>
1313
public static class TracerProviderBuilderExtensions
1414
{
15+
internal const string MissingDsnWarning = "Invalid DSN passed to AddSentryOTLP";
16+
1517
/// <summary>
1618
/// <para>
1719
/// Ensures OpenTelemetry trace information is sent to Sentry. OpenTelemetry spans will be converted to Sentry spans
1820
/// using a span processor. This is no longer recommended. SDK users should consider using
19-
/// <see cref="AddSentryOTLP(TracerProviderBuilder, string, TextMapPropagator?)"/> instead, which is the recommended
21+
/// <see cref="AddSentryOtlp"/> instead, which is the recommended
2022
/// way to send OpenTelemetry trace information to Sentry moving forward.
2123
/// </para>
2224
/// <para>
@@ -74,7 +76,7 @@ internal static BaseProcessor<Activity> ImplementationFactory(IServiceProvider s
7476
/// </para>
7577
/// <para>
7678
/// Note that if you use this method to configure the trace builder, you will also need to call
77-
/// <see cref="SentryOptionsExtensions.UseOTLP(SentryOptions)"/> when initialising Sentry, for Sentry to work
79+
/// <see cref="SentryOptionsExtensions.UseOtlp(Sentry.SentryOptions)"/> when initialising Sentry, for Sentry to work
7880
/// properly with OpenTelemetry.
7981
/// </para>
8082
/// </summary>
@@ -92,33 +94,31 @@ internal static BaseProcessor<Activity> ImplementationFactory(IServiceProvider s
9294
/// </para>
9395
/// </param>
9496
/// <returns>The supplied <see cref="TracerProviderBuilder"/> for chaining.</returns>
95-
public static TracerProviderBuilder AddSentryOTLP(this TracerProviderBuilder tracerProviderBuilder, string dsnString,
97+
public static TracerProviderBuilder AddSentryOtlp(this TracerProviderBuilder tracerProviderBuilder, string dsnString,
9698
TextMapPropagator? defaultTextMapPropagator = null)
9799
{
98-
if (string.IsNullOrWhiteSpace(dsnString))
100+
if (Dsn.TryParse(dsnString) is not { } dsn)
99101
{
100-
throw new ArgumentException("Sentry DSN must be provided for OLTP instrumentation", nameof(dsnString));
102+
throw new ArgumentException("Invalid DSN passed to AddSentryOTLP", nameof(dsnString));
101103
}
102104

103105
defaultTextMapPropagator ??= new SentryPropagator();
104106
Sdk.SetDefaultTextMapPropagator(defaultTextMapPropagator);
105107

106-
if (Dsn.TryParse(dsnString) is not { } dsn)
107-
{
108-
return tracerProviderBuilder;
109-
}
108+
tracerProviderBuilder.AddOtlpExporter(options => OtlpConfigurationCallback(options, dsn));
109+
return tracerProviderBuilder;
110+
}
110111

111-
tracerProviderBuilder.AddOtlpExporter(options =>
112+
// Internal helper method for testing purposes
113+
internal static void OtlpConfigurationCallback(OtlpExporterOptions options, Dsn dsn)
114+
{
115+
options.Endpoint = dsn.GetOtlpTracesEndpointUri();
116+
options.Protocol = OtlpExportProtocol.HttpProtobuf;
117+
options.HttpClientFactory = () =>
112118
{
113-
options.Endpoint = dsn.GetOtlpTracesEndpointUri();
114-
options.Protocol = OtlpExportProtocol.HttpProtobuf;
115-
options.HttpClientFactory = () =>
116-
{
117-
var client = new HttpClient();
118-
client.DefaultRequestHeaders.Add("X-Sentry-Auth", $"sentry sentry_key={dsn.PublicKey}");
119-
return client;
120-
};
121-
});
122-
return tracerProviderBuilder;
119+
var client = new HttpClient();
120+
client.DefaultRequestHeaders.Add("X-Sentry-Auth", $"sentry sentry_key={dsn.PublicKey}");
121+
return client;
122+
};
123123
}
124124
}

src/Sentry/DynamicSamplingContext.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,12 @@ public static DynamicSamplingContext CreateFromUnsampledTransaction(UnsampledTra
227227
replaySession);
228228
}
229229

230+
/// <summary>
231+
/// Creates a <see cref="DynamicSamplingContext"/> from the given <see cref="IPropagationContext"/>.
232+
/// </summary>
233+
/// <exception cref="ArgumentOutOfRangeException">Can be thrown when using OpenTelemetry instrumentation and
234+
/// System.Diagnostics.Activity.Current is null. This method should not be used when instrumenting with OTEL.
235+
/// </exception>
230236
public static DynamicSamplingContext CreateFromPropagationContext(IPropagationContext propagationContext, SentryOptions options, IReplaySession? replaySession)
231237
{
232238
var traceId = propagationContext.TraceId;
@@ -256,6 +262,12 @@ public static DynamicSamplingContext CreateDynamicSamplingContext(this Transacti
256262
public static DynamicSamplingContext CreateDynamicSamplingContext(this UnsampledTransaction transaction, SentryOptions options, IReplaySession? replaySession)
257263
=> DynamicSamplingContext.CreateFromUnsampledTransaction(transaction, options, replaySession);
258264

265+
/// <summary>
266+
/// Creates a <see cref="DynamicSamplingContext"/> from the given <see cref="IPropagationContext"/>.
267+
/// </summary>
268+
/// <exception cref="ArgumentOutOfRangeException">Can be thrown when using OpenTelemetry instrumentation and
269+
/// System.Diagnostics.Activity.Current is null. This method should not be used when instrumenting with OTEL.
270+
/// </exception>
259271
public static DynamicSamplingContext CreateDynamicSamplingContext(this IPropagationContext propagationContext, SentryOptions options, IReplaySession? replaySession)
260272
=> DynamicSamplingContext.CreateFromPropagationContext(propagationContext, options, replaySession);
261273
}

src/Sentry/IHub.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ public ITransactionTracer StartTransaction(
7070
/// <summary>
7171
/// Gets the Sentry baggage header that allows tracing across services
7272
/// </summary>
73+
/// <exception cref="ArgumentOutOfRangeException">Can be thrown when using OpenTelemetry instrumentation and
74+
/// System.Diagnostics.Activity.Current is null. This method should not be used when instrumenting with OTEL.
75+
/// </exception>
7376
public BaggageHeader? GetBaggage();
7477

7578
/// <summary>

src/Sentry/Internal/Hub.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,11 @@ public SentryTraceHeader GetTraceHeader()
312312

313313
public BaggageHeader GetBaggage()
314314
{
315+
if (_options.Instrumenter is Instrumenter.OpenTelemetry)
316+
{
317+
_options.LogWarning("GetBaggage should not be called when using OpenTelemetry - it may throw an exception");
318+
}
319+
315320
var span = GetSpan();
316321
if (span?.GetTransaction().GetDynamicSamplingContext() is { IsEmpty: false } dsc)
317322
{
@@ -489,7 +494,10 @@ private void ApplyTraceContextToEvent(SentryEvent evt, IPropagationContext propa
489494
evt.Contexts.Trace.TraceId = propagationContext.TraceId;
490495
evt.Contexts.Trace.SpanId = propagationContext.SpanId;
491496
evt.Contexts.Trace.ParentSpanId = propagationContext.ParentSpanId;
492-
evt.DynamicSamplingContext = propagationContext.GetOrCreateDynamicSamplingContext(_options, _replaySession);
497+
if (_options.Instrumenter is Instrumenter.Sentry)
498+
{
499+
evt.DynamicSamplingContext = propagationContext.GetOrCreateDynamicSamplingContext(_options, _replaySession);
500+
}
493501
}
494502

495503
public bool CaptureEnvelope(Envelope envelope) => CurrentClient.CaptureEnvelope(envelope);

src/Sentry/SentryMessageHandler.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,15 @@ private void PropagateTraceHeaders(HttpRequestMessage request, string url, ISpan
133133
}
134134
}
135135

136+
// We only propogate trace headers for Sentry's native intstumentation. It isn't possible to propogate
137+
// headers when OTEL instrumentation is used since the traceId can be SentryId.Empty if there is no active
138+
// OTEL span... which would result in an exception being thrown when trying to create the
139+
// DynamicSamplingContext.
140+
if (_options?.Instrumenter is Instrumenter.Sentry)
141+
{
142+
return;
143+
}
144+
136145
if (_options?.TracePropagationTargets.MatchesSubstringOrRegex(url) is true or null)
137146
{
138147
AddSentryTraceHeader(request, parentSpan);
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
namespace Sentry.OpenTelemetry.Tests;
2+
3+
public class OtelPropagationContextTests
4+
{
5+
private class Fixture
6+
{
7+
public SentryId ActiveReplayId { get; } = SentryId.Create();
8+
public IReplaySession ActiveReplaySession { get; }
9+
public SentryOptions SentryOptions { get; }
10+
11+
public Fixture()
12+
{
13+
ActiveReplaySession = Substitute.For<IReplaySession>();
14+
ActiveReplaySession.ActiveReplayId.Returns(ActiveReplayId);
15+
16+
SentryOptions = new SentryOptions { Dsn = "https://examplePublicKey@o0.ingest.sentry.io/123456" };
17+
}
18+
}
19+
20+
private readonly Fixture _fixture = new();
21+
22+
[Fact]
23+
public void TraceId_NoActivityCurrent_ReturnsDefault()
24+
{
25+
// Arrange
26+
var sut = new OtelPropagationContext();
27+
Activity.Current = null;
28+
29+
// Act
30+
var traceId = sut.TraceId;
31+
32+
// Assert
33+
traceId.Should().Be(default(SentryId));
34+
}
35+
36+
[Fact]
37+
public void TraceId_WithActivityCurrent_ReturnsSentryIdFromActivityTraceId()
38+
{
39+
// Arrange
40+
using var activity = new Activity("test").Start();
41+
var sut = new OtelPropagationContext();
42+
43+
// Act
44+
var traceId = sut.TraceId;
45+
46+
// Assert
47+
traceId.Should().NotBe(default(SentryId));
48+
traceId.Should().Be(activity.TraceId.AsSentryId());
49+
}
50+
51+
[Fact]
52+
public void SpanId_NoActivityCurrent_ReturnsDefault()
53+
{
54+
// Arrange
55+
var sut = new OtelPropagationContext();
56+
Activity.Current = null;
57+
58+
// Act
59+
var spanId = sut.SpanId;
60+
61+
// Assert
62+
spanId.Should().Be(default(SpanId));
63+
}
64+
65+
[Fact]
66+
public void SpanId_WithActivityCurrent_ReturnsSpanIdFromActivitySpanId()
67+
{
68+
// Arrange
69+
using var activity = new Activity("test").Start();
70+
var sut = new OtelPropagationContext();
71+
72+
// Act
73+
var spanId = sut.SpanId;
74+
75+
// Assert
76+
spanId.Should().NotBe(default(SpanId));
77+
spanId.Should().Be(activity.SpanId.AsSentrySpanId());
78+
}
79+
80+
[Fact]
81+
public void ParentSpanId_NoActivityCurrent_ReturnsNull()
82+
{
83+
// Arrange
84+
var sut = new OtelPropagationContext();
85+
Activity.Current = null;
86+
87+
// Act
88+
var parentSpanId = sut.ParentSpanId;
89+
90+
// Assert
91+
parentSpanId.Should().BeNull();
92+
}
93+
94+
[Fact]
95+
public void ParentSpanId_WithActivityCurrent_ReturnsParentSpanIdFromActivity()
96+
{
97+
// Arrange
98+
using var parentActivity = new Activity("parent").Start();
99+
using var childActivity = new Activity("child").Start();
100+
var sut = new OtelPropagationContext();
101+
102+
// Act
103+
var parentSpanId = sut.ParentSpanId;
104+
105+
// Assert
106+
parentSpanId.Should().NotBeNull();
107+
parentSpanId.Should().Be(parentActivity.SpanId.AsSentrySpanId());
108+
}
109+
110+
[Fact]
111+
public void DynamicSamplingContext_ByDefault_IsNull()
112+
{
113+
// Arrange & Act
114+
var sut = new OtelPropagationContext();
115+
116+
// Assert
117+
sut.DynamicSamplingContext.Should().BeNull();
118+
}
119+
120+
[Fact]
121+
public void GetOrCreateDynamicSamplingContext_DynamicSamplingContextIsNull_CreatesDynamicSamplingContext()
122+
{
123+
// Arrange
124+
using var activity = new Activity("test").Start();
125+
var sut = new OtelPropagationContext();
126+
sut.DynamicSamplingContext.Should().BeNull();
127+
128+
// Act
129+
var result = sut.GetOrCreateDynamicSamplingContext(_fixture.SentryOptions, _fixture.ActiveReplaySession);
130+
131+
// Assert
132+
result.Should().NotBeNull();
133+
sut.DynamicSamplingContext.Should().NotBeNull();
134+
sut.DynamicSamplingContext.Should().BeSameAs(result);
135+
}
136+
137+
[Fact]
138+
public void GetOrCreateDynamicSamplingContext_DynamicSamplingContextIsNotNull_ReturnsSameDynamicSamplingContext()
139+
{
140+
// Arrange
141+
using var activity = new Activity("test").Start();
142+
var sut = new OtelPropagationContext();
143+
var firstResult = sut.GetOrCreateDynamicSamplingContext(_fixture.SentryOptions, _fixture.ActiveReplaySession);
144+
145+
// Act
146+
var secondResult = sut.GetOrCreateDynamicSamplingContext(_fixture.SentryOptions, _fixture.ActiveReplaySession);
147+
148+
// Assert
149+
firstResult.Should().BeSameAs(secondResult);
150+
sut.DynamicSamplingContext.Should().BeSameAs(firstResult);
151+
}
152+
153+
[Fact]
154+
public void GetOrCreateDynamicSamplingContext_WithActiveReplaySession_IncludesReplayIdInDynamicSamplingContext()
155+
{
156+
// Arrange
157+
using var activity = new Activity("test").Start();
158+
var sut = new OtelPropagationContext();
159+
160+
// Act
161+
var result = sut.GetOrCreateDynamicSamplingContext(_fixture.SentryOptions, _fixture.ActiveReplaySession);
162+
163+
// Assert
164+
result.Should().NotBeNull();
165+
result.Items.Should().Contain(kvp => kvp.Key == "replay_id" && kvp.Value == _fixture.ActiveReplayId.ToString());
166+
}
167+
}

0 commit comments

Comments
 (0)