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

[Breaking change for dd-trace-api] Deprecate scope finishOnClose and continuation close #1424

Merged
merged 1 commit into from
May 5, 2020

Conversation

tylerbenson
Copy link
Contributor

@tylerbenson tylerbenson commented May 1, 2020

Note: These deprecated methods will be removed in the following release.

The finishOnClose functionality will remain in the OpenTracing compatibility layer, but behavior may change if utilizing on continuations.

Migrating usage can impact the way span duration is calculated. For example given the following code:

@Trace
def someMethod() {
  executor.execute {
    // Assume this executes after someMethod() already returned.
    httpClient.request(url)
  }
}

Historically, the Java Tracer prevented the top level span from finishing until the child work finished, resulting in a trace like this:

[-------------- someMethod --------------------]
                       [---clientRequest-----]

After this change, it will produce a trace more like this, which more accurately depicts the duration of the method execution.

[---someMethod---]   (likely even shorter)
                       [----clientRequest---]

Since trace duration is based on the parent/top level span, this change can impact the trace duration calculation.

@tylerbenson tylerbenson requested a review from a team as a code owner May 1, 2020 20:28
@tylerbenson tylerbenson added the tag: breaking change Breaking changes label May 1, 2020
@tylerbenson tylerbenson added this to the 0.51.0 milestone May 1, 2020
@tylerbenson tylerbenson force-pushed the tyler/deprecate-finishOnClose branch 2 times, most recently from 32c5add to b823c72 Compare May 1, 2020 21:17
These deprecated methods will be removed in the following release.
@tylerbenson tylerbenson force-pushed the tyler/deprecate-finishOnClose branch from b823c72 to cf8bea6 Compare May 4, 2020 17:37
@devinsba
Copy link
Contributor

devinsba commented May 4, 2020

Not related to the change, but maybe the title of the PR should state the module (dd-trace-api) that this is a breaking change for

@tylerbenson tylerbenson force-pushed the tyler/deprecate-finishOnClose branch from cf8bea6 to dcc1763 Compare May 4, 2020 19:12
@tylerbenson tylerbenson changed the title [Breaking change] Deprecate scope finishOnClose and continuation close [Breaking change for dd-trace-api] Deprecate scope finishOnClose and continuation close May 4, 2020
@tylerbenson tylerbenson force-pushed the tyler/deprecate-finishOnClose branch from bb6b76a to c4958fb Compare May 5, 2020 16:43
@tylerbenson
Copy link
Contributor Author

I'm pulling out all the changes to internal instrumentation into a separate PR.

@tylerbenson tylerbenson merged commit 4d2d55f into master May 5, 2020
@tylerbenson tylerbenson deleted the tyler/deprecate-finishOnClose branch May 5, 2020 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: breaking change Breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants