Skip to content

Completing the wrong span should be impossible #34

Description

@carterkozak

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.

Activity

added this to the Major API Revision milestone on Nov 1, 2018

markelliot commented on Nov 1, 2018

@markelliot
Contributor

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

carterkozak commented on Nov 1, 2018

@carterkozak
ContributorAuthor

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 commented on Jan 15, 2019

@berler

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions