From c977f73246b3fc1daa4a24224e7299f87481a4a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Tue, 11 Mar 2025 22:55:03 +0100 Subject: [PATCH] [Shims.OpenTracing] enable code analysis (#6160) Co-authored-by: Rajkumar Rangaraj --- .../OpenTelemetry.Shims.OpenTracing.csproj | 1 + .../SpanBuilderShim.cs | 9 ++---- .../SpanShim.cs | 6 ++-- .../TracerShim.cs | 8 +---- src/Shared/AssemblyVersionExtensions.cs | 6 +++- .../IntegrationTests.cs | 4 +-- .../ListenAndSampleAllActivitySources.cs | 30 ++--------------- ...istenAndSampleAllActivitySourcesFixture.cs | 32 +++++++++++++++++++ ...enTelemetry.Shims.OpenTracing.Tests.csproj | 1 + .../SpanBuilderShimTests.cs | 8 ++--- .../TestFormatTextMap.cs | 2 +- .../TestSpan.cs | 2 +- .../TestSpanContext.cs | 2 +- .../TestTextMap.cs | 2 +- .../TracerShimTests.cs | 22 ++----------- 15 files changed, 61 insertions(+), 74 deletions(-) create mode 100644 test/OpenTelemetry.Shims.OpenTracing.Tests/ListenAndSampleAllActivitySourcesFixture.cs diff --git a/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj b/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj index 127cbfb3aa9..b25add35f36 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj +++ b/src/OpenTelemetry.Shims.OpenTracing/OpenTelemetry.Shims.OpenTracing.csproj @@ -4,6 +4,7 @@ OpenTracing shim for OpenTelemetry .NET $(PackageTags);distributed-tracing;OpenTracing coreunstable- + latest-all diff --git a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs index f95b1b77be3..8f256920127 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs +++ b/src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs @@ -27,7 +27,7 @@ internal sealed class SpanBuilderShim : ISpanBuilder /// /// The OpenTelemetry links. These correspond loosely to OpenTracing references. /// - private readonly List links = new(); + private readonly List links = []; /// /// The OpenTelemetry attributes. These correspond to OpenTracing Tags. @@ -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; @@ -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) { diff --git a/src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs b/src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs index c1bde927a1b..f8a5d981207 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs +++ b/src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs @@ -14,8 +14,8 @@ internal sealed class SpanShim : ISpan /// public const string DefaultEventName = "log"; - private static readonly IReadOnlyCollection OpenTelemetrySupportedAttributeValueTypes = new List - { + private static readonly IReadOnlyCollection OpenTelemetrySupportedAttributeValueTypes = + [ typeof(string), typeof(bool), typeof(byte), @@ -24,7 +24,7 @@ internal sealed class SpanShim : ISpan typeof(long), typeof(float), typeof(double), - }; + ]; private readonly SpanContextShim spanContextShim; diff --git a/src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs b/src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs index cbdbd9dfc76..b67dfd9a82a 100644 --- a/src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs +++ b/src/OpenTelemetry.Shims.OpenTracing/TracerShim.cs @@ -48,13 +48,7 @@ public TracerShim(Trace.TracerProvider tracerProvider, TextMapPropagator? textFo /// 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; /// public global::OpenTracing.ISpanBuilder BuildSpan(string operationName) diff --git a/src/Shared/AssemblyVersionExtensions.cs b/src/Shared/AssemblyVersionExtensions.cs index 67926d99c03..cd830a33187 100644 --- a/src/Shared/AssemblyVersionExtensions.cs +++ b/src/Shared/AssemblyVersionExtensions.cs @@ -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; diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/IntegrationTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/IntegrationTests.cs index cb592e8906e..a5ad1d33244 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/IntegrationTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/IntegrationTests.cs @@ -55,7 +55,7 @@ public void WithActivities( b => b.AddSource(ChildActivitySource)) .Build(); - ITracer otTracer = new TracerShim( + var otTracer = new TracerShim( tracerProvider, Propagators.DefaultTextMapPropagator); @@ -99,7 +99,7 @@ public void WithActivities( } } - private class TestSampler : Sampler + private sealed class TestSampler : Sampler { private readonly Func shouldSampleDelegate; diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/ListenAndSampleAllActivitySources.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/ListenAndSampleAllActivitySources.cs index 9cc17e9e59f..a4df1603801 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/ListenAndSampleAllActivitySources.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/ListenAndSampleAllActivitySources.cs @@ -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 -{ - 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 options) => ActivitySamplingResult.AllDataAndRecorded, - }; - - ActivitySource.AddActivityListener(this.listener); - } - - public void Dispose() - { - this.listener.Dispose(); - } - } -} +#pragma warning disable CA1515 +public sealed class ListenAndSampleAllActivitySources : ICollectionFixture; +#pragma warning restore CA1515 diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/ListenAndSampleAllActivitySourcesFixture.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/ListenAndSampleAllActivitySourcesFixture.cs new file mode 100644 index 00000000000..ee99e2c5ec2 --- /dev/null +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/ListenAndSampleAllActivitySourcesFixture.cs @@ -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 options) => ActivitySamplingResult.AllDataAndRecorded, + }; + + ActivitySource.AddActivityListener(this.listener); + } + + public void Dispose() + { + this.listener.Dispose(); + } +} 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 d21c157d1b9..fc89c6ebae9 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/OpenTelemetry.Shims.OpenTracing.Tests.csproj @@ -4,6 +4,7 @@ Unit test project for OpenTelemetry.Shims.OpenTracing $(TargetFrameworksForTests) $(DefineConstants);BUILDING_USING_PROJECTS + latest-all diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs index b99bcb2f890..5e8b2adc011 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/SpanBuilderShimTests.cs @@ -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 @@ -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 } @@ -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()); } diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/TestFormatTextMap.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/TestFormatTextMap.cs index 83d04095313..c1f7c7e3d08 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/TestFormatTextMap.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/TestFormatTextMap.cs @@ -5,6 +5,6 @@ namespace OpenTelemetry.Shims.OpenTracing.Tests; -internal class TestFormatTextMap : IFormat +internal sealed class TestFormatTextMap : IFormat { } diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/TestSpan.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/TestSpan.cs index f1e836f76b7..8cfb51c457a 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/TestSpan.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/TestSpan.cs @@ -6,7 +6,7 @@ namespace OpenTelemetry.Shims.OpenTracing.Tests; -internal class TestSpan : ISpan +internal sealed class TestSpan : ISpan { public ISpanContext Context => throw new NotImplementedException(); diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/TestSpanContext.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/TestSpanContext.cs index d0b5af69991..80f207e4a94 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/TestSpanContext.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/TestSpanContext.cs @@ -5,7 +5,7 @@ namespace OpenTelemetry.Shims.OpenTracing.Tests; -internal class TestSpanContext : ISpanContext +internal sealed class TestSpanContext : ISpanContext { public string TraceId => throw new NotImplementedException(); diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/TestTextMap.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/TestTextMap.cs index 7396b5543e6..39e5893fbe3 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/TestTextMap.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/TestTextMap.cs @@ -6,7 +6,7 @@ namespace OpenTelemetry.Shims.OpenTracing.Tests; -internal class TestTextMap : ITextMap +internal sealed class TestTextMap : ITextMap { public bool GetEnumeratorCalled { get; private set; } diff --git a/test/OpenTelemetry.Shims.OpenTracing.Tests/TracerShimTests.cs b/test/OpenTelemetry.Shims.OpenTracing.Tests/TracerShimTests.cs index 64f08959424..0fb2c90b1ba 100644 --- a/test/OpenTelemetry.Shims.OpenTracing.Tests/TracerShimTests.cs +++ b/test/OpenTelemetry.Shims.OpenTracing.Tests/TracerShimTests.cs @@ -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); @@ -144,7 +144,7 @@ private static void AssertOpenTracerSpanContextEqual(ISpanContext source, ISpanC /// Simple ITextMap implementation used for the inject/extract tests. /// /// - private class TextMapCarrier : ITextMap + private sealed class TextMapCarrier : ITextMap { private readonly Dictionary map = new(); @@ -159,22 +159,4 @@ public void Set(string key, string value) IEnumerator IEnumerable.GetEnumerator() => this.map.GetEnumerator(); } - - /// - /// Simple IBinary implementation used for the inject/extract tests. - /// - /// - 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); - } - } }