Skip to content

Conversation

@dsd987
Copy link
Contributor

@dsd987 dsd987 commented Jan 10, 2019

Previously, when Tracers::wrapWithNewTrace was called with no trace state, the trace state would incorrectly be restored to a new trace rather than the cleared state. Now, the trace state will be appropriately returned to the cleared state.

@dsd987 dsd987 requested a review from a team as a code owner January 10, 2019 18:27
@dsd987
Copy link
Contributor Author

dsd987 commented Jan 10, 2019

Proposed fix for #51

return Preconditions.checkNotNull(currentTrace.get(), "There is no root span").getTraceId();
}

/** Clears the current trace id and returns (a copy of) it if present */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this makes a copy

@dsd987 dsd987 force-pushed the dd/wrapWithNewTraceRestore branch 2 times, most recently from fda93bc to 72ba144 Compare January 10, 2019 22:26
}

/** Clears the current trace id and returns it if present. */
static Optional<Trace> getAndClearTraceIfPresent() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like if this was public, but we can look into that in a separate PR!

@dsd987 dsd987 force-pushed the dd/wrapWithNewTraceRestore branch from 72ba144 to 63b6ce5 Compare January 10, 2019 22:41
Copy link
Contributor

@carterkozak carterkozak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bulldozer-bot bulldozer-bot bot merged commit bd813da into palantir:develop Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants