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.

Sign up for GitHub

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

Jump to bottom

Add specs on Span creation. #124

Merged
merged 6 commits into from
Jun 17, 2019
Merged

Add specs on Span creation. #124

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fd20911
Add specs on Span creation.
songy23 Jun 14, 2019
3747963
Put back Tracer.SpanBuilder
songy23 Jun 14, 2019
a7e7ec0
s/called/set
songy23 Jun 14, 2019
6e03ea0
Fix spaces. Add a note about span creation from Tracer.
songy23 Jun 14, 2019
8dbbd91
Update recordEvents and StartSpan.
songy23 Jun 14, 2019
cd1e5f7
Update default samplers, links and StartSpan.
songy23 Jun 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions specification/tracing-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Table of Content
* [SpanContext](#spancontext)
* [Span](#span)
* [Span creation](#span-creation)
* [StartSpan](#startspan)
* [Span operations](#span-operations)
* [GetContext](#getcontext)
* [IsRecordingEvents](#isrecordingevents)
Expand Down Expand Up @@ -107,7 +108,8 @@ Returns an object that defines a scope where the given `Span` will be set to the
The scope is exited and previous state should be restored when the returned object is closed.

#### SpanBuilder
Returns a `SpanBuilder` to create and start a new `Span`.
Returns a `SpanBuilder` to create and start a new `Span`
if a `Builder` pattern for [Span creation](#span-creation) is used.

Required parameters:

Expand Down Expand Up @@ -191,7 +193,44 @@ creation](#span-creation).

### Span creation

TODO: SpanBuilder API https://github.com/open-telemetry/opentelemetry-specification/issues/37
API MUST provide a way to create a new `Span`. Each language implementation should
follow its own convention on `Span` creation, for example `Builder` in Java,
`Options` in Go, etc. `Span` creation method MUST be defined on `Tracer`.

Required parameters:

- Name of the span.

Optional parameters (or corresponding setters on `Builder` if using a `Builder` pattern):

- Parent `Span`. If not set, the value of [Tracer.getCurrentSpan](#getcurrentspan)
at `StartSpan` time will be used as parent. MUST be used to create a `Span`
when manual Context propagation is used OR when creating a root `Span` with
a parent with an invalid `SpanContext`.
- Parent `SpanContext`. If not set, the value of [Tracer.getCurrentSpan](#getcurrentspan)
at `StartSpan` time will be used as parent. MUST be used to create a `Span`
Copy link
Contributor

Choose a reason for hiding this comment

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

you can only use Span or SpanContext but not both. If both is used then should it return an error, or one takes precedence over the other.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, at least for the non-builder pattern (as for the builder pattern it simply overrides any previous value).

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated in #131.

when the parent is in a different process.
- The option to become a root `Span` for a new trace.
Copy link
Contributor

@rghetia rghetia Jun 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

If there is an option to make it a root Span then in the previous option (Span and SpanContext) it should not be 'MUST' in "MUST be used to create a Span when creating a root Span with a parent with an invalid SpanContext.

I think if an option is provided to make it a root span then parent span/span context should be ignored. There should not be an option to pass Invalid Span or Invalid SpanContext to create a root span. Just provide one way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed as well, also for the non-builder pattern ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think if an option is provided to make it a root span then parent span/span context should be ignored.

Good catch, and that's consistent with current Java APIs. Created #131 to clarify.

If not set, the value of [Tracer.getCurrentSpan](#getcurrentspan) at `StartSpan`
time will be used as parent.
- `Sampler` to the newly created `Span`. If not set, the implementation should provide a
default sampler used by Tracer.
- Collection of `Link`s that will be associated with the newly created Span
- The override value for [a flag indicating whether events should be recorded](#isrecordingevents)
for the newly created `Span`. If not set, the implementation will provide a default.
- `SpanKind` for the newly created `Span`. If not set, the implementation will
provide a default value `INTERNAL`.
SergeyKanzhelev marked this conversation as resolved.
Show resolved Hide resolved

#### StartSpan

Starts a new `Span`.
SergeyKanzhelev marked this conversation as resolved.
Show resolved Hide resolved

If called multiple times with `Builder` pattern, the same `Span` will be returned.

There should be no parameter if using a `Builder` pattern. Otherwise, `StartSpan`
should accept all the optional parameters described in [Span creation](#span-creation).

Returns the newly created `Span`.

### Span operations

Expand Down