Wcf instrumentation + example apps

[CodeBlanch]

Uses IClientMessageInspector & IDispatchMessageInspector to add instrumentation for WCF clients and servers.

/cc @mbakalov

TODOs

• Error logging.
• Unit tests.
• Documentation.

[codecov]

Codecov Report

Merging #38 (828752d) into main (9c11735) will increase coverage by 0.44%.
The diff coverage is 76.61%.

@@            Coverage Diff             @@
##             main      #38      +/-   ##
==========================================
+ Coverage   72.71%   73.16%   +0.44%     
==========================================
  Files          34       41       +7     
  Lines         953     1077     +124     
==========================================
+ Hits          693      788      +95     
- Misses        260      289      +29     

Impacted Files	Coverage Δ
...cf/Implementation/WcfInstrumentationEventSource.cs	21.42% <21.42%> (ø)
...rumentation.Wcf/TracerProviderBuilderExtensions.cs	75.00% <75.00%> (ø)
...rumentation.Wcf/TelemetryClientMessageInspector.cs	80.30% <80.30%> (ø)
...b.Instrumentation.Wcf/WcfInstrumentationOptions.cs	88.88% <88.88%> (ø)
...b.Instrumentation.Wcf/TelemetryEndpointBehavior.cs	91.66% <91.66%> (ø)
...Implementation/WcfInstrumentationActivitySource.cs	92.30% <92.30%> (ø)
...strumentation.Wcf/Implementation/ActionMetadata.cs	100.00% <100.00%> (ø)
... and 4 more

[alexvaluyskiy]

Will this instrumentation library support WCF client for .NET Core?
https://github.com/dotnet/wcf

[CodeBlanch]

@alexvaluyskiy Not sure. I just poked around the repo, I think it could also work for the .NET Core version. I'll see if I can get it working when I find some cycles.

[CodeBlanch]

@alexvaluyskiy I just pushed support for .NET Core WCF clients. There's no way to build a service client from config yet (that I could tell) but you can add endpoint behavior manually like this:

client.Endpoint.EndpointBehaviors.Add(new TelemetryEndpointBehavior());

There's an example in there of how to do it.

/cc @mconnew

[mconnew]

I'm not sure if I'm about to make you really happy or a little bit sad. We shipped in .NET Framework 4.8 support in WCF for really strong integration for telemetry. Another team were meant to create the piece for emitting the telemetry data but they never followed through. I created DiagnosticSource events in all the key areas where you would want to create telemetry information, many of them in places which are inaccessible via extensibility points. Because this was dropped by the other team so there was nothing to consume the diagnostics data I never bothered to port it to the open source WCF Core client.
You can see this support if you check the source code here: https://github.com/microsoft/referencesource/blob/master/System.ServiceModel/System/ServiceModel/Dispatcher/ImmutableDispatchRuntime.cs#L248

I can't remember how much I put in to the client side. I think I added the code to pick up the standard HTTP diagnostics headers and made it flow correctly. If you want to hook into this to get really rich data let me know and I'll dig out the commit diff so you have an exhaustive list of all the places I added things. If you want to integrate with what I added to .NET Framework, I would happily port the changes to the WCF client. I also have an unreviewed internal PR with some initial integration done which I could make available.

[CodeBlanch]

@mconnew Cool thanks for the info. Right now I'm registering an IClientMessageInspector for .NET Framework & .NET Core which starts & stops Activity objects using the new .NET 5 ActivitySource API. ActivitySource is what the OpenTelemetry .NET engine is wired up to receive (we have a way to translate DiagnosticSources). For .NET Framework server-side, I'm registering an IDispatchMessageInspector and doing the same. I went with that approach over DiagnosticSource because I didn't know the latter existed until I was done 😄 But it sounds like I got lucky and that is the only option available for pre-4.8 .NET Framework & .NET Core? The one benefit to using DiagnosticSource that I can see would be we could trap those automatically, user wouldn't need to mess with their service/client config. But pre-4.8 support is very nice to have, so I'm not sure which option is best. Might need both?

As far as the level of detail, for OpenTelemetry tracing, traping start/end is probably fine. But OpenTelemetry also has a metric side to it. To provide really good metrics having more raw data would probably be useful. It's an area we're just getting into with the library & specification though so might want to wait a bit before spending a bunch of time on it?

[mconnew]

@CodeBlanch, the places I added the ability to emit events were intended to answer the question "Why has my service got slower", so I've added hooks into anything where the request can get delayed along the line. So for example, the throttling code emits multiple events. If a request gets throttled, I actually add a message property with the data so a later extensibility point could pick it up and emit the values. A property is added at the point of throttling, and another when the throttle is released. That way you can see if your P99 request time has gone up because some of your requests are getting delayed due to throttling. Also every extensibility point such as message inspectors installed also emit an event with how long it took to execute it.

It's been 2 years since I wrote that code so just had a look at it and I didn't actually use DiagnosticSource as it wasn't available in the framework. What I actually did was create a custom EventSource and emitted events using that. I had some other code (which I've just started the process of getting approval to release as sample code to help flesh out this implementation) which created an EventSourceListener and emitted the data using DiagnosticSource. It could be easily changed to use ActivitySource. I also wrote some channel layer classes (a ChannelListener and ChannelFactory) which did all the work of making the relevant id's flow through a client to the server via either HTTP headers if using HTTP or a custom SOAP header. It was all done via a single service behavior which hooked in and injected itself in all the required places so all you needed to do was add a single service behavior and everything else gets wired up for you. What's missing is defining exactly what data you want to be emitting such as all the tag's.

I'm not suggesting this work should be replaced by the sample code I'm getting approval to release, it's way too out of date for the current design of telemetry as I know it is quite different now than it was 2 years ago. The intention is as a reference implementation on how to collect all this data using already written helper code that exists in the framework and an example how to do end 2 end telemetry between the client and the server with correlated data.

[CodeBlanch]

Thanks @mconnew! When you get the sample up please ping back here and I'll check it out!

[meastp]

Is there any progress on this? I'd love to be able to depend on a nuget package instead of copy+paste source code :)

[SergeyKanzhelev]

@CodeBlanch is it ready to be merged? Is there any other outstanding comments?

[CodeBlanch]

@SergeyKanzhelev There are a couple of open questions for @mconnew still on here, but nothing blocking that I'm aware of. I think it's a good starting point, we can always continue to improve it 😄

[SergeyKanzhelev]

@SergeyKanzhelev There are a couple of open questions for @mconnew still on here, but nothing blocking that I'm aware of. I think it's a good starting point, we can always continue to improve it 😄

I suggest we merge it than and track open questions as issues. Merging, can you please create issues?