[Shims, W3C.Tests] Nullable (open-telemetry#5797)

Co-authored-by: Piotr Kiełkowicz <[email protected]>
Co-authored-by: Mikel Blanchard <[email protected]>

Author: 3 people

src/OpenTelemetry.Shims.OpenTracing/.publicApi/PublicAPI.Shipped.txt

@@ -0,0 +1 @@
+#nullable enable

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<TCarrier>(OpenTracing.Propagation.IFormat<TCarrier> format, TCarrier carrier) -> OpenTracing.ISpanContext
-OpenTelemetry.Shims.OpenTracing.TracerShim.Inject<TCarrier>(OpenTracing.ISpanContext spanContext, OpenTracing.Propagation.IFormat<TCarrier> 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<TCarrier>(OpenTracing.Propagation.IFormat<TCarrier>! format, TCarrier carrier) -> OpenTracing.ISpanContext?
+OpenTelemetry.Shims.OpenTracing.TracerShim.Inject<TCarrier>(OpenTracing.ISpanContext! spanContext, OpenTracing.Propagation.IFormat<TCarrier>! 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

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

src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj

@@ -4,9 +4,6 @@
     <Description>OpenTracing shim for OpenTelemetry .NET</Description>
     <PackageTags>$(PackageTags);distributed-tracing;OpenTracing</PackageTags>
     <MinVerTagPrefix>coreunstable-</MinVerTagPrefix>
-
-    <!-- this is temporary. will remove in future PR. -->
-    <Nullable>disable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>

src/OpenTelemetry.Shims.OpenTracing/ScopeManagerShim.cs

@@ -19,7 +19,7 @@ internal sealed class ScopeManagerShim : IScopeManager
 #endif
 
     /// <inheritdoc/>
-    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;

src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs

@@ -32,12 +32,12 @@ internal sealed class SpanBuilderShim : ISpanBuilder
     /// <summary>
     /// The OpenTelemetry attributes. These correspond to OpenTracing Tags.
     /// </summary>
-    private readonly List<KeyValuePair<string, object>> attributes = new();
+    private readonly SpanAttributes attributes = new();
 
     /// <summary>
     /// The parent as an TelemetrySpan, if any.
     /// </summary>
-    private TelemetrySpan parentSpan;
+    private TelemetrySpan? parentSpan;
 
     /// <summary>
     /// 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;
 
     /// <inheritdoc/>
-    public ISpanBuilder AsChildOf(ISpanContext parent)
+    public ISpanBuilder AsChildOf(ISpanContext? parent)
     {
         if (parent == null)
         {
@@ -81,7 +81,7 @@ public ISpanBuilder AsChildOf(ISpanContext parent)
     }
 
     /// <inheritdoc/>
-    public ISpanBuilder AsChildOf(ISpan parent)
+    public ISpanBuilder AsChildOf(ISpan? parent)
     {
         if (parent == null)
         {
@@ -98,7 +98,7 @@ public ISpanBuilder AsChildOf(ISpan parent)
     }
 
     /// <inheritdoc/>
-    public ISpanBuilder AddReference(string referenceType, ISpanContext referencedContext)
+    public ISpanBuilder AddReference(string referenceType, ISpanContext? referencedContext)
     {
         if (referencedContext == null)
         {
@@ -132,30 +132,25 @@ public ISpanBuilder IgnoreActiveSpan()
     /// <inheritdoc/>
     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)
     }
 
     /// <inheritdoc/>
-    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<string, object>(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<string, object>(key, value));
+            this.attributes.Add(key, value);
         }
 
         return this;
@@ -233,31 +228,31 @@ public ISpanBuilder WithTag(string key, bool value)
     /// <inheritdoc/>
     public ISpanBuilder WithTag(string key, int value)
     {
-        this.attributes.Add(new KeyValuePair<string, object>(key, value));
+        this.attributes.Add(key, value);
         return this;
     }
 
     /// <inheritdoc/>
     public ISpanBuilder WithTag(string key, double value)
     {
-        this.attributes.Add(new KeyValuePair<string, object>(key, value));
+        this.attributes.Add(key, value);
         return this;
     }
 
     /// <inheritdoc/>
     public ISpanBuilder WithTag(global::OpenTracing.Tag.BooleanTag tag, bool value)
     {
-        Guard.ThrowIfNull(tag?.Key);
+        Guard.ThrowIfNull(tag);
 
         return this.WithTag(tag.Key, value);
     }
 
     /// <inheritdoc/>
-    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
     /// <inheritdoc/>
     public ISpanBuilder WithTag(global::OpenTracing.Tag.IntTag tag, int value)
     {
-        Guard.ThrowIfNull(tag?.Key);
+        Guard.ThrowIfNull(tag);
 
         return this.WithTag(tag.Key, value);
     }
 
     /// <inheritdoc/>
-    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);
     }

src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs

@@ -39,7 +39,7 @@ public SpanShim(TelemetrySpan span)
     /// <inheritdoc/>
     public ISpanContext Context => this.spanContextShim;
 
-    public TelemetrySpan Span { get; private set; }
+    public TelemetrySpan Span { get; }
 
     /// <inheritdoc/>
     public void Finish()
@@ -54,7 +54,7 @@ public void Finish(DateTimeOffset finishTimestamp)
     }
 
     /// <inheritdoc/>
-    public string GetBaggageItem(string key)
+    public string? GetBaggageItem(string key)
         => Baggage.GetBaggage(key);
 
     /// <inheritdoc/>
@@ -137,7 +137,7 @@ public ISpan Log(DateTimeOffset timestamp, string @event)
     }
 
     /// <inheritdoc/>
-    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)
     }
 
     /// <inheritdoc/>
-    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)
     /// <inheritdoc/>
     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);
     }
 
     /// <inheritdoc/>
-    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);
     }
 
     /// <inheritdoc/>
     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);
     }
 
     /// <inheritdoc/>
-    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);
     }
 
     /// <summary>
@@ -234,7 +242,7 @@ public ISpan SetTag(global::OpenTracing.Tag.StringTag tag, string value)
     /// <returns>A 2-Tuple containing the event name and payload information.</returns>
     private static Tuple<string, IDictionary<string, object>> ConvertToEventPayload(IEnumerable<KeyValuePair<string, object>> fields)
     {
-        string eventName = null;
+        string? eventName = null;
         var attributes = new Dictionary<string, object>();
 
         foreach (var field in fields)
@@ -268,7 +276,7 @@ private static Tuple<string, IDictionary<string, object>> ConvertToEventPayload(
             else
             {
                 // TODO should we completely ignore unsupported types?
-                attributes.Add(field.Key, field.Value.ToString());
+                attributes.Add(field.Key, field.Value.ToString()!);
             }
         }
 

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;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="TracerShim"/> class.
@@ -30,7 +30,7 @@ public TracerShim(Trace.TracerProvider tracerProvider)
     /// </summary>
     /// <param name="tracerProvider"><see cref="Trace.TracerProvider"/>.</param>
     /// <param name="textFormat"><see cref="TextMapPropagator"/>.</param>
-    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; }
 
     /// <inheritdoc/>
-    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
     }
 
     /// <inheritdoc/>
-    public global::OpenTracing.ISpanContext Extract<TCarrier>(IFormat<TCarrier> format, TCarrier carrier)
+    public global::OpenTracing.ISpanContext? Extract<TCarrier>(IFormat<TCarrier> 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<string> GetCarrierKeyValue(Dictionary<string, IEnumerable<string>> source, string key)
+            static IEnumerable<string>? GetCarrierKeyValue(Dictionary<string, IEnumerable<string>> source, string key)
             {
                 if (key == null || !source.TryGetValue(key, out var value))
                 {

test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/OpenTelemetry.Instrumentation.W3cTraceContext.Tests.csproj

@@ -3,8 +3,6 @@
   <PropertyGroup>
     <Description>Unit test project for OpenTelemetry ASP.NET Core instrumentation for W3C Trace Context Trace</Description>
     <TargetFrameworks>$(TargetFrameworksForAspNetCoreTests)</TargetFrameworks>
-    <!-- this is temporary. will remove in future PR. -->
-    <Nullable>disable</Nullable>
   </PropertyGroup>
 
   <ItemGroup>