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

Change AgentTracer.activeScope() to return AgentScope #2902

Merged
merged 1 commit into from
Nov 10, 2021

Conversation

mcculls
Copy link
Contributor

@mcculls mcculls commented Jul 8, 2021

This changes a number of internal APIs and some non-public types in dd-trace-ot

It also updates a test to check that internal scopes are handled correctly when a custom scope manager is used.

@mcculls mcculls added the tag: breaking change Breaking changes label Jul 8, 2021
@mcculls mcculls requested a review from a team as a code owner July 8, 2021 12:37
@mcculls mcculls marked this pull request as draft November 5, 2021 11:13
@mcculls mcculls force-pushed the mcculls/returnAgentScope branch 5 times, most recently from 0e23627 to 820d7f2 Compare November 8, 2021 12:31
@mcculls mcculls marked this pull request as ready for review November 8, 2021 13:28
@mcculls mcculls added comp: core Tracer core type: refactoring and removed tag: breaking change Breaking changes labels Nov 8, 2021
@mcculls mcculls merged commit 86b8f63 into master Nov 10, 2021
@mcculls mcculls deleted the mcculls/returnAgentScope branch November 10, 2021 11:05
@github-actions github-actions bot added this to the 0.91.0 milestone Nov 10, 2021
} else {
return new OTScopeManager.OTScope((AgentScope) scope, finishSpanOnClose, this);
return new CustomScope(scope);
Copy link
Contributor

Choose a reason for hiding this comment

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

@mcculls I'm wondering what the reason for this change was?


@Override
public AgentSpan span() {
return toAgentSpan(delegate.span());
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately adding this class makes dd-trace-ot fail compile time checks for Graalvm because of this call when using opentracing 0.33, because Scope.span() was removed. Even if this code can never actually be executed

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.

None yet

5 participants