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.

Sign up for GitHub

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

Jump to bottom

Tiny cleanup of static Tracer instance usages #4936

Merged
merged 4 commits into from
Nov 30, 2023
Merged

Tiny cleanup of static Tracer instance usages #4936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aae42fb
nit: use the tracer instance associated with a span
andrewlock Nov 29, 2023
60fb88f
Add methods required by v3
andrewlock Nov 29, 2023
e747a17
Fix OTEL remapper tests
andrewlock Nov 29, 2023
ece6aa1
Handle the fact that we may not have a tracer instance associated wit…
andrewlock Nov 29, 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 tracer/src/Datadog.Trace/Activity/OtlpHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ internal static class OtlpHelpers
internal static void UpdateSpanFromActivity<TInner>(TInner activity, Span span)
where TInner : IActivity
{
var activity5 = activity as IActivity5;

AgentConvertSpan(activity, span);
}

Expand Down Expand Up @@ -61,8 +59,10 @@ private static void AgentConvertSpan<TInner>(TInner activity, Span span)
}

// Fixup "version" tag
if (Tracer.Instance.Settings.ServiceVersionInternal is null
Copy link
Member

Choose a reason for hiding this comment

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

Good catch.

&& span.GetTag("service.version") is { Length: > 1 } otelServiceVersion)
// Fallback to static instance if no tracer associated with the trace
var tracer = span.Context.TraceContext?.Tracer ?? Tracer.Instance;
if (tracer.Settings.ServiceVersionInternal is null
&& span.GetTag("service.version") is { Length: > 1 } otelServiceVersion)
{
span.SetTag(Tags.Version, otelServiceVersion);
}
Expand Down Expand Up @@ -96,7 +96,7 @@ private static void AgentConvertSpan<TInner>(TInner activity, Span span)

// Later: Support config 'span_name_as_resource_name'
// Later: Support config 'span_name_remappings'
if (Tracer.Instance.Settings.OpenTelemetryLegacyOperationNameEnabled && activity5 is not null && string.IsNullOrEmpty(span.OperationName))
if (tracer.Settings.OpenTelemetryLegacyOperationNameEnabled && activity5 is not null && string.IsNullOrEmpty(span.OperationName))
{
span.OperationName = activity5.Source.Name switch
{
Expand Down
5 changes: 5 additions & 0 deletions tracer/src/Datadog.Trace/ExtensionMethods/SpanExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ public static class SpanExtensions
public static void SetTraceSamplingPriority(this ISpan span, SamplingPriority samplingPriority)
{
TelemetryFactory.Metrics.Record(PublicApiUsage.SpanExtensions_SetTraceSamplingPriority);
SetTraceSamplingPriorityInternal(span, samplingPriority);
Copy link
Member

Choose a reason for hiding this comment

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

fwiw, I think longer term we should move away from using the SamplingPriority enum or even numeric values (-1, 0, 1, 2) in the public API. From a user's perspective, it should be a Boolean keep/drop. Internally, we already use int everywhere instead of SamplingPriority.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good to me! v3? 😄

}

internal static void SetTraceSamplingPriorityInternal(this ISpan span, SamplingPriority samplingPriority)
{
if (span == null) { ThrowHelper.ThrowArgumentNullException(nameof(span)); }

if (span.Context is SpanContext { TraceContext: { } traceContext })
Expand Down
10 changes: 10 additions & 0 deletions tracer/src/Datadog.Trace/SpanContextExtractor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,21 @@ public SpanContextExtractor()
TelemetryFactory.Metrics.Record(PublicApiUsage.SpanContextExtractor_Ctor);
}

internal SpanContextExtractor(bool unusedParamNotToUsePublicApi)
{
// unused parameter is to give us a non-public API we can use
}

/// <inheritdoc />
[PublicApi]
public ISpanContext? Extract<TCarrier>(TCarrier carrier, Func<TCarrier, string, IEnumerable<string?>> getter)
{
TelemetryFactory.Metrics.Record(PublicApiUsage.SpanContextExtractor_Extract);
return ExtractInternal(carrier, getter);
}

internal static ISpanContext? ExtractInternal<TCarrier>(TCarrier carrier, Func<TCarrier, string, IEnumerable<string?>> getter)
{
var spanContext = SpanContextPropagator.Instance.Extract(carrier, getter);
if (spanContext is not null
&& Tracer.Instance.TracerManager.DataStreamsManager is { IsEnabled: true } dsm
Expand Down
4 changes: 4 additions & 0 deletions tracer/src/Datadog.Trace/SpanContextInjector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ public SpanContextInjector()
public void Inject<TCarrier>(TCarrier carrier, Action<TCarrier, string, string> setter, ISpanContext context)
{
TelemetryFactory.Metrics.Record(PublicApiUsage.SpanContextInjector_Inject);
InjectInternal(carrier, setter, context);
}

internal static void InjectInternal<TCarrier>(TCarrier carrier, Action<TCarrier, string, string> setter, ISpanContext context)
{
if (context is SpanContext spanContext)
{
SpanContextPropagator.Instance.Inject(spanContext, carrier, setter);
Expand Down
8 changes: 8 additions & 0 deletions tracer/src/Datadog.Trace/SpanExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ public static partial class SpanExtensions
public static void SetUser(this ISpan span, UserDetails userDetails)
{
TelemetryFactory.Metrics.Record(PublicApiUsage.SpanExtensions_SetUser);
SetUserInternal(span, userDetails);
}

internal static void SetUserInternal(this ISpan span, UserDetails userDetails)
{
if (span is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(span));
Expand Down Expand Up @@ -96,7 +100,11 @@ public static void SetUser(this ISpan span, UserDetails userDetails)
public static ISpan SetTag(this ISpan span, string key, double? value)
{
TelemetryFactory.Metrics.Record(PublicApiUsage.SpanExtensions_SetTag);
return span.SetTagInternal(key, value);
}

internal static ISpan SetTagInternal(this ISpan span, string key, double? value)
{
if (span is null)
{
ThrowHelper.ThrowArgumentNullException(nameof(span));
Expand Down
36 changes: 30 additions & 6 deletions tracer/test/Datadog.Trace.Tests/OpenTelemetrySpecialTagRemapperTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
using System.Collections.Generic;
using Datadog.Trace.Activity;
using Datadog.Trace.Activity.DuckTypes;
using Datadog.Trace.Agent;
using Datadog.Trace.Configuration;
using Datadog.Trace.Sampling;
using Moq;
using Xunit;

Expand All @@ -15,6 +18,17 @@ namespace Datadog.Trace.Tests
[Collection(nameof(OpenTelemetrySpecialTagRemapperTests))]
public class OpenTelemetrySpecialTagRemapperTests
{
private readonly Tracer _tracer;

public OpenTelemetrySpecialTagRemapperTests()
{
var settings = new TracerSettings();
var writerMock = new Mock<IAgentWriter>();
var samplerMock = new Mock<ITraceSampler>();

_tracer = new Tracer(settings, writerMock.Object, samplerMock.Object, scopeManager: null, statsd: null);
}

[Fact]
public void OperationName_Tag_Should_Override_OperationName()
{
Expand All @@ -25,11 +39,13 @@ public void OperationName_Tag_Should_Override_OperationName()
var tagObjects = new Dictionary<string, object>
{
{ "http.request.method", "GET" },
{ "operation.name", expected }
{ "operation.name", inputValue }
};
activityMock.Setup(x => x.TagObjects).Returns(tagObjects);

var span = new Span(new SpanContext(1, 1), DateTimeOffset.UtcNow);
var spanContext = _tracer.CreateSpanContext(parent: null, serviceName: null, traceId: new TraceId(0, 1), spanId: 1);
var span = new Span(spanContext, DateTimeOffset.UtcNow);
using var scope = new Scope(parent: null, span, new AsyncLocalScopeManager(), finishOnClose: true);
OtlpHelpers.UpdateSpanFromActivity(activityMock.Object, span);

Assert.Equal(expected, span.OperationName);
Expand All @@ -48,7 +64,9 @@ public void ResourceName_Tag_Should_Override_ResourceName()
};
activityMock.Setup(x => x.TagObjects).Returns(tagObjects);

var span = new Span(new SpanContext(1, 1), DateTimeOffset.UtcNow);
var spanContext = _tracer.CreateSpanContext(parent: null, serviceName: null, traceId: new TraceId(0, 1), spanId: 1);
var span = new Span(spanContext, DateTimeOffset.UtcNow);
using var scope = new Scope(parent: null, span, new AsyncLocalScopeManager(), finishOnClose: true);
OtlpHelpers.UpdateSpanFromActivity(activityMock.Object, span);

Assert.Equal(expected, span.ResourceName);
Expand All @@ -67,7 +85,9 @@ public void ServiceName_Tag_Should_Override_ServiceName()
};
activityMock.Setup(x => x.TagObjects).Returns(tagObjects);

var span = new Span(new SpanContext(1, 1), DateTimeOffset.UtcNow);
var spanContext = _tracer.CreateSpanContext(parent: null, serviceName: null, traceId: new TraceId(0, 1), spanId: 1);
var span = new Span(spanContext, DateTimeOffset.UtcNow);
using var scope = new Scope(parent: null, span, new AsyncLocalScopeManager(), finishOnClose: true);
OtlpHelpers.UpdateSpanFromActivity(activityMock.Object, span);

Assert.Equal(expected, span.ServiceName);
Expand All @@ -86,7 +106,9 @@ public void SpanType_Tag_Should_Override_SpanType()
};
activityMock.Setup(x => x.TagObjects).Returns(tagObjects);

var span = new Span(new SpanContext(1, 1), DateTimeOffset.UtcNow);
var spanContext = _tracer.CreateSpanContext(parent: null, serviceName: null, traceId: new TraceId(0, 1), spanId: 1);
var span = new Span(spanContext, DateTimeOffset.UtcNow);
using var scope = new Scope(parent: null, span, new AsyncLocalScopeManager(), finishOnClose: true);
OtlpHelpers.UpdateSpanFromActivity(activityMock.Object, span);

Assert.Equal(expected, span.Type);
Expand All @@ -106,7 +128,9 @@ public void AnalyticsEvent_Tag_Should_Override_AnalyticsSamplingRateMetric(strin
};
activityMock.Setup(x => x.TagObjects).Returns(tagObjects);

var span = new Span(new SpanContext(1, 1), DateTimeOffset.UtcNow);
var spanContext = _tracer.CreateSpanContext(parent: null, serviceName: null, traceId: new TraceId(0, 1), spanId: 1);
var span = new Span(spanContext, DateTimeOffset.UtcNow);
using var scope = new Scope(parent: null, span, new AsyncLocalScopeManager(), finishOnClose: true);
OtlpHelpers.UpdateSpanFromActivity(activityMock.Object, span);

Assert.Equal(expected, span.GetMetric(Tags.Analytics));
Expand Down
Loading