Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Instrumentation.Wcf] Add .NET6 as target framework #2243

Merged
merged 10 commits into from
Oct 28, 2024
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Instrumentation.Wcf/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* Add target for `net6.0`.
([#2243](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2243))
Kielek marked this conversation as resolved.
Show resolved Hide resolved

## 1.0.0-rc.17

Released 2024-Jun-18
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static string Name
public static object CreateNew()
{
AssertHttpEnabled();
return Activator.CreateInstance(ReflectedValues!.Type);
return Activator.CreateInstance(ReflectedValues!.Type)!;
}

public static WebHeaderCollection GetHeaders(object httpRequestMessageProperty)
Expand All @@ -49,23 +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)!;

var headersProp = type.GetProperty("Headers", BindingFlags.Public | BindingFlags.Instance, null, typeof(WebHeaderCollection), Array.Empty<Type>(), null);
if (headersProp == null)
{
throw new NotSupportedException("HttpRequestMessageProperty.Headers property 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<Type>(), null)
?? throw new NotSupportedException("HttpRequestMessageProperty.Headers property not found");

var nameProp = type.GetProperty("Name", BindingFlags.Public | BindingFlags.Static, null, typeof(string), Array.Empty<Type>(), null)
?? throw new NotSupportedException("HttpRequestMessageProperty.Name property not found");

var nameProp = type.GetProperty("Name", BindingFlags.Public | BindingFlags.Static, null, typeof(string), Array.Empty<Type>(), 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<WebHeaderCollection>("Headers"));
}
catch (Exception ex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,20 +178,27 @@ private IAsyncResult SendInternal(Message message, TimeSpan timeout, Func<AsyncC
private void SendInternal(Message message, TimeSpan timeout, Action<RequestTelemetryState> 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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the part I'm not sure about. I'm going to try and find an expert on the .NET runtime team to ask about what to do here. I'll post back.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking for any advice you can give : )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't nailed it down yet but I approved the PR. We're adding a throw here for null EC but ExecutionContext.Run should be throwing today in that case anyway and no user has complained about it AFAIK.

We can circle back to it if any new info comes to light.

ExecutionContext.Run(executionContext, ExecuteInChildContext, null);
}
catch (Exception)
{
ClientChannelInstrumentation.AfterRequestCompleted(null, telemetryState!);
ClientChannelInstrumentation.AfterRequestCompleted(null, telemetryState);
throw;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,21 @@ Message IRequestChannel.EndRequest(IAsyncResult result)
private IAsyncResult InternalBeginRequest(Message message, Func<AsyncCallback, object?, IAsyncResult> 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!;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ internal static class RequestTelemetryStateTracker
{
private static readonly Dictionary<string, Entry> OutstandingRequestStates = new Dictionary<string, Entry>();
private static readonly SortedSet<EntryTimeoutProperties> TimeoutQueue = new SortedSet<EntryTimeoutProperties>();
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<RequestTelemetryState> timeoutCallback)
Expand Down Expand Up @@ -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<Tuple<EntryTimeoutProperties, Entry>>? timedOutEntries = null;
lock (Sync)
Expand Down Expand Up @@ -139,9 +139,13 @@ public EntryTimeoutProperties(string messageId, long expiresAt, Action<RequestTe
this.TimeoutCallback = timeoutCallback;
}

public int CompareTo(object obj)
public int CompareTo(object? obj)
{
var other = (EntryTimeoutProperties)obj;
if (obj is not EntryTimeoutProperties other)
{
return 1;
}

var result = this.ExpiresAt.CompareTo(other.ExpiresAt);
if (result == 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ public static void HttpRequestHeaders(Message request, string name, string value
return;
}

object prop;
if (!request.Properties.TryGetValue(HttpRequestMessagePropertyWrapper.Name, out prop))
if (!request.Properties.TryGetValue(HttpRequestMessagePropertyWrapper.Name, out var prop))
{
prop = HttpRequestMessagePropertyWrapper.CreateNew();
request.Properties.Add(HttpRequestMessagePropertyWrapper.Name, prop);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal static class WcfInstrumentationActivitySource
{
internal static readonly Assembly Assembly = typeof(WcfInstrumentationActivitySource).Assembly;
internal static readonly AssemblyName AssemblyName = Assembly.GetName();
internal static readonly string ActivitySourceName = AssemblyName.Name;
internal static readonly string ActivitySourceName = AssemblyName.Name!;
internal static readonly string IncomingRequestActivityName = ActivitySourceName + ".IncomingRequest";
internal static readonly string OutgoingRequestActivityName = ActivitySourceName + ".OutgoingRequest";

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

<PropertyGroup>
<!-- OmniSharp/VS Code requires TargetFrameworks to be in descending order for IntelliSense and analysis. -->
<TargetFrameworks>$(NetStandardMinimumSupportedVersion);$(NetFrameworkMinimumSupportedVersion)</TargetFrameworks>
<TargetFrameworks>net6.0;$(NetStandardMinimumSupportedVersion);$(NetFrameworkMinimumSupportedVersion)</TargetFrameworks>
CodeBlanch marked this conversation as resolved.
Show resolved Hide resolved
<Description>OpenTelemetry instrumentation for WCF.</Description>
<PackageTags>$(PackageTags);distributed-tracing;WCF</PackageTags>
<MinVerTagPrefix>Instrumentation.Wcf-</MinVerTagPrefix>
Expand All @@ -23,9 +23,10 @@
<Reference Include="System.ServiceModel" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == '$(NetStandardMinimumSupportedVersion)'">
<ItemGroup Condition="'$(TargetFramework)' == '$(NetStandardMinimumSupportedVersion)' OR '$(TargetFramework)' == 'net6.0' " >
<PackageReference Include="System.Security.Cryptography.Xml" Version="4.7.1" />
<PackageReference Include="System.ServiceModel.Primitives" Version="4.7.0" />
<PackageReference Include="System.Drawing.Common" Version="4.7.2" Condition=" '$(TargetFramework)' == 'net6.0' " />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
<ItemGroup Condition="'$(TargetFramework)' != '$(NetFrameworkMinimumSupportedVersion)'">
<PackageReference Include="System.ServiceModel.Http" Version="4.7.0" />
<PackageReference Include="System.ServiceModel.NetTcp" Version="4.7.0" />
<!-- System.Drawing.Common is indirect reference. It is needed to upgrade it directly to avoid https://github.com/advisories/GHSA-rxg9-xrhp-64gj -->
<PackageReference Include="System.Drawing.Common" Version="4.7.3" />
</ItemGroup>

<ItemGroup>
Expand Down
Loading