Skip to content

Commit

Permalink
[Shims, W3C.Tests] Nullable (#5797)
Browse files Browse the repository at this point in the history
Co-authored-by: Piotr Kiełkowicz <[email protected]>
Co-authored-by: Mikel Blanchard <[email protected]>
  • Loading branch information
3 people authored Sep 16, 2024
1 parent 9b83188 commit 1c01770
Show file tree
Hide file tree
Showing 16 changed files with 170 additions and 137 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#nullable enable
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:/open-telemetry/opentelemetry-dotnet/pull/5797))

## 1.9.0-beta.2

Released 2024-Jun-24
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
8 changes: 4 additions & 4 deletions src/OpenTelemetry.Shims.OpenTracing/ScopeManagerShim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal sealed class ScopeManagerShim : IScopeManager
#endif

/// <inheritdoc/>
public IScope Active
public IScope? Active
{
get
{
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down
59 changes: 27 additions & 32 deletions src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
{
Expand All @@ -81,7 +81,7 @@ public ISpanBuilder AsChildOf(ISpanContext parent)
}

/// <inheritdoc/>
public ISpanBuilder AsChildOf(ISpan parent)
public ISpanBuilder AsChildOf(ISpan? parent)
{
if (parent == null)
{
Expand All @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
{
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
36 changes: 22 additions & 14 deletions src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -54,7 +54,7 @@ public void Finish(DateTimeOffset finishTimestamp)
}

/// <inheritdoc/>
public string GetBaggageItem(string key)
public string? GetBaggageItem(string key)
=> Baggage.GetBaggage(key);

/// <inheritdoc/>
Expand Down Expand Up @@ -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;
Expand All @@ -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);

Expand Down Expand Up @@ -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>
Expand All @@ -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)
Expand Down Expand Up @@ -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()!);
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);

Expand All @@ -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
{
Expand All @@ -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);
Expand All @@ -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))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading

0 comments on commit 1c01770

Please sign in to comment.