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

Add ParentReferenceType. #906

Closed
wants to merge 1 commit into from
Closed

Add ParentReferenceType. #906

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 27 additions & 0 deletions specification/trace/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Table of Contents
* [GetDescription](#getdescription)
* [GetIsOk](#getisok)
* [SpanKind](#spankind)
* [ParentReferenceKind](#parentreferencekind)
* [Concurrency](#concurrency)
* [Included Propagators](#included-propagators)

Expand Down Expand Up @@ -303,6 +304,8 @@ The API MUST accept the following parameters:
option for implicit parenting from the current context as a default behavior.
See [Determining the Parent Span from a Context](#determining-the-parent-span-from-a-context)
for guidance on `Span` parenting from explicit and implicit `Context`s.
- [`ParentReferenceKind`](#parentreferencekind), default to
`ParentReferenceKind.CHILD_OF` if not specified.
- [`SpanKind`](#spankind), default to `SpanKind.Internal` if not specified.
- [`Attributes`](../common/common.md#attributes). Additionally,
these attributes may be used to make a sampling decision as noted in [sampling
Expand Down Expand Up @@ -358,6 +361,8 @@ description](../overview.md#links-between-spans).
A `Link` is defined by the following properties:

- (Required) `SpanContext` of the `Span` to link to.
- (Optional) `ParentReferenceKind`, default to `ParentReferenceKind.CHILD_OF`
if not specified.
- (Optional) One or more `Attribute`s as defined [here](../common/common.md#attributes).

The `Link` SHOULD be an immutable type.
Expand Down Expand Up @@ -676,6 +681,28 @@ To summarize the interpretation of these kinds:
| `CONSUMER` | | yes | maybe | |
| `INTERNAL` | | | | |

## ParentReferenceKind

`ParentReferenceKind` describes an additional property of the causal relationship
between the `Span` and its parent. Similarly to `SpanKind`, it describes a property
that benefit tracing systems during analysis.

The property described by `ParentReferenceKind` reflects whether the parent `Span`
depends on the child `Span` or not, and can be used to calculate the critical
path.

These are the possible `ParentReferenceKind`s:
Copy link
Member

Choose a reason for hiding this comment

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

As also explained in the comment the value names are not very. In both cases the Span is a child Span, it is just that parent waits or not for the result of the child Span.

So if we really need to support something that express this property names like: waits_for_result and ignores_result (or something like that are more meaningful).

I would like to understand how important is to know this information? Can this be added later? Are we breaking any real backend if this is not supported?

I think we should have a longer debate on this, I have some ideas about how to solve this:

  • I would suggest always using links when parent does not wait for any result from the child. This is because the child Span can finish long time after the parent (this can also happen when parent waits for result, but less likely).
  • In cases like batches we encourage using links as well, so we need probably to distinguish between these cases.
  • In OpenCensus my initial idea was to use MessageEvents to describe the relationship between messages exchanged by the child and parent, with the idea that it may actually be a bidirectional stream so there may be more waiting moments not just one.

So to summarize: Can we ignore this for the moment and add it later?

Copy link
Member

@Oberon00 Oberon00 Sep 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

See the linked issue. We decided that we cannot go GA without having OpenTracing compatibility here. So we need to add some way of representing this information, if only for backward compatibility reasons.

Copy link
Member

Choose a reason for hiding this comment

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

Then we really need an OTEP :)


* `CHILD_OF` Indicates the parent `Span` depends on the outcome of the child
`Span` in some capacity. Observe the child `Span` may run synchronously or
asynchronously, but ultimately the parent depends on its outcome. Although
it is expected the child `Span` completes before the parent, this is not
always the case. For example, the parent may timeout before the child
completes, thus ignoring the child's pending outcome.
* `FOLLOWS_FROM` Indicates the parent `Span` does not depend in any way in the
result of the child `Span`. The child `Span` may or may not complete
before the parent is done.

## Concurrency

For languages which support concurrent execution the Tracing APIs provide
Expand Down