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

Better attributes for ignored parents #461

Closed
krnowak opened this issue Feb 3, 2020 · 6 comments · Fixed by #1726
Closed

Better attributes for ignored parents #461

krnowak opened this issue Feb 3, 2020 · 6 comments · Fixed by #1726
Assignees
Labels
pkg:SDK Related to an SDK package
Milestone

Comments

@krnowak
Copy link
Member

krnowak commented Feb 3, 2020

When #451 gets merged, we will get the WithNewRoot() option, that will ignore local and remote parents in the context and create a span with a new trace ID. The ignored parents will end up as links with a string attribute being "ignored-on-demand": "local" or "ignored-on-demand": "remote" for local and remote parents, respectively.

This needs a better name and likely some piece of specification in SDK.

@jmacd
Copy link
Contributor

jmacd commented Feb 3, 2020

I agree that the spec should discuss this. Does the spec describe WithNewRoot()?

@krnowak
Copy link
Member Author

krnowak commented Feb 3, 2020

Not outright, no. There is a confusing sentence in https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#span-creation (in the the second bullet point): API MAY also have an option for implicit parenting from the current context as a default behavior. So we introduce an option for "explicit non-parenting". ;)

So not sure about the way forward with it - OTEP or a PR to spec?

@jmacd
Copy link
Contributor

jmacd commented Feb 3, 2020

I think a PR to the spec would be appropriate.

@krnowak
Copy link
Member Author

krnowak commented Feb 4, 2020

So, went through the spec and found a sentence in a paragraph after the bullet points: "Implementations MUST provide an option to create a Span as a root span, and MUST generate a new TraceId for each root span created.".

So I think the WithNewRoot as an option is already covered, but added some wording for adding the parents as links, please see the PR here: open-telemetry/opentelemetry-specification#436.

@krnowak krnowak self-assigned this Feb 4, 2020
@rghetia rghetia added this to the Alpha v0.4 milestone Feb 6, 2020
@MrAlias MrAlias added the blocked:specification Waiting on clarification of the OpenTelemetry specification before progress can be made label May 15, 2020
@MrAlias MrAlias modified the milestones: Beta v0.6, Beta v0.7 May 15, 2020
@MrAlias
Copy link
Contributor

MrAlias commented Sep 30, 2020

Given the specification has deferred this decision we should back out the attributes that we are setting. This will ensure we are able to add them back in in a compatible manner after GA.

@MrAlias MrAlias added pkg:SDK Related to an SDK package priority:p1 labels Sep 30, 2020
@MrAlias MrAlias modified the milestones: Next, RC1 Sep 30, 2020
@MrAlias MrAlias removed the blocked:specification Waiting on clarification of the OpenTelemetry specification before progress can be made label Sep 30, 2020
@MrAlias
Copy link
Contributor

MrAlias commented Feb 16, 2021

We need to remove the links (so we do not have to introduce breaking changes), document how users can add their own links if they want them, and wait for the specification to come to a consensus on this (open-telemetry/opentelemetry-specification#1188).

@MrAlias MrAlias self-assigned this Feb 16, 2021
MrAlias referenced this issue in MrAlias/opentelemetry-go Mar 24, 2021
To ensure forwards compatibility, remove the unspecified links currently
set on `NewRoot` spans.

Resolves #461
MrAlias referenced this issue in MrAlias/opentelemetry-go Mar 24, 2021
To ensure forwards compatibility, remove the unspecified links currently
set on `NewRoot` spans.

Resolves #461
Aneurysm9 pushed a commit that referenced this issue Mar 25, 2021
* Remove links on NewRoot spans

To ensure forwards compatibility, remove the unspecified links currently
set on `NewRoot` spans.

Resolves #461

* Remove links from oteltest tracer to match
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:SDK Related to an SDK package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants