From 1397add634ce8995c666149d44fc28889b9ae3a1 Mon Sep 17 00:00:00 2001 From: martincostello Date: Fri, 3 Feb 2023 15:32:06 +0000 Subject: [PATCH 1/2] [Instrumentation.AspNet] Fix analysis warnings Fix/suppress code analysis warnings in OpenTelemetry.Instrumentation.AspNet. Contributes to #950. --- .../TelemetryHttpModule.cs | 3 +++ .../MeterProviderBuilderExtensions.cs | 2 ++ .../ActivityHelperTest.cs | 6 +++--- .../HttpContextHelper.cs | 8 ++++---- .../WebConfigTransformTest.cs | 9 +++++---- .../WebConfigWithLocationTagTransformTest.cs | 9 +++++---- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs index 62a45ef49d..2b9455bca1 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule/TelemetryHttpModule.cs @@ -18,6 +18,7 @@ using System.Diagnostics; using System.Reflection; using System.Web; +using OpenTelemetry.Internal; namespace OpenTelemetry.Instrumentation.AspNet; @@ -57,6 +58,8 @@ public void Dispose() /// public void Init(HttpApplication context) { + Guard.ThrowIfNull(context); + context.BeginRequest += this.Application_BeginRequest; context.EndRequest += this.Application_EndRequest; context.Error += this.Application_Error; diff --git a/src/OpenTelemetry.Instrumentation.AspNet/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNet/MeterProviderBuilderExtensions.cs index ee69839171..48c23efd61 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/MeterProviderBuilderExtensions.cs @@ -34,7 +34,9 @@ public static MeterProviderBuilder AddAspNetInstrumentation( { Guard.ThrowIfNull(builder); +#pragma warning disable CA2000 // Dispose objects before losing scope var instrumentation = new AspNetMetrics(); +#pragma warning restore CA2000 // Dispose objects before losing scope builder.AddMeter(AspNetMetrics.InstrumentationName); return builder.AddInstrumentation(() => instrumentation); } diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs index 130614390c..f491020cd4 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/ActivityHelperTest.cs @@ -429,7 +429,9 @@ public void Can_Create_RootActivity_And_Saved_In_HttContext() } [Fact] +#pragma warning disable CA1030 // Use events where appropriate public void Fire_Exception_Events() +#pragma warning restore CA1030 // Use events where appropriate { int callbacksFired = 0; @@ -529,13 +531,11 @@ public TestHttpContext(Exception error = null) private class NoopTextMapPropagator : TextMapPropagator { - private static readonly PropagationContext DefaultPropagationContext = default; - public override ISet Fields => null; public override PropagationContext Extract(PropagationContext context, T carrier, Func> getter) { - return DefaultPropagationContext; + return default; } public override void Inject(PropagationContext context, T carrier, Action setter) diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/HttpContextHelper.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/HttpContextHelper.cs index dd1864609b..b39a654aa9 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/HttpContextHelper.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/HttpContextHelper.cs @@ -79,9 +79,9 @@ public override string[][] GetUnknownRequestHeaders() public override string GetUnknownRequestHeader(string name) { - if (this.headers.ContainsKey(name)) + if (this.headers.TryGetValue(name, out var value)) { - return this.headers[name]; + return value; } return base.GetUnknownRequestHeader(name); @@ -91,9 +91,9 @@ public override string GetKnownRequestHeader(int index) { var name = GetKnownRequestHeaderName(index); - if (this.headers.ContainsKey(name)) + if (this.headers.TryGetValue(name, out var value)) { - return this.headers[name]; + return value; } return base.GetKnownRequestHeader(index); diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/WebConfigTransformTest.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/WebConfigTransformTest.cs index a232bb7bb4..e64c93868d 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/WebConfigTransformTest.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/WebConfigTransformTest.cs @@ -391,16 +391,17 @@ private XDocument ApplyTransformation(string originalConfiguration, string trans var document = new XmlTransformableDocument(); using var transformation = new XmlTransformation(stream, null); stream = null; +#pragma warning disable CA3075 // Insecure DTD processing in XML document.LoadXml(originalConfiguration); +#pragma warning restore CA3075 // Insecure DTD processing in XML transformation.Apply(document); result = XDocument.Parse(document.OuterXml); } finally { - if (stream != null) - { - stream.Dispose(); - } +#pragma warning disable CA1508 // Avoid dead conditional code + stream?.Dispose(); +#pragma warning restore CA1508 // Avoid dead conditional code } return result; diff --git a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/WebConfigWithLocationTagTransformTest.cs b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/WebConfigWithLocationTagTransformTest.cs index 0e84dbb24b..7e39fc8ee4 100644 --- a/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/WebConfigWithLocationTagTransformTest.cs +++ b/test/OpenTelemetry.Instrumentation.AspNet.TelemetryHttpModule.Tests/WebConfigWithLocationTagTransformTest.cs @@ -421,16 +421,17 @@ private XDocument ApplyTransformation(string originalConfiguration, string trans var document = new XmlTransformableDocument(); using var transformation = new XmlTransformation(stream, null); stream = null; +#pragma warning disable CA3075 // Insecure DTD processing in XML document.LoadXml(originalConfiguration); +#pragma warning restore CA3075 // Insecure DTD processing in XML transformation.Apply(document); result = XDocument.Parse(document.OuterXml); } finally { - if (stream != null) - { - stream.Dispose(); - } +#pragma warning disable CA1508 // Avoid dead conditional code + stream?.Dispose(); +#pragma warning restore CA1508 // Avoid dead conditional code } return result; From f3864e366d6b6b1a0f36b737b6ec5a30c0113196 Mon Sep 17 00:00:00 2001 From: martincostello Date: Mon, 6 Feb 2023 15:01:11 +0000 Subject: [PATCH 2/2] Lazily create AspNetMetrics Create `AspNetMetrics` in the func to remove code analysis warning. --- .../MeterProviderBuilderExtensions.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNet/MeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.AspNet/MeterProviderBuilderExtensions.cs index 48c23efd61..2b2b6cf3cc 100644 --- a/src/OpenTelemetry.Instrumentation.AspNet/MeterProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.AspNet/MeterProviderBuilderExtensions.cs @@ -34,10 +34,7 @@ public static MeterProviderBuilder AddAspNetInstrumentation( { Guard.ThrowIfNull(builder); -#pragma warning disable CA2000 // Dispose objects before losing scope - var instrumentation = new AspNetMetrics(); -#pragma warning restore CA2000 // Dispose objects before losing scope builder.AddMeter(AspNetMetrics.InstrumentationName); - return builder.AddInstrumentation(() => instrumentation); + return builder.AddInstrumentation(() => new AspNetMetrics()); } }