opentelemetry-dotnet-instrumentation
opentelemetry-dotnet-instrumentation copied to clipboard
open-telemetry
Minimize usage of singletons
Why
Overuse of singleton like here makes the code very tightly coupled and almost impossible to unit test.
What
Refactor the code to get rid of as many singletons as possible. Where applicable it would be good to use dependency injection (e.g. via constructor). It would be also good to minimize the number of constructors. Each type should ideally have one constructor or two if a second is needed just for sake of testing internals.
For example classes like WebRequest_GetResponseAsync_Integration could have a constructor accepting a tracer and a second parametress which injects the Tracer.Instance. The tracer can be then passed when invoking WebRequestCommon.GetResponse_OnMethodBegin(). However, we would also need to make sure that OnMethodBegin and OnMethodEnd are NOT static (and instrumentation is still working).
Discovered a issue with Tracer.Instance singleton that creates a new tracer when private instance field is unset.
A lot of tests are using pattern new Tracer(...) which does not ensure that global Tracer.Instance is created, so the test can create 2 instances of a tracer (1 intentional and 1 unintentional due internal Tracer.Instance reference).
