From 11a8b0aa7eeeae73ff5226e28dda303a09d565c4 Mon Sep 17 00:00:00 2001 From: martincostello Date: Fri, 3 Feb 2023 15:45:24 +0000 Subject: [PATCH] [Instrumentation.Quartz] Fix analysis warnings Fix/suppress code analysis warnings in OpenTelemetry.Instrumentation.Quartz. Contributes to #950. --- .../Implementation/QuartzDiagnosticListener.cs | 14 +++++++------- .../OperationName.cs | 2 ++ .../QuartzInstrumentationOptions.cs | 2 ++ .../TraceProviderBuilderExtensions.cs | 3 +++ .../QuartzDiagnosticListenerTests.cs | 8 ++++---- 5 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Quartz/Implementation/QuartzDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.Quartz/Implementation/QuartzDiagnosticListener.cs index 9208c22bad..6e38ee2f7f 100644 --- a/src/OpenTelemetry.Instrumentation.Quartz/Implementation/QuartzDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.Quartz/Implementation/QuartzDiagnosticListener.cs @@ -53,10 +53,10 @@ public override void OnStartActivity(Activity activity, object payload) return; } - activity.DisplayName = this.GetDisplayName(activity); + activity.DisplayName = GetDisplayName(activity); ActivityInstrumentationHelper.SetActivitySourceProperty(activity, ActivitySource); - ActivityInstrumentationHelper.SetKindProperty(activity, this.GetActivityKind(activity)); + ActivityInstrumentationHelper.SetKindProperty(activity, GetActivityKind(activity)); try { @@ -115,17 +115,17 @@ public override void OnException(Activity activity, object payload) } } - private string GetDisplayName(Activity activity) + private static string GetDisplayName(Activity activity) { return activity.OperationName switch { - OperationName.Job.Execute => $"execute {this.GetTag(activity.Tags, TagName.JobName)}", - OperationName.Job.Veto => $"veto {this.GetTag(activity.Tags, TagName.JobName)}", + OperationName.Job.Execute => $"execute {GetTag(activity.Tags, TagName.JobName)}", + OperationName.Job.Veto => $"veto {GetTag(activity.Tags, TagName.JobName)}", _ => activity.DisplayName, }; } - private ActivityKind GetActivityKind(Activity activity) + private static ActivityKind GetActivityKind(Activity activity) { return activity.OperationName switch { @@ -135,7 +135,7 @@ private ActivityKind GetActivityKind(Activity activity) }; } - private string GetTag(IEnumerable> tags, string tagName) + private static string GetTag(IEnumerable> tags, string tagName) { var tag = tags.SingleOrDefault(kv => kv.Key == tagName); return tag.Value; diff --git a/src/OpenTelemetry.Instrumentation.Quartz/OperationName.cs b/src/OpenTelemetry.Instrumentation.Quartz/OperationName.cs index b173e14041..4a1e6ed492 100644 --- a/src/OpenTelemetry.Instrumentation.Quartz/OperationName.cs +++ b/src/OpenTelemetry.Instrumentation.Quartz/OperationName.cs @@ -24,7 +24,9 @@ public static class OperationName /// /// Quartz Job category constants. /// +#pragma warning disable CA1034 // Nested types should not be visible public static class Job +#pragma warning restore CA1034 // Nested types should not be visible { /// /// Quartz job execute diagnostic source operation name. diff --git a/src/OpenTelemetry.Instrumentation.Quartz/QuartzInstrumentationOptions.cs b/src/OpenTelemetry.Instrumentation.Quartz/QuartzInstrumentationOptions.cs index ac9572ef54..4e726f3f72 100644 --- a/src/OpenTelemetry.Instrumentation.Quartz/QuartzInstrumentationOptions.cs +++ b/src/OpenTelemetry.Instrumentation.Quartz/QuartzInstrumentationOptions.cs @@ -56,5 +56,7 @@ public class QuartzInstrumentationOptions /// /// Gets or sets traced operations set. /// +#pragma warning disable CA2227 // Collection properties should be read only public HashSet TracedOperations { get; set; } = new(DefaultTracedOperations); +#pragma warning restore CA2227 // Collection properties should be read only } diff --git a/src/OpenTelemetry.Instrumentation.Quartz/TraceProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.Quartz/TraceProviderBuilderExtensions.cs index dd84eb51f4..9461dcc585 100644 --- a/src/OpenTelemetry.Instrumentation.Quartz/TraceProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.Quartz/TraceProviderBuilderExtensions.cs @@ -17,6 +17,7 @@ using System; using OpenTelemetry.Instrumentation.Quartz; using OpenTelemetry.Instrumentation.Quartz.Implementation; +using OpenTelemetry.Internal; // ReSharper disable once CheckNamespace namespace OpenTelemetry.Trace; @@ -44,6 +45,8 @@ public static TracerProviderBuilder AddQuartzInstrumentation( this TracerProviderBuilder builder, Action configure) { + Guard.ThrowIfNull(builder); + var options = new QuartzInstrumentationOptions(); configure?.Invoke(options); diff --git a/test/OpenTelemetry.Instrumentation.Quartz.Tests/QuartzDiagnosticListenerTests.cs b/test/OpenTelemetry.Instrumentation.Quartz.Tests/QuartzDiagnosticListenerTests.cs index b5f98b7d07..10a3e9ff76 100644 --- a/test/OpenTelemetry.Instrumentation.Quartz.Tests/QuartzDiagnosticListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.Quartz.Tests/QuartzDiagnosticListenerTests.cs @@ -106,9 +106,9 @@ public async Task Should_Create_Activity_And_Enrich_When_Enrich() if (payload is IJobDetail jobDetail) { var dataMap = jobDetail.JobDataMap; - if (dataMap.ContainsKey("TestId")) + if (dataMap.TryGetValue("TestId", out var value)) { - a.SetTag("test.id", dataMap["TestId"]); + a.SetTag("test.id", value); } } }) @@ -229,9 +229,9 @@ public async Task Should_Enrich_Exception_When_Record_Exception_Enabled_And_Enri if (p is IJobDetail jobDetail) { var dataMap = jobDetail.JobDataMap; - if (dataMap.ContainsKey("TestId")) + if (dataMap.TryGetValue("TestId", out var value)) { - a.SetTag("test.id", dataMap["TestId"]); + a.SetTag("test.id", value); } } };