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

Example/test for catching Span close when using ScopeManager.Active #3

Closed
ndrwrbgs opened this issue Aug 30, 2018 · 8 comments
Closed

Comments

@ndrwrbgs
Copy link
Contributor

This was a problem I hit when trying to implement a similar library and from what I can see I believe we may have it here also (I’ll update for sure later).

If the user wants to decorate span closes, eg to emit metrics, but accesses the span from IScopeManager.Active, at that hook point we cannot know if the IScope returned should close its contained span or not (and the IScope returned from the impl manager won’t be our decorator type).

@ndrwrbgs
Copy link
Contributor Author

Related to #2, because at the current level of abstraction we just expose the call to IScopeManager.Active, and not the SpanClosed event. Though potentially support for #2 since direct consumers are unlikel to realize this until later in development.

@jrouaix
Copy link
Collaborator

jrouaix commented Aug 30, 2018

work in progress in this test : https://github.com/opentracing-contrib/csharp-decorators/blob/dev/test/OpenTracing.Contrib.Decorators.Tests/Examples/SpanCounterExample.cs

For example I have to figure out how to propagate the information that the scope won't finish the span on dispose (if needed)

@ndrwrbgs
Copy link
Contributor Author

I requested a change to the OT spec due to the impossibility of doing this, but the only ways I think you can do are

  1. Accept a delegate that allows the user to tell you how to cast the ISpan to a concrete type and interpret if it's going to close or not
  2. Keep track of the context yourself (this is the approach I chose for ease) - e.g. use the AsyncContext like in the base library to implement this.

opentracing/opentracing-csharp#95 relates to this item

@jrouaix
Copy link
Collaborator

jrouaix commented Sep 17, 2018

@ndrwrbgs I finally dropped to enable any method decoration, but based on the example provided by @cwe1ss in opentracing/opentracing-csharp#95, i finally had all span/scope close/finish/dispose scenarios working

@ndrwrbgs
Copy link
Contributor Author

Be sure to test with an implementation that expects the types to match it's implementation type (e.g. Jaeger I believe does this, based on recommendations they gave me).

E.g. I don't think the comment here is valid for implementations

// Now we activate our span and not the wrapped one
// We don't have to pass it the wrapped span as IScopeManager is a completely separate
// concept that just happens to sit on ITracer (for easier accessibility).
IScope scope = impl.Activate(span, finishSpanOnDispose);

@jrouaix
Copy link
Collaborator

jrouaix commented Sep 18, 2018

image
Seems ok to me :-)

@ndrwrbgs
Copy link
Contributor Author

ooo, nice that they didn't do what they recommended me to do (which was really bad design :-( ). I'd still want to ensure if possible that a tracer that expects it's own type will get it back, but as it won't affect anyone other than me it seems I can take on looking into that later :)

@ndrwrbgs
Copy link
Contributor Author

ndrwrbgs commented Apr 1, 2019

Closed out thanks!

@ndrwrbgs ndrwrbgs closed this as completed Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants