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

Apply breaking changes to Span API (from Sampling OTEP 0006) #132

Closed
Oberon00 opened this issue Sep 10, 2019 · 6 comments
Closed

Apply breaking changes to Span API (from Sampling OTEP 0006) #132

Oberon00 opened this issue Sep 10, 2019 · 6 comments
Assignees
Labels
api Affects the API package. tracing
Milestone

Comments

@Oberon00
Copy link
Member

Oberon00 commented Sep 10, 2019

Even though we don't support sampling yet, the following already applies:

  1. Add capability to record Attributes that can be used for sampling decision during the Span
    creation time.
  2. Remove addLink APIs from the Span interface, and allow recording links only during the span
    construction time.

Reference:

@Oberon00 Oberon00 added api Affects the API package. tracing labels Sep 12, 2019
@Oberon00
Copy link
Member Author

Related #134 (also part of the same OTEP, but that one is not breaking)

@c24t c24t added this to the Alpha v0.2 milestone Oct 11, 2019
@c24t c24t modified the milestones: Alpha v0.2, Alpha v0.3 Oct 31, 2019
@codeboten
Copy link
Contributor

codeboten commented Oct 31, 2019

When removing add_link from the api, i'm assuming we'll also want to remove add_lazy_link as well:

def add_link(
self,
link_target_context: "SpanContext",
attributes: types.Attributes = None,
) -> None:
"""Adds a `Link` to another span.
Adds a single `Link` from this Span to another Span identified by the
`SpanContext` passed as argument.
"""
def add_lazy_link(self, link: "Link") -> None:
"""Adds a `Link` to another span.
Adds a `Link` that has previously been created.
"""

@codeboten
Copy link
Contributor

Started the work here #259

@codeboten
Copy link
Contributor

On the following:

  1. Add capability to record Attributes that can be used for sampling decision during the Span
    creation time.

It's currently possible for a Sampler to pass attributes when creating a Decision. This sampling decision's attributes are then passed into a Span. This approach seems to address the otep. Any thoughts?

@Oberon00
Copy link
Member Author

Oberon00 commented Nov 8, 2019

@codeboten No, the OTEP is confusingly worded here: This is about adding span attributes right when creating the span so that the sampler can e.g. sample out all spans that have a http.url that starts with http://example.com/healthcheck. Normally you would call start_span first and the set_attribute on the span. The OTEP basically requires start_span to have an additional parameter to accept span attributes right away, and for these attributes to be also provided to the sampler.

@Oberon00
Copy link
Member Author

Oberon00 commented Nov 8, 2019

This OTEP was also already "implemented" in the spec, see https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#span-creation:

The API MUST accept the following parameters:
[...]

  • Attributes - similar API with Span::SetAttributes. These attributes will be used to make a sampling decision as noted in sampling description. Empty list will be assumed if not specified.

@c24t c24t closed this as completed Dec 5, 2019
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
* feat(plugin): add supportedVersions property

Closes open-telemetry#132

Signed-off-by: Olivier Albertini <[email protected]>

* fix: add recommendations

from markwolff
from mayurkale22

Signed-off-by: Olivier Albertini <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the API package. tracing
Projects
None yet
Development

No branches or pull requests

3 participants