Skip to content

Commit

Permalink
[Shims.OpenTracing] enable code analysis (#6160)
Browse files Browse the repository at this point in the history
Co-authored-by: Rajkumar Rangaraj <rajrang@microsoft.com>
  • Loading branch information
Kielek and rajkumar-rangaraj authored Mar 11, 2025
1 parent 8943733 commit c977f73
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<Description>OpenTracing shim for OpenTelemetry .NET</Description>
<PackageTags>$(PackageTags);distributed-tracing;OpenTracing</PackageTags>
<MinVerTagPrefix>coreunstable-</MinVerTagPrefix>
<AnalysisLevel>latest-all</AnalysisLevel>
</PropertyGroup>

<ItemGroup>
Expand Down
9 changes: 3 additions & 6 deletions src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ internal sealed class SpanBuilderShim : ISpanBuilder
/// <summary>
/// The OpenTelemetry links. These correspond loosely to OpenTracing references.
/// </summary>
private readonly List<Link> links = new();
private readonly List<Link> links = [];

/// <summary>
/// The OpenTelemetry attributes. These correspond to OpenTracing Tags.
Expand Down Expand Up @@ -65,7 +65,7 @@ public SpanBuilderShim(Tracer tracer, string spanName)
this.ScopeManager = new ScopeManagerShim();
}

private IScopeManager ScopeManager { get; }
private ScopeManagerShim ScopeManager { get; }

private bool ParentSet => this.parentSpan != null || this.parentSpanContext.IsValid;

Expand Down Expand Up @@ -148,10 +148,7 @@ public ISpan Start()
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), this.attributes, null, this.explicitStartTime ?? default);
}
span ??= this.tracer.StartSpan(this.spanName, this.spanKind, default(SpanContext), this.attributes, null, this.explicitStartTime ?? default);

if (this.error)
{
Expand Down
6 changes: 3 additions & 3 deletions src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ internal sealed class SpanShim : ISpan
/// </summary>
public const string DefaultEventName = "log";

private static readonly IReadOnlyCollection<Type> OpenTelemetrySupportedAttributeValueTypes = new List<Type>
{
private static readonly IReadOnlyCollection<Type> OpenTelemetrySupportedAttributeValueTypes =
[
typeof(string),
typeof(bool),
typeof(byte),
Expand All @@ -24,7 +24,7 @@ internal sealed class SpanShim : ISpan
typeof(long),
typeof(float),
typeof(double),
};
];

private readonly SpanContextShim spanContextShim;

Expand Down
8 changes: 1 addition & 7 deletions src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,7 @@ public TracerShim(Trace.TracerProvider tracerProvider, TextMapPropagator? textFo
/// <inheritdoc/>
public global::OpenTracing.ISpan? ActiveSpan => this.ScopeManager.Active?.Span;

private TextMapPropagator Propagator
{
get
{
return this.definedPropagator ?? Propagators.DefaultTextMapPropagator;
}
}
private TextMapPropagator Propagator => this.definedPropagator ?? Propagators.DefaultTextMapPropagator;

/// <inheritdoc/>
public global::OpenTracing.ISpanBuilder BuildSpan(string operationName)
Expand Down
6 changes: 5 additions & 1 deletion src/Shared/AssemblyVersionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ private static string ParsePackageVersion(string informationalVersion)
// The following parts are optional: pre-release label, pre-release version, git height, Git SHA of current commit
// For package version, value of AssemblyInformationalVersionAttribute without commit hash is returned.

var indexOfPlusSign = informationalVersion!.IndexOf('+');
#if NET
var indexOfPlusSign = informationalVersion.IndexOf('+', StringComparison.Ordinal);
#else
var indexOfPlusSign = informationalVersion.IndexOf('+');
#endif
return indexOfPlusSign > 0
? informationalVersion.Substring(0, indexOfPlusSign)
: informationalVersion;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void WithActivities(
b => b.AddSource(ChildActivitySource))
.Build();

ITracer otTracer = new TracerShim(
var otTracer = new TracerShim(
tracerProvider,
Propagators.DefaultTextMapPropagator);

Expand Down Expand Up @@ -99,7 +99,7 @@ public void WithActivities(
}
}

private class TestSampler : Sampler
private sealed class TestSampler : Sampler
{
private readonly Func<SamplingParameters, SamplingDecision> shouldSampleDelegate;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,35 +1,11 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using System.Diagnostics;
using Xunit;

namespace OpenTelemetry.Shims.OpenTracing.Tests;

[CollectionDefinition(nameof(ListenAndSampleAllActivitySources))]
public sealed class ListenAndSampleAllActivitySources : ICollectionFixture<ListenAndSampleAllActivitySources.Fixture>
{
public sealed class Fixture : IDisposable
{
private readonly ActivityListener listener;

public Fixture()
{
Activity.DefaultIdFormat = ActivityIdFormat.W3C;
Activity.ForceDefaultIdFormat = true;

this.listener = new ActivityListener
{
ShouldListenTo = _ => true,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllDataAndRecorded,
};

ActivitySource.AddActivityListener(this.listener);
}

public void Dispose()
{
this.listener.Dispose();
}
}
}
#pragma warning disable CA1515
public sealed class ListenAndSampleAllActivitySources : ICollectionFixture<ListenAndSampleAllActivitySourcesFixture>;
#pragma warning restore CA1515
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using System.Diagnostics;

namespace OpenTelemetry.Shims.OpenTracing.Tests;

#pragma warning disable CA1515
public sealed class ListenAndSampleAllActivitySourcesFixture : IDisposable
#pragma warning restore CA1515
{
private readonly ActivityListener listener;

public ListenAndSampleAllActivitySourcesFixture()
{
Activity.DefaultIdFormat = ActivityIdFormat.W3C;
Activity.ForceDefaultIdFormat = true;

this.listener = new ActivityListener
{
ShouldListenTo = _ => true,
Sample = (ref ActivityCreationOptions<ActivityContext> options) => ActivitySamplingResult.AllDataAndRecorded,
};

ActivitySource.AddActivityListener(this.listener);
}

public void Dispose()
{
this.listener.Dispose();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<Description>Unit test project for OpenTelemetry.Shims.OpenTracing</Description>
<TargetFrameworks>$(TargetFrameworksForTests)</TargetFrameworks>
<DefineConstants Condition="'$(RunningDotNetPack)' != 'true'">$(DefineConstants);BUILDING_USING_PROJECTS</DefineConstants>
<AnalysisLevel>latest-all</AnalysisLevel>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ public void AsChildOf_WithSpan()
public void Start_ActivityOperationRootSpanChecks()
{
// Create an activity
using var activity = new Activity("foo")
.SetIdFormat(ActivityIdFormat.W3C)
using var activity = new Activity("foo");
activity.SetIdFormat(ActivityIdFormat.W3C)
.Start();

// matching root operation name
Expand Down Expand Up @@ -132,7 +132,7 @@ public void AsChildOf_MultipleCallsWithSpan()

Assert.NotNull(spanShim.Span.Activity);
Assert.Equal("foo", spanShim.Span.Activity.OperationName);
Assert.Contains(spanShim.Context.TraceId, spanShim.Span.Activity.TraceId.ToHexString());
Assert.Contains(spanShim.Context.TraceId, spanShim.Span.Activity.TraceId.ToHexString(), StringComparison.Ordinal);

// TODO: Check for multi level parenting
}
Expand Down Expand Up @@ -191,7 +191,7 @@ public void AsChildOf_MultipleCallsWithSpanContext()
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.Contains(spanContext1.TraceId, spanShim.Span.Activity.ParentId, StringComparison.Ordinal);
Assert.Equal(spanContext2.SpanId, spanShim.Span.Activity.Links.First().Context.SpanId.ToHexString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@

namespace OpenTelemetry.Shims.OpenTracing.Tests;

internal class TestFormatTextMap : IFormat<ITextMap>
internal sealed class TestFormatTextMap : IFormat<ITextMap>
{
}
2 changes: 1 addition & 1 deletion test/OpenTelemetry.Shims.OpenTracing.Tests/TestSpan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace OpenTelemetry.Shims.OpenTracing.Tests;

internal class TestSpan : ISpan
internal sealed class TestSpan : ISpan
{
public ISpanContext Context => throw new NotImplementedException();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace OpenTelemetry.Shims.OpenTracing.Tests;

internal class TestSpanContext : ISpanContext
internal sealed class TestSpanContext : ISpanContext
{
public string TraceId => throw new NotImplementedException();

Expand Down
2 changes: 1 addition & 1 deletion test/OpenTelemetry.Shims.OpenTracing.Tests/TestTextMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace OpenTelemetry.Shims.OpenTracing.Tests;

internal class TestTextMap : ITextMap
internal sealed class TestTextMap : ITextMap
{
public bool GetEnumeratorCalled { get; private set; }

Expand Down
22 changes: 2 additions & 20 deletions test/OpenTelemetry.Shims.OpenTracing.Tests/TracerShimTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void InjectExtract_TextMap_Ok()
AssertOpenTracerSpanContextEqual(spanContextShim, extractedSpanContext!);
}

private static void AssertOpenTracerSpanContextEqual(ISpanContext source, ISpanContext target)
private static void AssertOpenTracerSpanContextEqual(SpanContextShim source, ISpanContext target)
{
Assert.Equal(source.TraceId, target.TraceId);
Assert.Equal(source.SpanId, target.SpanId);
Expand All @@ -144,7 +144,7 @@ private static void AssertOpenTracerSpanContextEqual(ISpanContext source, ISpanC
/// Simple ITextMap implementation used for the inject/extract tests.
/// </summary>
/// <seealso cref="OpenTracing.Propagation.ITextMap" />
private class TextMapCarrier : ITextMap
private sealed class TextMapCarrier : ITextMap
{
private readonly Dictionary<string, string> map = new();

Expand All @@ -159,22 +159,4 @@ public void Set(string key, string value)

IEnumerator IEnumerable.GetEnumerator() => this.map.GetEnumerator();
}

/// <summary>
/// Simple IBinary implementation used for the inject/extract tests.
/// </summary>
/// <seealso cref="OpenTracing.Propagation.IBinary" />
private class BinaryCarrier : IBinary
{
private readonly MemoryStream carrierStream = new();

public MemoryStream Get() => this.carrierStream;

public void Set(MemoryStream stream)
{
this.carrierStream.SetLength(stream.Length);
this.carrierStream.Seek(0, SeekOrigin.Begin);
stream.CopyTo(this.carrierStream, (int)this.carrierStream.Length);
}
}
}

0 comments on commit c977f73

Please # to comment.