census-instrumentation / opencensus-csharp

Distributed tracing and stats collecting framework
https://opencensus.io
Apache License 2.0
139 stars 32 forks source link

Closing spans in diagnostic listeners #126

Closed jacostro closed 5 years ago

jacostro commented 5 years ago

Hi,

I'm recently implementing OpenCensus in the CMS-type application our company is developing. Beside using listeners for incoming and outgoing HTTP connections, provided in OpenCensus.Collector.AspNetCore and OpenCensus.Collector.Dependencies packages, I've reused the same pattern for DiagnosticSources our code is instrumented with.

I have some doubts with regards to my understanding of the way in which collectors/listeners plug into span hierarchy. In these two files:

https://github.com/census-instrumentation/opencensus-csharp/blob/develop/src/OpenCensus.Collector.AspNetCore/Implementation/HttpInListener.cs https://githhub.com/census-instrumentation/opencensus-csharp/blob/develop/src/OpenCensus.Collector.Dependencies/Implementation/HttpHandlerDiagnosticListener.cs

OnStopActivity handler closes active span by calling its End method. This however does not change Tracer.CurrentSpan, and therefore next started span becomes a child span of the closed one, instead of a following. Current span is correctly reset when IScope instance produced by SpanBuilder.StartScopedSpan is disposed (which I did in my own listener, adding an async local stack of scopes).

Is here some thought I don't understand or mechanism I don't see? I can understand that for incoming HTTP requests resetting Tracer.CurrentSpan may be not important, because they are top level in a (local) trace, but for outgoing connections correct setting of CurrentSpan seems to me important.

Cheers, Jacek

SergeyKanzhelev commented 5 years ago

clearly a bug. Perhaps scope object needs to be closed instead of span. Need to investigate

SergeyKanzhelev commented 5 years ago

Moved to OpenTelemetry