Apm-agent-dotnet: Dependencies in Elastic.Apm

[gregkalapos]

Since the Elastic.Apm project is the core of the agent it can end up basically in any types of application on any types of framework that we support. Therefore we should be super careful about what other packages/assemblies this package depends on.

We already had suggestions from the community to add fancy logging libraries and I expect some other ideas in the future. All those are valid ideas, but one of the main design goals regarding this package should be portability (also beyond just .NET Standard 2.0). If we lock ourself into a specific framework or into a relatively high framework version then it can block our productivity and limit the supported frameworks/platforms heavily.

Therefore:

• There should be a documentation describing this and each time someone suggests to add a new dependency to Elastic.Apm then we should just check if against or design goals and make the decision based on that. (hint: probably most of the suggestions to add a dependency will be declined...)

• We should revisit the existing dependencies and think about solutions.

• [ ] Json.NET (the elasticsearch client already solves this, we should reuse this solution)

• [ ] System.Threading.Tasks.Dataflow (caused problems with our full framework sample on @SergeyKleyman`s setup, we should check if this is ok)
• [ ] System.Diagnostics.DiagnosticSource

[Mpdreamz]

We use an internalized and renamespaced Json.NET in 6.x of the client but we are actually moving to a source included Utf8Json in 7.x.

We choice Utf8Json because out of the box it supports quite a bit that we need in the client, custom type (de)serialization routines being the most important. Also since the client is quite often instantiated several times with different configuration we can not depend on a static serializer that does not carry over a per serialization context.

For APM it might make sense to source include Utf8Json/SpanJson or even already roll with: https://github.com/dotnet/corefx/blob/master/src/System.Text.Json/source_package/README.md

Since the number of actual DTO's is small and their serialization is known Json.NET seems overkill.

Especially if we limit the amount of custom objects that can be attached (pending the custom data discussion in APM) shipping with optimized serializers (even handwritten) starts to make a lot sense.

Re: data flow, i will look into this once the PR is merged. DataFlow has been out since the .NET 3.5 days so should run fine on full framework. I think we might hit a NuGet bug @SergeyKleyman mind sharing your exact setup (VS edition etc).

[tomasfreund]

Please use System.Text.Json, Utf8Json is not maintained, does not produce reasonable exceptions on deserialization failure and also has issues with not clearing shared buffers (leading to potential vulnerabilities - not exactly sure if that applies in this case).
https://github.com/neuecc/Utf8Json/issues/154

[Mpdreamz]

Most likely we will move to System.Text.Json @tomasfreund because its maintained.

In the .NET Elaticsearch client we depend on a fork of Utf8Json we maintain that tackles many of the outstanding issues including https://github.com/neuecc/Utf8Json/issues/154 see https://github.com/nullean/Utf8Json/commit/429e0611cb0fd30245c042ea6d13566c68b75b95

But the decision to do so there predates System.Text.Json 😄

[tomap]

Any news on this move? removal of newtonsoft & switch to System.Text.Json ?

[gregkalapos]

https://github.com/elastic/apm-agent-dotnet/pull/1241 internalized Newtonsoft.Json - so the next agent version won't depend on it anymore.

So far that was the one causing the most pain and that's solve, closing this now.