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

Completing the wrong span should be impossible #34

Open
carterkozak opened this issue Nov 1, 2018 · 3 comments
Open

Completing the wrong span should be impossible #34

carterkozak opened this issue Nov 1, 2018 · 3 comments
Labels
enhancement New feature or request

Comments

@carterkozak
Copy link
Contributor

Proposal: Opening a span returns a token used to safely close that span, it should not be possible to close the wrong span using this API.

@carterkozak carterkozak added the enhancement New feature or request label Nov 1, 2018
@carterkozak carterkozak added this to the Major API Revision milestone Nov 1, 2018
@markelliot
Copy link
Contributor

I think this is likely overkill and would prefer we keep the library simpler than to cover this edge case.

@carterkozak
Copy link
Contributor Author

This would prevent palantir/tritium#131 and #28

The simple implementation of this would return an object similar to the existing CloseableTracer from startSpan methods. The API would be arguably simpler, though I could buy that the cost of changing the api may outweigh the benefits of the proposal.

@berler
Copy link

berler commented Jan 15, 2019

The bugs that @cakofony linked to I would argue are a direct consequence of the design of the tracing API (specifically not specifying the span you intend to close but instead relying on the thread local state to be correct at the time you request to complete the span). I also disagree that using "tokens" adds any considerable complexity. The "token" can be as simple as a pointer to the OpenSpan.

The current API is incredibly easy to misuse and inadvertently drop traces or cause wrong timings to be recorded. To be confident that your trace logs are accurate you need to audit your code as well as every library you depend on to make sure that they are guaranteed to complete their spans (e.g. using finally blocks), never call completeSpan when they did not create a span, and don't incorrectly clear the state or init a new trace when you were already in the middle of a trace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants