Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

V0.32.0 README update #332

Merged

Conversation

carlosalberto
Copy link
Collaborator

The README is now updated with the new Scope/Span usage, as well as mention of deprecation of automatic Span finish upon Scope close (and related members).

This is done prior to do a release (finally) ;)

@coveralls
Copy link

coveralls commented Mar 1, 2019

Coverage Status

Coverage remained the same at 77.301% when pulling e3306ae on carlosalberto:v0.32.0_documentation_update into 8176208 on opentracing:v0.32.0.

@carlosalberto
Copy link
Collaborator Author

Any objection to merging this one @yurishkuro ? ;)

README.md Outdated
// cannot record the exception in the span since scope is not accessible and span is finished
}
```

**If there is a `Scope`, it will act as the parent to any newly started `Span`** unless
Copy link
Member

Choose a reason for hiding this comment

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

If there is already an active Span

README.md Outdated
2. In the closure/`Runnable`/`Future`/etc itself, invoke `tracer.scopeManager().activate(span, false)` to re-activate the `Span` and get a new `Scope`, then `deactivate()` it when the `Span` is no longer active (or use try-with-resources for less typing).
3. In the closure/`Runnable`/`Future`/etc where the end of the task is reached, invoke `tracer.scopeManager().activate(span, true)` to re-activate the `Span` and have the new `Scope` close the `Span` automatically.
1. Start a `Span` via `start`.
2. In the closure/`Runnable`/`Future`/etc itself, invoke `tracer.scopeManager().activate(span)` to re-activate the `Span` and get a new `Scope`, then `deactivate()` it when the `Span` is no longer active (or use try-with-resources for less typing).
Copy link
Member

Choose a reason for hiding this comment

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

s/In the/At the beginning of/

README.md Outdated
3. In the closure/`Runnable`/`Future`/etc where the end of the task is reached, invoke `tracer.scopeManager().activate(span, true)` to re-activate the `Span` and have the new `Scope` close the `Span` automatically.
1. Start a `Span` via `start`.
2. In the closure/`Runnable`/`Future`/etc itself, invoke `tracer.scopeManager().activate(span)` to re-activate the `Span` and get a new `Scope`, then `deactivate()` it when the `Span` is no longer active (or use try-with-resources for less typing).
3. In the closure/`Runnable`/`Future`/etc where the end of the task is reached, invoke `tracer.scopeManager().activate(span)` to re-activate the `Span` and invoke `span.finish()` when the work is done.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure there's a reason to re-activate the span here, just finishing it is fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That makes sense. I'm guessing users will know they can do 2) (reactivation) anytime they need it, including when they are finishing the Span .

README.md Outdated
...
doMoreAsyncWork(new Runnable() {
Copy link
Member

Choose a reason for hiding this comment

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

a bit weird example. Might be better to show an example with completable future and whenComplete()

@carlosalberto
Copy link
Collaborator Author

Hey @yurishkuro docs got updated. Let me know ;)

README.md Outdated
...
}
```
`Scope.span()` and `ScopeManager.scope()` have been deprecated in order to prevent passing of `Scope` objects between threads (`Scope` objects are not guaranteed to be thread-safe).
Copy link
Member

Choose a reason for hiding this comment

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

In order to prevent the anti-pattern of passing scope...

}
}).thenRun(() -> {
// STEP 3 ABOVE: finish the Span when the work is done.
span.finish();
});
Copy link
Member

Choose a reason for hiding this comment

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

Much better!

@carlosalberto
Copy link
Collaborator Author

@yurishkuro Updated ;)

@carlosalberto carlosalberto merged commit 7010904 into opentracing:v0.32.0 Mar 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants