From dda61234916471c7e2980b5231c07a199da1aec9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Wed, 23 Oct 2024 16:43:27 +0200 Subject: [PATCH 1/7] [Instrumentation.Wcf] Add .NET6 as target framework --- .../OpenTelemetry.Instrumentation.Wcf.csproj | 5 +++-- .../OpenTelemetry.Instrumentation.Wcf.Tests.csproj | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Wcf/OpenTelemetry.Instrumentation.Wcf.csproj b/src/OpenTelemetry.Instrumentation.Wcf/OpenTelemetry.Instrumentation.Wcf.csproj index b9558189a8..8724466ada 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/OpenTelemetry.Instrumentation.Wcf.csproj +++ b/src/OpenTelemetry.Instrumentation.Wcf/OpenTelemetry.Instrumentation.Wcf.csproj @@ -2,7 +2,7 @@ - $(NetStandardMinimumSupportedVersion);$(NetFrameworkMinimumSupportedVersion) + net6.0;$(NetStandardMinimumSupportedVersion);$(NetFrameworkMinimumSupportedVersion) OpenTelemetry instrumentation for WCF. $(PackageTags);distributed-tracing;WCF Instrumentation.Wcf- @@ -23,9 +23,10 @@ - + + diff --git a/test/OpenTelemetry.Instrumentation.Wcf.Tests/OpenTelemetry.Instrumentation.Wcf.Tests.csproj b/test/OpenTelemetry.Instrumentation.Wcf.Tests/OpenTelemetry.Instrumentation.Wcf.Tests.csproj index 8343f2c6ae..04e89ab423 100644 --- a/test/OpenTelemetry.Instrumentation.Wcf.Tests/OpenTelemetry.Instrumentation.Wcf.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.Wcf.Tests/OpenTelemetry.Instrumentation.Wcf.Tests.csproj @@ -20,10 +20,12 @@ - - - - + + + + + + From 55ec7a93aa42d3f3985d5716eae69589f22b508f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Wed, 23 Oct 2024 16:44:15 +0200 Subject: [PATCH 2/7] CHANGELOG --- src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md index e3080ccd88..8b9114f677 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* Add target for `net6.0`. + ([#2243](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2243)) + ## 1.0.0-rc.17 Released 2024-Jun-18 From 6dea5d6eec06279f085501c947e319bcbff40d2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Wed, 23 Oct 2024 17:29:58 +0200 Subject: [PATCH 3/7] nullable fixes --- .../AsyncResultWithTelemetryState.cs | 2 +- .../ClientChannelInstrumentation.cs | 4 ++-- .../HttpRequestMessagePropertyWrapper.cs | 19 ++++++++++++------- .../InstrumentedDuplexChannel.cs | 15 +++++++++++---- .../InstrumentedRequestChannel.cs | 13 ++++++++++--- .../RequestTelemetryStateTracker.cs | 13 +++++++++---- .../TelemetryPropagationWriter.cs | 3 +-- .../WcfInstrumentationActivitySource.cs | 2 +- 8 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/AsyncResultWithTelemetryState.cs b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/AsyncResultWithTelemetryState.cs index 7d50404090..cf26181c7c 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/AsyncResultWithTelemetryState.cs +++ b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/AsyncResultWithTelemetryState.cs @@ -15,7 +15,7 @@ public AsyncResultWithTelemetryState(IAsyncResult inner, RequestTelemetryState t public RequestTelemetryState TelemetryState { get; } - object IAsyncResult.AsyncState => this.Inner.AsyncState; + object? IAsyncResult.AsyncState => this.Inner.AsyncState; WaitHandle IAsyncResult.AsyncWaitHandle => this.Inner.AsyncWaitHandle; diff --git a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/ClientChannelInstrumentation.cs b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/ClientChannelInstrumentation.cs index ecba99793d..6fca3efada 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/ClientChannelInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/ClientChannelInstrumentation.cs @@ -82,7 +82,7 @@ public static RequestTelemetryState BeforeSendRequest(Message request, Uri? remo }; } - public static void AfterRequestCompleted(Message? reply, RequestTelemetryState state) + public static void AfterRequestCompleted(Message? reply, RequestTelemetryState? state) { Guard.ThrowIfNull(state); @@ -128,7 +128,7 @@ private static ActionMetadata GetActionMetadata(Message request, string action) if (request.Properties.TryGetValue(TelemetryContextMessageProperty.Name, out object telemetryContextProperty)) { var actionMappings = (telemetryContextProperty as TelemetryContextMessageProperty)?.ActionMappings; - if (actionMappings != null && actionMappings.TryGetValue(action, out ActionMetadata metadata)) + if (actionMappings != null && actionMappings.TryGetValue(action, out var metadata)) { actionMetadata = metadata; } diff --git a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/HttpRequestMessagePropertyWrapper.cs b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/HttpRequestMessagePropertyWrapper.cs index 9864302715..ad524a4cae 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/HttpRequestMessagePropertyWrapper.cs +++ b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/HttpRequestMessagePropertyWrapper.cs @@ -20,7 +20,7 @@ internal static class HttpRequestMessagePropertyWrapper public static bool IsHttpFunctionalityEnabled => ReflectedValues != null; - public static string Name + public static string? Name { get { @@ -29,13 +29,13 @@ public static string Name } } - public static object CreateNew() + public static object? CreateNew() { AssertHttpEnabled(); return Activator.CreateInstance(ReflectedValues!.Type); } - public static WebHeaderCollection GetHeaders(object httpRequestMessageProperty) + public static WebHeaderCollection GetHeaders(object? httpRequestMessageProperty) { AssertHttpEnabled(); AssertIsFrameworkMessageProperty(httpRequestMessageProperty); @@ -51,6 +51,11 @@ public static WebHeaderCollection GetHeaders(object httpRequestMessageProperty) "System.ServiceModel.Channels.HttpRequestMessageProperty, System.ServiceModel, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", true); + if (type == null) + { + throw new NotSupportedException("HttpRequestMessageProperty type not found"); + } + var headersProp = type.GetProperty("Headers", BindingFlags.Public | BindingFlags.Instance, null, typeof(WebHeaderCollection), Array.Empty(), null); if (headersProp == null) { @@ -65,7 +70,7 @@ public static WebHeaderCollection GetHeaders(object httpRequestMessageProperty) return new ReflectedInfo( type: type, - name: (string)nameProp.GetValue(null), + name: (string?)nameProp.GetValue(null), headersFetcher: new PropertyFetcher("Headers")); } catch (Exception ex) @@ -86,7 +91,7 @@ private static void AssertHttpEnabled() } [Conditional("DEBUG")] - private static void AssertIsFrameworkMessageProperty(object httpRequestMessageProperty) + private static void AssertIsFrameworkMessageProperty(object? httpRequestMessageProperty) { AssertHttpEnabled(); if (httpRequestMessageProperty == null || !httpRequestMessageProperty.GetType().Equals(ReflectedValues!.Type)) @@ -98,10 +103,10 @@ private static void AssertIsFrameworkMessageProperty(object httpRequestMessagePr private sealed class ReflectedInfo { public Type Type; - public string Name; + public string? Name; public PropertyFetcher HeadersFetcher; - public ReflectedInfo(Type type, string name, PropertyFetcher headersFetcher) + public ReflectedInfo(Type type, string? name, PropertyFetcher headersFetcher) { this.Type = type; this.Name = name; diff --git a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/InstrumentedDuplexChannel.cs b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/InstrumentedDuplexChannel.cs index ef58ab4ef8..d58157d836 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/InstrumentedDuplexChannel.cs +++ b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/InstrumentedDuplexChannel.cs @@ -178,20 +178,27 @@ private IAsyncResult SendInternal(Message message, TimeSpan timeout, Func executeSend) { RequestTelemetryState? telemetryState = null; - ContextCallback executeInChildContext = _ => + + void ExecuteInChildContext(object? unused) { telemetryState = ClientChannelInstrumentation.BeforeSendRequest(message, this.RemoteAddress?.Uri); RequestTelemetryStateTracker.PushTelemetryState(message, telemetryState, timeout, OnTelemetryStateTimedOut); executeSend(telemetryState); - }; + } + + var executionContext = ExecutionContext.Capture(); + if (executionContext == null) + { + throw new InvalidOperationException("Cannot fetch execution context"); + } try { - ExecutionContext.Run(ExecutionContext.Capture(), executeInChildContext, null); + ExecutionContext.Run(executionContext, ExecuteInChildContext, null); } catch (Exception) { - ClientChannelInstrumentation.AfterRequestCompleted(null, telemetryState!); + ClientChannelInstrumentation.AfterRequestCompleted(null, telemetryState); throw; } } diff --git a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/InstrumentedRequestChannel.cs b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/InstrumentedRequestChannel.cs index 7c6caa3fce..30c245cc91 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/InstrumentedRequestChannel.cs +++ b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/InstrumentedRequestChannel.cs @@ -99,14 +99,21 @@ Message IRequestChannel.EndRequest(IAsyncResult result) private IAsyncResult InternalBeginRequest(Message message, Func beginRequestDelegate, AsyncCallback callback, object? state) { IAsyncResult? result = null; - ContextCallback executeInChildContext = _ => + + void ExecuteInChildContext(object? unused) { var telemetryState = ClientChannelInstrumentation.BeforeSendRequest(message, ((IRequestChannel)this).RemoteAddress?.Uri); var asyncCallback = AsyncResultWithTelemetryState.GetAsyncCallback(callback, telemetryState); result = new AsyncResultWithTelemetryState(beginRequestDelegate(asyncCallback, state), telemetryState); - }; + } + + var executionContext = ExecutionContext.Capture(); + if (executionContext == null) + { + throw new InvalidOperationException("Cannot fetch execution context"); + } - ExecutionContext.Run(ExecutionContext.Capture(), executeInChildContext, null); + ExecutionContext.Run(executionContext, ExecuteInChildContext, null); return result!; } } diff --git a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/RequestTelemetryStateTracker.cs b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/RequestTelemetryStateTracker.cs index 874d7b3d36..a7bda4db97 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/RequestTelemetryStateTracker.cs +++ b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/RequestTelemetryStateTracker.cs @@ -9,8 +9,8 @@ internal static class RequestTelemetryStateTracker { private static readonly Dictionary OutstandingRequestStates = new Dictionary(); private static readonly SortedSet TimeoutQueue = new SortedSet(); - private static readonly object Sync = new object(); - private static readonly Timer Timer = new Timer(OnTimer); + private static readonly object Sync = new(); + private static readonly Timer Timer = new(OnTimer); private static long currentTimerDueAt = Timeout.Infinite; public static void PushTelemetryState(Message request, RequestTelemetryState telemetryState, TimeSpan timeout, Action timeoutCallback) @@ -50,7 +50,7 @@ public static void PushTelemetryState(Message request, RequestTelemetryState tel } } - private static void OnTimer(object state) + private static void OnTimer(object? state) { List>? timedOutEntries = null; lock (Sync) @@ -139,8 +139,13 @@ public EntryTimeoutProperties(string messageId, long expiresAt, Action Date: Wed, 23 Oct 2024 17:42:24 +0200 Subject: [PATCH 4/7] revert unwanted changes --- .../OpenTelemetry.Instrumentation.Wcf.Tests.csproj | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.Wcf.Tests/OpenTelemetry.Instrumentation.Wcf.Tests.csproj b/test/OpenTelemetry.Instrumentation.Wcf.Tests/OpenTelemetry.Instrumentation.Wcf.Tests.csproj index 04e89ab423..64ee92e89b 100644 --- a/test/OpenTelemetry.Instrumentation.Wcf.Tests/OpenTelemetry.Instrumentation.Wcf.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.Wcf.Tests/OpenTelemetry.Instrumentation.Wcf.Tests.csproj @@ -20,12 +20,8 @@ - - - - - - + + From 80258e3baf9f7cc2e6627328e512a065ae04fb27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Thu, 24 Oct 2024 07:36:35 +0200 Subject: [PATCH 5/7] PR feedback - Comparable Co-authored-by: Mikel Blanchard --- .../Implementation/RequestTelemetryStateTracker.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/RequestTelemetryStateTracker.cs b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/RequestTelemetryStateTracker.cs index a7bda4db97..282f0c91e0 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/RequestTelemetryStateTracker.cs +++ b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/RequestTelemetryStateTracker.cs @@ -141,12 +141,11 @@ public EntryTimeoutProperties(string messageId, long expiresAt, Action Date: Thu, 24 Oct 2024 07:39:47 +0200 Subject: [PATCH 6/7] PR feedback - nullable for HttpRequestMessagePropertyWrapper Co-authored-by: Mikel Blanchard --- .../HttpRequestMessagePropertyWrapper.cs | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/HttpRequestMessagePropertyWrapper.cs b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/HttpRequestMessagePropertyWrapper.cs index ad524a4cae..26e62a054a 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/Implementation/HttpRequestMessagePropertyWrapper.cs +++ b/src/OpenTelemetry.Instrumentation.Wcf/Implementation/HttpRequestMessagePropertyWrapper.cs @@ -20,7 +20,7 @@ internal static class HttpRequestMessagePropertyWrapper public static bool IsHttpFunctionalityEnabled => ReflectedValues != null; - public static string? Name + public static string Name { get { @@ -29,13 +29,13 @@ public static string? Name } } - public static object? CreateNew() + public static object CreateNew() { AssertHttpEnabled(); - return Activator.CreateInstance(ReflectedValues!.Type); + return Activator.CreateInstance(ReflectedValues!.Type)!; } - public static WebHeaderCollection GetHeaders(object? httpRequestMessageProperty) + public static WebHeaderCollection GetHeaders(object httpRequestMessageProperty) { AssertHttpEnabled(); AssertIsFrameworkMessageProperty(httpRequestMessageProperty); @@ -49,28 +49,25 @@ public static WebHeaderCollection GetHeaders(object? httpRequestMessageProperty) { type = Type.GetType( "System.ServiceModel.Channels.HttpRequestMessageProperty, System.ServiceModel, Version=0.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089", - true); + true)!; - if (type == null) - { - throw new NotSupportedException("HttpRequestMessageProperty type not found"); - } + var constructor = type.GetConstructor(Type.EmptyTypes) + ?? throw new NotSupportedException("HttpRequestMessageProperty public parameterless constructor was not found"); - var headersProp = type.GetProperty("Headers", BindingFlags.Public | BindingFlags.Instance, null, typeof(WebHeaderCollection), Array.Empty(), null); - if (headersProp == null) - { - throw new NotSupportedException("HttpRequestMessageProperty.Headers property not found"); - } + var headersProp = type.GetProperty("Headers", BindingFlags.Public | BindingFlags.Instance, null, typeof(WebHeaderCollection), Array.Empty(), null) + ?? throw new NotSupportedException("HttpRequestMessageProperty.Headers property not found"); + + var nameProp = type.GetProperty("Name", BindingFlags.Public | BindingFlags.Static, null, typeof(string), Array.Empty(), null) + ?? throw new NotSupportedException("HttpRequestMessageProperty.Name property not found"); - var nameProp = type.GetProperty("Name", BindingFlags.Public | BindingFlags.Static, null, typeof(string), Array.Empty(), null); - if (nameProp == null) + if (nameProp.GetValue(null) is not string name) { - throw new NotSupportedException("HttpRequestMessageProperty.Name property not found"); + throw new NotSupportedException("HttpRequestMessageProperty.Name property was null"); } return new ReflectedInfo( type: type, - name: (string?)nameProp.GetValue(null), + name: name, headersFetcher: new PropertyFetcher("Headers")); } catch (Exception ex) @@ -91,7 +88,7 @@ private static void AssertHttpEnabled() } [Conditional("DEBUG")] - private static void AssertIsFrameworkMessageProperty(object? httpRequestMessageProperty) + private static void AssertIsFrameworkMessageProperty(object httpRequestMessageProperty) { AssertHttpEnabled(); if (httpRequestMessageProperty == null || !httpRequestMessageProperty.GetType().Equals(ReflectedValues!.Type)) @@ -103,10 +100,10 @@ private static void AssertIsFrameworkMessageProperty(object? httpRequestMessageP private sealed class ReflectedInfo { public Type Type; - public string? Name; + public string Name; public PropertyFetcher HeadersFetcher; - public ReflectedInfo(Type type, string? name, PropertyFetcher headersFetcher) + public ReflectedInfo(Type type, string name, PropertyFetcher headersFetcher) { this.Type = type; this.Name = name; From 3545cf14e7fd9d28f6cb1a8e5db991942629308a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Fri, 25 Oct 2024 20:24:28 +0200 Subject: [PATCH 7/7] Update src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md --- src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md index 8b9114f677..745314baa5 100644 --- a/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md @@ -2,7 +2,8 @@ ## Unreleased -* Add target for `net6.0`. +* Add target for `net6.0` to ensure that non-vulnerable transient + dependencies are referenced by default for .NET6+. ([#2243](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2243)) ## 1.0.0-rc.17