Skip to content

Minor ScopeManager cleanup#1462

Merged
tylerbenson merged 3 commits into
masterfrom
tyler/remove-simplescope
May 15, 2020
Merged

Minor ScopeManager cleanup#1462
tylerbenson merged 3 commits into
masterfrom
tyler/remove-simplescope

Conversation

@tylerbenson
Copy link
Copy Markdown
Contributor

I wanted to do more, but was limited by the fact that we allow access to the current scope.

@tylerbenson tylerbenson requested a review from a team as a code owner May 14, 2020 21:08
@tylerbenson tylerbenson force-pushed the tyler/remove-simplescope branch from f4f3f3c to 16936cd Compare May 14, 2020 21:38
Copy link
Copy Markdown
Contributor

@richardstartin richardstartin left a comment

Choose a reason for hiding this comment

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

this escapes in Continuation which is a dangerous practice in multithreaded settings.

Comment thread dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScope.java Outdated
Comment thread dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java
Comment thread dd-trace-core/src/main/java/datadog/trace/core/PendingTrace.java
@tylerbenson tylerbenson dismissed richardstartin’s stale review May 15, 2020 16:43

I think escaping of instances out of the constructor is something that should be dealt with, but let's handle it in a separate PR as it seems independent from the changes here.

@tylerbenson
Copy link
Copy Markdown
Contributor Author

@richardstartin the change you suggested was smaller than I though, so I went ahead and made the change here. Sorry about that.

@tylerbenson tylerbenson merged commit 4c6153c into master May 15, 2020
@tylerbenson tylerbenson deleted the tyler/remove-simplescope branch May 15, 2020 18:12
@github-actions github-actions Bot added this to the 0.52.0 milestone May 15, 2020
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

Successfully merging this pull request may close these issues.

2 participants