From 1c01770882bb5113ff308b2b4398347fab6e0404 Mon Sep 17 00:00:00 2001 From: Yevhenii Solomchenko Date: Tue, 17 Sep 2024 00:03:39 +0200 Subject: [PATCH] [Shims, W3C.Tests] Nullable (#5797) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Piotr Kiełkowicz Co-authored-by: Mikel Blanchard --- .../.publicApi/PublicAPI.Shipped.txt | 1 + .../.publicApi/PublicAPI.Unshipped.txt | 14 +-- .../CHANGELOG.md | 4 + .../OpenTelemetry.Shims.OpenTracing.csproj | 3 - .../ScopeManagerShim.cs | 8 +- .../SpanBuilderShim.cs | 59 +++++----- .../SpanShim.cs | 36 ++++--- .../TracerShim.cs | 10 +- ...strumentation.W3cTraceContext.Tests.csproj | 2 - .../W3CTraceContextTests.cs | 8 +- .../IntegrationTests.cs | 2 +- ...enTelemetry.Shims.OpenTracing.Tests.csproj | 2 - .../ScopeManagerShimTests.cs | 2 + .../SpanBuilderShimTests.cs | 39 ++++--- .../SpanShimTests.cs | 101 ++++++++++-------- .../TracerShimTests.cs | 16 +-- 16 files changed, 170 insertions(+), 137 deletions(-) diff --git a/src/OpenTelemetry.Shims.OpenTracing/.publicApi/PublicAPI.Shipped.txt b/src/OpenTelemetry.Shims.OpenTracing/.publicApi/PublicAPI.Shipped.txt index e69de29bb2d..7dc5c58110b 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/.publicApi/PublicAPI.Shipped.txt +++ b/src/OpenTelemetry.Shims.OpenTracing/.publicApi/PublicAPI.Shipped.txt @@ -0,0 +1 @@ +#nullable enable diff --git a/src/OpenTelemetry.Shims.OpenTracing/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Shims.OpenTracing/.publicApi/PublicAPI.Unshipped.txt index 90f671f8a12..c5d387112fb 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Shims.OpenTracing/.publicApi/PublicAPI.Unshipped.txt @@ -1,8 +1,8 @@ OpenTelemetry.Shims.OpenTracing.TracerShim -OpenTelemetry.Shims.OpenTracing.TracerShim.ActiveSpan.get -> OpenTracing.ISpan -OpenTelemetry.Shims.OpenTracing.TracerShim.BuildSpan(string operationName) -> OpenTracing.ISpanBuilder -OpenTelemetry.Shims.OpenTracing.TracerShim.Extract(OpenTracing.Propagation.IFormat format, TCarrier carrier) -> OpenTracing.ISpanContext -OpenTelemetry.Shims.OpenTracing.TracerShim.Inject(OpenTracing.ISpanContext spanContext, OpenTracing.Propagation.IFormat format, TCarrier carrier) -> void -OpenTelemetry.Shims.OpenTracing.TracerShim.ScopeManager.get -> OpenTracing.IScopeManager -OpenTelemetry.Shims.OpenTracing.TracerShim.TracerShim(OpenTelemetry.Trace.TracerProvider tracerProvider) -> void -OpenTelemetry.Shims.OpenTracing.TracerShim.TracerShim(OpenTelemetry.Trace.TracerProvider tracerProvider, OpenTelemetry.Context.Propagation.TextMapPropagator textFormat) -> void +OpenTelemetry.Shims.OpenTracing.TracerShim.ActiveSpan.get -> OpenTracing.ISpan? +OpenTelemetry.Shims.OpenTracing.TracerShim.BuildSpan(string! operationName) -> OpenTracing.ISpanBuilder! +OpenTelemetry.Shims.OpenTracing.TracerShim.Extract(OpenTracing.Propagation.IFormat! format, TCarrier carrier) -> OpenTracing.ISpanContext? +OpenTelemetry.Shims.OpenTracing.TracerShim.Inject(OpenTracing.ISpanContext! spanContext, OpenTracing.Propagation.IFormat! format, TCarrier carrier) -> void +OpenTelemetry.Shims.OpenTracing.TracerShim.ScopeManager.get -> OpenTracing.IScopeManager! +OpenTelemetry.Shims.OpenTracing.TracerShim.TracerShim(OpenTelemetry.Trace.TracerProvider! tracerProvider) -> void +OpenTelemetry.Shims.OpenTracing.TracerShim.TracerShim(OpenTelemetry.Trace.TracerProvider! tracerProvider, OpenTelemetry.Context.Propagation.TextMapPropagator? textFormat) -> void diff --git a/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md b/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md index 68929da6a4e..23775d1cdd8 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md +++ b/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md @@ -6,6 +6,10 @@ Notes](../../RELEASENOTES.md). ## Unreleased +* Fixed an issue causing all tag values added via the `ISpanBuilder` API to be + converted to strings on the `ISpan` started from the builder. + ([#5797](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5797)) + ## 1.9.0-beta.2 Released 2024-Jun-24 diff --git a/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj b/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj index e746024d006..127cbfb3aa9 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj +++ b/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj @@ -4,9 +4,6 @@ OpenTracing shim for OpenTelemetry .NET $(PackageTags);distributed-tracing;OpenTracing coreunstable- - - - disable diff --git a/src/OpenTelemetry.Shims.OpenTracing/ScopeManagerShim.cs b/src/OpenTelemetry.Shims.OpenTracing/ScopeManagerShim.cs index f9464d966a1..6b2fb783355 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/ScopeManagerShim.cs +++ b/src/OpenTelemetry.Shims.OpenTracing/ScopeManagerShim.cs @@ -19,7 +19,7 @@ internal sealed class ScopeManagerShim : IScopeManager #endif /// - public IScope Active + public IScope? Active { get { @@ -56,7 +56,7 @@ public IScope Activate(ISpan span, bool finishSpanOnDispose) Interlocked.Decrement(ref this.spanScopeTableCount); } #endif - scope.Dispose(); + scope!.Dispose(); }); SpanScopeTable.Add(shim.Span, instrumentation); @@ -69,9 +69,9 @@ public IScope Activate(ISpan span, bool finishSpanOnDispose) private sealed class ScopeInstrumentation : IScope { - private readonly Action disposeAction; + private readonly Action? disposeAction; - public ScopeInstrumentation(TelemetrySpan span, Action disposeAction = null) + public ScopeInstrumentation(TelemetrySpan span, Action? disposeAction = null) { this.Span = new SpanShim(span); this.disposeAction = disposeAction; diff --git a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs index 0359762ac3e..f95b1b77be3 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs +++ b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs @@ -32,12 +32,12 @@ internal sealed class SpanBuilderShim : ISpanBuilder /// /// The OpenTelemetry attributes. These correspond to OpenTracing Tags. /// - private readonly List> attributes = new(); + private readonly SpanAttributes attributes = new(); /// /// The parent as an TelemetrySpan, if any. /// - private TelemetrySpan parentSpan; + private TelemetrySpan? parentSpan; /// /// The parent as an SpanContext, if any. @@ -70,7 +70,7 @@ public SpanBuilderShim(Tracer tracer, string spanName) private bool ParentSet => this.parentSpan != null || this.parentSpanContext.IsValid; /// - public ISpanBuilder AsChildOf(ISpanContext parent) + public ISpanBuilder AsChildOf(ISpanContext? parent) { if (parent == null) { @@ -81,7 +81,7 @@ public ISpanBuilder AsChildOf(ISpanContext parent) } /// - public ISpanBuilder AsChildOf(ISpan parent) + public ISpanBuilder AsChildOf(ISpan? parent) { if (parent == null) { @@ -98,7 +98,7 @@ public ISpanBuilder AsChildOf(ISpan parent) } /// - public ISpanBuilder AddReference(string referenceType, ISpanContext referencedContext) + public ISpanBuilder AddReference(string referenceType, ISpanContext? referencedContext) { if (referencedContext == null) { @@ -132,30 +132,25 @@ public ISpanBuilder IgnoreActiveSpan() /// public ISpan Start() { - TelemetrySpan span = null; + TelemetrySpan? span = null; // If specified, this takes precedence. if (this.ignoreActiveSpan) { - span = this.tracer.StartRootSpan(this.spanName, this.spanKind, default, this.links, this.explicitStartTime ?? default); + span = this.tracer.StartRootSpan(this.spanName, this.spanKind, this.attributes, this.links, this.explicitStartTime ?? default); } else if (this.parentSpan != null) { - span = this.tracer.StartSpan(this.spanName, this.spanKind, this.parentSpan, default, this.links, this.explicitStartTime ?? default); + span = this.tracer.StartSpan(this.spanName, this.spanKind, this.parentSpan, this.attributes, this.links, this.explicitStartTime ?? default); } else if (this.parentSpanContext.IsValid) { - span = this.tracer.StartSpan(this.spanName, this.spanKind, this.parentSpanContext, default, this.links, this.explicitStartTime ?? default); + span = this.tracer.StartSpan(this.spanName, this.spanKind, this.parentSpanContext, this.attributes, this.links, this.explicitStartTime ?? default); } if (span == null) { - span = this.tracer.StartSpan(this.spanName, this.spanKind, default(SpanContext), default, null, this.explicitStartTime ?? default); - } - - foreach (var kvp in this.attributes) - { - span.SetAttribute(kvp.Key, kvp.Value.ToString()); + span = this.tracer.StartSpan(this.spanName, this.spanKind, default(SpanContext), this.attributes, null, this.explicitStartTime ?? default); } if (this.error) @@ -184,8 +179,13 @@ public ISpanBuilder WithStartTimestamp(DateTimeOffset timestamp) } /// - public ISpanBuilder WithTag(string key, string value) + public ISpanBuilder WithTag(string key, string? value) { + if (key == null) + { + return this; + } + // see https://opentracing.io/specification/conventions/ for special key handling. if (global::OpenTracing.Tag.Tags.SpanKind.Key.Equals(key, StringComparison.Ordinal)) { @@ -204,12 +204,7 @@ public ISpanBuilder WithTag(string key, string value) } else { - // Keys must be non-null. - // Null values => string.Empty. - if (key != null) - { - this.attributes.Add(new KeyValuePair(key, value ?? string.Empty)); - } + this.attributes.Add(key, value); } return this; @@ -224,7 +219,7 @@ public ISpanBuilder WithTag(string key, bool value) } else { - this.attributes.Add(new KeyValuePair(key, value)); + this.attributes.Add(key, value); } return this; @@ -233,31 +228,31 @@ public ISpanBuilder WithTag(string key, bool value) /// public ISpanBuilder WithTag(string key, int value) { - this.attributes.Add(new KeyValuePair(key, value)); + this.attributes.Add(key, value); return this; } /// public ISpanBuilder WithTag(string key, double value) { - this.attributes.Add(new KeyValuePair(key, value)); + this.attributes.Add(key, value); return this; } /// public ISpanBuilder WithTag(global::OpenTracing.Tag.BooleanTag tag, bool value) { - Guard.ThrowIfNull(tag?.Key); + Guard.ThrowIfNull(tag); return this.WithTag(tag.Key, value); } /// - public ISpanBuilder WithTag(global::OpenTracing.Tag.IntOrStringTag tag, string value) + public ISpanBuilder WithTag(global::OpenTracing.Tag.IntOrStringTag tag, string? value) { - Guard.ThrowIfNull(tag?.Key); + Guard.ThrowIfNull(tag); - if (int.TryParse(value, out var result)) + if (value != null && int.TryParse(value, out var result)) { return this.WithTag(tag.Key, result); } @@ -268,15 +263,15 @@ public ISpanBuilder WithTag(global::OpenTracing.Tag.IntOrStringTag tag, string v /// public ISpanBuilder WithTag(global::OpenTracing.Tag.IntTag tag, int value) { - Guard.ThrowIfNull(tag?.Key); + Guard.ThrowIfNull(tag); return this.WithTag(tag.Key, value); } /// - public ISpanBuilder WithTag(global::OpenTracing.Tag.StringTag tag, string value) + public ISpanBuilder WithTag(global::OpenTracing.Tag.StringTag tag, string? value) { - Guard.ThrowIfNull(tag?.Key); + Guard.ThrowIfNull(tag); return this.WithTag(tag.Key, value); } diff --git a/src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs b/src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs index 2ab6fee4fe8..c1bde927a1b 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs +++ b/src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs @@ -39,7 +39,7 @@ public SpanShim(TelemetrySpan span) /// public ISpanContext Context => this.spanContextShim; - public TelemetrySpan Span { get; private set; } + public TelemetrySpan Span { get; } /// public void Finish() @@ -54,7 +54,7 @@ public void Finish(DateTimeOffset finishTimestamp) } /// - public string GetBaggageItem(string key) + public string? GetBaggageItem(string key) => Baggage.GetBaggage(key); /// @@ -137,7 +137,7 @@ public ISpan Log(DateTimeOffset timestamp, string @event) } /// - public ISpan SetBaggageItem(string key, string value) + public ISpan SetBaggageItem(string key, string? value) { Baggage.SetBaggage(key, value); return this; @@ -153,7 +153,7 @@ public ISpan SetOperationName(string operationName) } /// - public ISpan SetTag(string key, string value) + public ISpan SetTag(string key, string? value) { Guard.ThrowIfNull(key); @@ -201,30 +201,38 @@ public ISpan SetTag(string key, double value) /// public ISpan SetTag(global::OpenTracing.Tag.BooleanTag tag, bool value) { - return this.SetTag(tag?.Key, value); + Guard.ThrowIfNull(tag); + + return this.SetTag(tag.Key, value); } /// - public ISpan SetTag(global::OpenTracing.Tag.IntOrStringTag tag, string value) + public ISpan SetTag(global::OpenTracing.Tag.IntOrStringTag tag, string? value) { - if (int.TryParse(value, out var result)) + Guard.ThrowIfNull(tag); + + if (value != null && int.TryParse(value, out var result)) { - return this.SetTag(tag?.Key, result); + return this.SetTag(tag.Key, result); } - return this.SetTag(tag?.Key, value); + return this.SetTag(tag.Key, value); } /// public ISpan SetTag(global::OpenTracing.Tag.IntTag tag, int value) { - return this.SetTag(tag?.Key, value); + Guard.ThrowIfNull(tag); + + return this.SetTag(tag.Key, value); } /// - public ISpan SetTag(global::OpenTracing.Tag.StringTag tag, string value) + public ISpan SetTag(global::OpenTracing.Tag.StringTag tag, string? value) { - return this.SetTag(tag?.Key, value); + Guard.ThrowIfNull(tag); + + return this.SetTag(tag.Key, value); } /// @@ -234,7 +242,7 @@ public ISpan SetTag(global::OpenTracing.Tag.StringTag tag, string value) /// A 2-Tuple containing the event name and payload information. private static Tuple> ConvertToEventPayload(IEnumerable> fields) { - string eventName = null; + string? eventName = null; var attributes = new Dictionary(); foreach (var field in fields) @@ -268,7 +276,7 @@ private static Tuple> ConvertToEventPayload( else { // TODO should we completely ignore unsupported types? - attributes.Add(field.Key, field.Value.ToString()); + attributes.Add(field.Key, field.Value.ToString()!); } } diff --git a/src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs b/src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs index 3ed448da945..504b28c5a52 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs +++ b/src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs @@ -14,7 +14,7 @@ namespace OpenTelemetry.Shims.OpenTracing; public class TracerShim : global::OpenTracing.ITracer { private readonly Trace.Tracer tracer; - private readonly TextMapPropagator definedPropagator; + private readonly TextMapPropagator? definedPropagator; /// /// Initializes a new instance of the class. @@ -30,7 +30,7 @@ public TracerShim(Trace.TracerProvider tracerProvider) /// /// . /// . - public TracerShim(Trace.TracerProvider tracerProvider, TextMapPropagator textFormat) + public TracerShim(Trace.TracerProvider tracerProvider, TextMapPropagator? textFormat) { Guard.ThrowIfNull(tracerProvider); @@ -46,7 +46,7 @@ public TracerShim(Trace.TracerProvider tracerProvider, TextMapPropagator textFor public global::OpenTracing.IScopeManager ScopeManager { get; } /// - public global::OpenTracing.ISpan ActiveSpan => this.ScopeManager.Active?.Span; + public global::OpenTracing.ISpan? ActiveSpan => this.ScopeManager.Active?.Span; private TextMapPropagator Propagator { @@ -63,7 +63,7 @@ private TextMapPropagator Propagator } /// - public global::OpenTracing.ISpanContext Extract(IFormat format, TCarrier carrier) + public global::OpenTracing.ISpanContext? Extract(IFormat format, TCarrier carrier) { Guard.ThrowIfNull(format); Guard.ThrowIfNull(carrier); @@ -79,7 +79,7 @@ private TextMapPropagator Propagator carrierMap.Add(entry.Key, new[] { entry.Value }); } - static IEnumerable GetCarrierKeyValue(Dictionary> source, string key) + static IEnumerable? GetCarrierKeyValue(Dictionary> source, string key) { if (key == null || !source.TryGetValue(key, out var value)) { diff --git a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/OpenTelemetry.Instrumentation.W3cTraceContext.Tests.csproj b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/OpenTelemetry.Instrumentation.W3cTraceContext.Tests.csproj index 8d9e8800c51..fb9e640731f 100644 --- a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/OpenTelemetry.Instrumentation.W3cTraceContext.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/OpenTelemetry.Instrumentation.W3cTraceContext.Tests.csproj @@ -3,8 +3,6 @@ Unit test project for OpenTelemetry ASP.NET Core instrumentation for W3C Trace Context Trace $(TargetFrameworksForAspNetCoreTests) - - disable diff --git a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs index b80a57f1ced..d557ea71c59 100644 --- a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs +++ b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs @@ -21,7 +21,7 @@ public class W3CTraceContextTests : IDisposable opentelemetry>docker compose --file=test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/docker-compose.yml --project-directory=. up --exit-code-from=tests --build */ private const string W3cTraceContextEnvVarName = "OTEL_W3CTRACECONTEXT"; - private static readonly Version AspNetCoreHostingVersion = typeof(Microsoft.AspNetCore.Hosting.Builder.IApplicationBuilderFactory).Assembly.GetName().Version; + private static readonly Version? AspNetCoreHostingVersion = typeof(Microsoft.AspNetCore.Hosting.Builder.IApplicationBuilderFactory).Assembly.GetName().Version; private readonly HttpClient httpClient = new(); private readonly ITestOutputHelper output; @@ -84,7 +84,7 @@ public void W3CTraceContextTestSuiteAsync(string value) // run the tests with console logger (done automatically by the CI // jobs). - if (AspNetCoreHostingVersion.Major <= 6) + if (AspNetCoreHostingVersion!.Major <= 6) { Assert.StartsWith("FAILED (failures=3)", lastLine); } @@ -137,9 +137,9 @@ private static string ParseLastLine(string output) public class Data { [JsonPropertyName("url")] - public string Url { get; set; } + public string? Url { get; set; } [JsonPropertyName("arguments")] - public Data[] Arguments { get; set; } + public Data[]? Arguments { get; set; } } } diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/IntegrationTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/IntegrationTests.cs index d8f93b2df0c..cb592e8906e 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/IntegrationTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/IntegrationTests.cs @@ -76,7 +76,7 @@ public void WithActivities( } } - var expectedExportedSpans = new string[] + var expectedExportedSpans = new string?[] { childActivitySamplingDecision == SamplingDecision.RecordAndSample ? ChildActivityName : null, shimSamplingDecision == SamplingDecision.RecordAndSample ? ShimActivityName : null, diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj b/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj index 430bb7057e2..143e90e7dc4 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj @@ -3,8 +3,6 @@ Unit test project for OpenTelemetry.Shims.OpenTracing $(TargetFrameworksForTests) $(DefineConstants);BUILDING_USING_PROJECTS - - disable diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/ScopeManagerShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/ScopeManagerShimTests.cs index 6f4ca876c55..0e24b2b1673 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/ScopeManagerShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/ScopeManagerShimTests.cs @@ -33,6 +33,7 @@ public void Active_IsNotNull() Assert.NotNull(scope); var activeScope = shim.Active; + Assert.NotNull(activeScope); Assert.Equal(scope.Span.Context.SpanId, activeScope.Span.Context.SpanId); openTracingSpan.Finish(); } @@ -64,6 +65,7 @@ public void Activate() #endif spanShim.Finish(); + Assert.NotNull(spanShim.Span.Activity); Assert.NotEqual(default, spanShim.Span.Activity.Duration); } } diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs index a3a58facfd9..b99bcb2f890 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs @@ -18,8 +18,8 @@ public class SpanBuilderShimTests public void CtorArgumentValidation() { var tracer = TracerProvider.Default.GetTracer(TracerName); - Assert.Throws(() => new SpanBuilderShim(null, "foo")); - Assert.Throws(() => new SpanBuilderShim(tracer, null)); + Assert.Throws(() => new SpanBuilderShim(null!, "foo")); + Assert.Throws(() => new SpanBuilderShim(tracer, null!)); } [Fact] @@ -36,7 +36,7 @@ public void IgnoreActiveSpan() // build var spanShim = (SpanShim)shim.Start(); - + Assert.NotNull(spanShim.Span.Activity); Assert.Equal("foo", spanShim.Span.Activity.OperationName); } @@ -51,7 +51,7 @@ public void StartWithExplicitTimestamp() // build var spanShim = (SpanShim)shim.Start(); - + Assert.NotNull(spanShim.Span.Activity); Assert.Equal(startTimestamp, spanShim.Span.Activity.StartTimeUtc); } @@ -62,11 +62,12 @@ public void AsChildOf_WithNullSpan() var shim = new SpanBuilderShim(tracer, "foo"); // Add a null parent - shim.AsChildOf((global::OpenTracing.ISpan)null); + shim.AsChildOf((global::OpenTracing.ISpan?)null); // build var spanShim = (SpanShim)shim.Start(); + Assert.NotNull(spanShim.Span.Activity); Assert.Equal("foo", spanShim.Span.Activity.OperationName); Assert.Null(spanShim.Span.Activity.Parent); } @@ -84,6 +85,7 @@ public void AsChildOf_WithSpan() // build var spanShim = (SpanShim)shim.Start(); + Assert.NotNull(spanShim.Span.Activity); Assert.Equal("foo", spanShim.Span.Activity.OperationName); Assert.NotNull(spanShim.Span.Activity.ParentId); } @@ -101,12 +103,14 @@ public void Start_ActivityOperationRootSpanChecks() var shim = new SpanBuilderShim(tracer, "foo"); var spanShim1 = (SpanShim)shim.Start(); + Assert.NotNull(spanShim1.Span.Activity); Assert.Equal("foo", spanShim1.Span.Activity.OperationName); // mis-matched root operation name shim = new SpanBuilderShim(tracer, "foo"); var spanShim2 = (SpanShim)shim.Start(); + Assert.NotNull(spanShim2.Span.Activity); Assert.Equal("foo", spanShim2.Span.Activity.OperationName); Assert.Equal(spanShim1.Context.TraceId, spanShim2.Context.TraceId); } @@ -126,6 +130,7 @@ public void AsChildOf_MultipleCallsWithSpan() // build var spanShim = (SpanShim)shim.Start(); + Assert.NotNull(spanShim.Span.Activity); Assert.Equal("foo", spanShim.Span.Activity.OperationName); Assert.Contains(spanShim.Context.TraceId, spanShim.Span.Activity.TraceId.ToHexString()); @@ -139,12 +144,13 @@ public void AsChildOf_WithNullSpanContext() var shim = new SpanBuilderShim(tracer, "foo"); // Add a null parent - shim.AsChildOf((global::OpenTracing.ISpanContext)null); + shim.AsChildOf((global::OpenTracing.ISpanContext?)null); // build var spanShim = (SpanShim)shim.Start(); // should be no parent. + Assert.NotNull(spanShim.Span.Activity); Assert.Null(spanShim.Span.Activity.Parent); } @@ -161,6 +167,7 @@ public void AsChildOfWithSpanContext() // build var spanShim = (SpanShim)shim.Start(); + Assert.NotNull(spanShim.Span.Activity); Assert.NotNull(spanShim.Span.Activity.ParentId); } @@ -182,7 +189,7 @@ public void AsChildOf_MultipleCallsWithSpanContext() // build var spanShim = (SpanShim)shim.Start(); - + Assert.NotNull(spanShim.Span.Activity); Assert.Equal("foo", spanShim.Span.Activity.OperationName); Assert.Contains(spanContext1.TraceId, spanShim.Span.Activity.ParentId); Assert.Equal(spanContext2.SpanId, spanShim.Span.Activity.Links.First().Context.SpanId.ToHexString()); @@ -200,6 +207,7 @@ public void WithTag_KeyIsSpanKindStringValue() var spanShim = (SpanShim)shim.Start(); // Not an attribute + Assert.NotNull(spanShim.Span.Activity); Assert.Empty(spanShim.Span.Activity.Tags); Assert.Equal("foo", spanShim.Span.Activity.OperationName); Assert.Equal(ActivityKind.Client, spanShim.Span.Activity.Kind); @@ -217,6 +225,7 @@ public void WithTag_KeyIsErrorStringValue() var spanShim = (SpanShim)shim.Start(); // Legacy span status tag should be set + Assert.NotNull(spanShim.Span.Activity); Assert.Equal("ERROR", spanShim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); if (VersionHelper.IsApiVersionGreaterThanOrEqualTo(1, 10)) @@ -236,17 +245,18 @@ public void WithTag_KeyIsNullStringValue() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanBuilderShim(tracer, "foo"); - shim.WithTag((string)null, "unused"); + shim.WithTag((string)null!, "unused"); // build var spanShim = (SpanShim)shim.Start(); // Null key was ignored + Assert.NotNull(spanShim.Span.Activity); Assert.Empty(spanShim.Span.Activity.Tags); } [Fact] - public void WithTag_ValueIsNullStringValue() + public void WithTag_ValueIsIgnoredWhenNull() { var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanBuilderShim(tracer, "foo"); @@ -257,8 +267,8 @@ public void WithTag_ValueIsNullStringValue() var spanShim = (SpanShim)shim.Start(); // Null value was turned into string.empty - Assert.Equal("foo", spanShim.Span.Activity.Tags.First().Key); - Assert.Equal(string.Empty, spanShim.Span.Activity.Tags.First().Value); + Assert.NotNull(spanShim.Span.Activity); + Assert.Empty(spanShim.Span.Activity.TagObjects); } [Fact] @@ -273,8 +283,8 @@ public void WithTag_KeyIsErrorBoolValue() var spanShim = (SpanShim)shim.Start(); // Legacy span status tag should be set + Assert.NotNull(spanShim.Span.Activity); Assert.Equal("ERROR", spanShim.Span.Activity.GetTagValue(SpanAttributeConstants.StatusCodeKey)); - if (VersionHelper.IsApiVersionGreaterThanOrEqualTo(1, 10)) { // Activity status code should also be set @@ -304,7 +314,8 @@ public void WithTag_VariousValueTypes() var spanShim = (SpanShim)shim.Start(); // Just verify the count - Assert.Equal(7, spanShim.Span.Activity.Tags.Count()); + Assert.NotNull(spanShim.Span.Activity); + Assert.Equal(7, spanShim.Span.Activity.TagObjects.Count()); } [Fact] @@ -319,6 +330,7 @@ public void Start() // Just check the return value is a SpanShim and that the underlying OpenTelemetry Span. // There is nothing left to verify because the rest of the tests were already calling .Start() prior to verification. Assert.NotNull(span); + Assert.NotNull(span.Span.Activity); Assert.Equal("foo", span.Span.Activity.OperationName); } @@ -337,6 +349,7 @@ public void Start_UnderAspNetCoreInstrumentation() Assert.NotNull(spanShim); var telemetrySpan = spanShim.Span; + Assert.NotNull(telemetrySpan.Activity); Assert.Same(telemetrySpan.Activity, Activity.Current); Assert.Same(parentSpan, telemetrySpan.Activity.Parent); diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs index 9911d9f76fc..66e8b63652b 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanShimTests.cs @@ -17,7 +17,7 @@ public class SpanShimTests [Fact] public void CtorArgumentValidation() { - Assert.Throws(() => new SpanShim(null)); + Assert.Throws(() => new SpanShim(null!)); } [Fact] @@ -35,9 +35,9 @@ public void FinishSpan() { var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - shim.Finish(); + Assert.NotNull(shim.Span.Activity); Assert.NotEqual(default, shim.Span.Activity.Duration); } @@ -58,7 +58,7 @@ public void FinishSpanUsingSpecificTimestamp() var endTime = DateTimeOffset.UtcNow; shim.Finish(endTime); - Assert.Equal(endTime - shim.Span.Activity.StartTimeUtc, shim.Span.Activity.Duration); + Assert.Equal(endTime - shim.Span.Activity!.StartTimeUtc, shim.Span.Activity.Duration); } [Fact] @@ -68,10 +68,10 @@ public void SetOperationName() var shim = new SpanShim(tracer.StartSpan(SpanName)); // parameter validation - Assert.Throws(() => shim.SetOperationName(null)); + Assert.Throws(() => shim.SetOperationName(null!)); shim.SetOperationName("bar"); - Assert.Equal("bar", shim.Span.Activity.DisplayName); + Assert.Equal("bar", shim.Span.Activity!.DisplayName); } [Fact] @@ -81,7 +81,7 @@ public void GetBaggageItem() var shim = new SpanShim(tracer.StartSpan(SpanName)); // parameter validation - Assert.Throws(() => shim.GetBaggageItem(null)); + Assert.Throws(() => shim.GetBaggageItem(null!)); // TODO - Method not implemented } @@ -94,8 +94,8 @@ public void Log() shim.Log("foo"); - Assert.Single(shim.Span.Activity.Events); - var first = shim.Span.Activity.Events.First(); + Assert.NotNull(shim.Span.Activity); + var first = Assert.Single(shim.Span.Activity.Events); Assert.Equal("foo", first.Name); Assert.False(first.Tags.Any()); } @@ -109,8 +109,8 @@ public void LogWithExplicitTimestamp() var now = DateTimeOffset.UtcNow; shim.Log(now, "foo"); - Assert.Single(shim.Span.Activity.Events); - var first = shim.Span.Activity.Events.First(); + Assert.NotNull(shim.Span.Activity); + var first = Assert.Single(shim.Span.Activity.Events); Assert.Equal("foo", first.Name); Assert.Equal(now, first.Timestamp); Assert.False(first.Tags.Any()); @@ -122,19 +122,21 @@ public void LogUsingFields() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - Assert.Throws(() => shim.Log((IEnumerable>)null)); + Assert.Throws(() => shim.Log((IEnumerable>)null!)); shim.Log(new List> { - new KeyValuePair("foo", "bar"), + new("foo", "bar"), }); // "event" is a special event name shim.Log(new List> { - new KeyValuePair("event", "foo"), + new("event", "foo"), }); + Assert.NotNull(shim.Span.Activity); + var first = shim.Span.Activity.Events.FirstOrDefault(); var last = shim.Span.Activity.Events.LastOrDefault(); @@ -153,20 +155,21 @@ public void LogUsingFieldsWithExplicitTimestamp() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - Assert.Throws(() => shim.Log((IEnumerable>)null)); + Assert.Throws(() => shim.Log((IEnumerable>)null!)); var now = DateTimeOffset.UtcNow; shim.Log(now, new List> { - new KeyValuePair("foo", "bar"), + new("foo", "bar"), }); // "event" is a special event name shim.Log(now, new List> { - new KeyValuePair("event", "foo"), + new("event", "foo"), }); + Assert.NotNull(shim.Span.Activity); Assert.Equal(2, shim.Span.Activity.Events.Count()); var first = shim.Span.Activity.Events.First(); var last = shim.Span.Activity.Events.Last(); @@ -186,13 +189,14 @@ public void SetTagStringValue() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - Assert.Throws(() => shim.SetTag((string)null, "foo")); + Assert.Throws(() => shim.SetTag((string)null!, "foo")); shim.SetTag("foo", "bar"); - Assert.Single(shim.Span.Activity.Tags); - Assert.Equal("foo", shim.Span.Activity.Tags.First().Key); - Assert.Equal("bar", shim.Span.Activity.Tags.First().Value); + Assert.NotNull(shim.Span.Activity); + var first = Assert.Single(shim.Span.Activity.Tags); + Assert.Equal("foo", first.Key); + Assert.Equal("bar", first.Value); } [Fact] @@ -201,13 +205,16 @@ public void SetTagBoolValue() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - Assert.Throws(() => shim.SetTag((string)null, true)); + Assert.Throws(() => shim.SetTag((string)null!, true)); shim.SetTag("foo", true); shim.SetTag(Tags.Error.Key, true); - Assert.Equal("foo", shim.Span.Activity.TagObjects.First().Key); - Assert.True((bool)shim.Span.Activity.TagObjects.First().Value); + Assert.NotNull(shim.Span.Activity); + var first = shim.Span.Activity.TagObjects.First(); + Assert.Equal("foo", first.Key); + Assert.NotNull(first.Value); + Assert.True((bool)first.Value); // A boolean tag named "error" is a special case that must be checked @@ -246,13 +253,14 @@ public void SetTagIntValue() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - Assert.Throws(() => shim.SetTag((string)null, 1)); + Assert.Throws(() => shim.SetTag((string)null!, 1)); shim.SetTag("foo", 1); + Assert.NotNull(shim.Span.Activity); Assert.Single(shim.Span.Activity.TagObjects); Assert.Equal("foo", shim.Span.Activity.TagObjects.First().Key); - Assert.Equal(1L, (int)shim.Span.Activity.TagObjects.First().Value); + Assert.Equal(1L, (int)shim.Span.Activity.TagObjects.First().Value!); } [Fact] @@ -261,13 +269,15 @@ public void SetTagDoubleValue() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - Assert.Throws(() => shim.SetTag(null, 1D)); + Assert.Throws(() => shim.SetTag(null!, 1D)); shim.SetTag("foo", 1D); - Assert.Single(shim.Span.Activity.TagObjects); - Assert.Equal("foo", shim.Span.Activity.TagObjects.First().Key); - Assert.Equal(1, (double)shim.Span.Activity.TagObjects.First().Value); + Assert.NotNull(shim.Span.Activity); + var first = Assert.Single(shim.Span.Activity.TagObjects); + Assert.Equal("foo", first.Key); + Assert.NotNull(first.Value); + Assert.Equal(1, (double)first.Value); } [Fact] @@ -276,13 +286,14 @@ public void SetTagStringTagValue() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - Assert.Throws(() => shim.SetTag((StringTag)null, "foo")); + Assert.Throws(() => shim.SetTag((StringTag)null!, "foo")); shim.SetTag(new StringTag("foo"), "bar"); - Assert.Single(shim.Span.Activity.Tags); - Assert.Equal("foo", shim.Span.Activity.Tags.First().Key); - Assert.Equal("bar", shim.Span.Activity.Tags.First().Value); + Assert.NotNull(shim.Span.Activity); + var first = Assert.Single(shim.Span.Activity.Tags); + Assert.Equal("foo", first.Key); + Assert.Equal("bar", first.Value); } [Fact] @@ -291,13 +302,15 @@ public void SetTagIntTagValue() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - Assert.Throws(() => shim.SetTag((IntTag)null, 1)); + Assert.Throws(() => shim.SetTag((IntTag)null!, 1)); shim.SetTag(new IntTag("foo"), 1); - Assert.Single(shim.Span.Activity.TagObjects); - Assert.Equal("foo", shim.Span.Activity.TagObjects.First().Key); - Assert.Equal(1L, (int)shim.Span.Activity.TagObjects.First().Value); + Assert.NotNull(shim.Span.Activity); + var first = Assert.Single(shim.Span.Activity.TagObjects); + Assert.Equal("foo", first.Key); + Assert.NotNull(first.Value); + Assert.Equal(1L, (int)first.Value); } [Fact] @@ -306,17 +319,21 @@ public void SetTagIntOrStringTagValue() var tracer = TracerProvider.Default.GetTracer(TracerName); var shim = new SpanShim(tracer.StartSpan(SpanName)); - Assert.Throws(() => shim.SetTag((IntOrStringTag)null, "foo")); + Assert.Throws(() => shim.SetTag((IntOrStringTag)null!, "foo")); shim.SetTag(new IntOrStringTag("foo"), 1); shim.SetTag(new IntOrStringTag("bar"), "baz"); + Assert.NotNull(shim.Span.Activity); Assert.Equal(2, shim.Span.Activity.TagObjects.Count()); - Assert.Equal("foo", shim.Span.Activity.TagObjects.First().Key); - Assert.Equal(1L, (int)shim.Span.Activity.TagObjects.First().Value); + var first = shim.Span.Activity.TagObjects.First(); + Assert.Equal("foo", first.Key); + Assert.NotNull(first.Value); + Assert.Equal(1L, (int)first.Value); - Assert.Equal("bar", shim.Span.Activity.TagObjects.Last().Key); - Assert.Equal("baz", shim.Span.Activity.TagObjects.Last().Value); + var second = shim.Span.Activity.TagObjects.Last(); + Assert.Equal("bar", second.Key); + Assert.Equal("baz", second.Value); } } diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/TracerShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/TracerShimTests.cs index 578ed8ccbf3..64f08959424 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/TracerShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/TracerShimTests.cs @@ -17,10 +17,10 @@ public class TracerShimTests public void CtorArgumentValidation() { // null tracer provider and text format - Assert.Throws(() => new TracerShim(null, null)); + Assert.Throws(() => new TracerShim(null!, null)); // null tracer provider - Assert.Throws(() => new TracerShim(null, new TraceContextPropagator())); + Assert.Throws(() => new TracerShim(null!, new TraceContextPropagator())); } [Fact] @@ -50,10 +50,10 @@ public void Inject_ArgumentValidation() var testFormat = new TestFormatTextMap(); var testCarrier = new TestTextMap(); - Assert.Throws(() => shim.Inject(null, testFormat, testCarrier)); + Assert.Throws(() => shim.Inject(null!, testFormat, testCarrier)); Assert.Throws(() => shim.Inject(new TestSpanContext(), testFormat, testCarrier)); - Assert.Throws(() => shim.Inject(spanContextShim, null, testCarrier)); - Assert.Throws(() => shim.Inject(spanContextShim, testFormat, null)); + Assert.Throws(() => shim.Inject(spanContextShim, null!, testCarrier)); + Assert.Throws(() => shim.Inject(spanContextShim, testFormat!, null)); } [Fact] @@ -76,8 +76,8 @@ public void Extract_ArgumentValidation() { var shim = new TracerShim(TracerProvider.Default, new TraceContextPropagator()); - Assert.Throws(() => shim.Extract(null, new TestTextMap())); - Assert.Throws(() => shim.Extract(new TestFormatTextMap(), null)); + Assert.Throws(() => shim.Extract(null!, new TestTextMap())); + Assert.Throws(() => shim.Extract(new TestFormatTextMap()!, null)); } [Fact] @@ -129,7 +129,7 @@ public void InjectExtract_TextMap_Ok() // then extract var extractedSpanContext = shim.Extract(BuiltinFormats.TextMap, carrier); - AssertOpenTracerSpanContextEqual(spanContextShim, extractedSpanContext); + AssertOpenTracerSpanContextEqual(spanContextShim, extractedSpanContext!); } private static void AssertOpenTracerSpanContextEqual(ISpanContext source, ISpanContext target)