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

Span should support creation with lazy links #272

Closed
lmolkova opened this issue Oct 11, 2019 · 7 comments
Closed

Span should support creation with lazy links #272

lmolkova opened this issue Oct 11, 2019 · 7 comments
Labels
pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package

Comments

@lmolkova
Copy link

No description provided.

@lmolkova lmolkova added this to the API v0.1 complete milestone Oct 12, 2019
@bruno-garcia
Copy link
Member

On #273 there's a link to the new approved sampling spec. There under TLDR it says:

Remove addLink APIs from the Span interface, and allow recording links only during the span construction time.

Maybe I don't understand this issue but does it conflict with the quite above?

@lmolkova
Copy link
Author

no, links should be provided in StartSpan, but not later.
Today we only have API that accepts array of links, which leads to open-telemetry/opentelemetry-specification#305

So here, we need to remove Span.AddLink and find a nice way to accept Func<IEnumberable<Link>> on span creation.
Open question: do we need to keep overloads that accepts IEnumerable

@bruno-garcia
Copy link
Member

bruno-garcia commented Oct 15, 2019

If we take a Func<IEnumerable<Link>>, we'd pass it along to the sampling decision and in case the span is sampled in, we'd need to invoke the func again to get links?

Do you see a way around that?
We could take a type (like LinkProvider) which takes the Func in the constructor and caches the value if invoked. This way if the sampler does invoke it, the value is kept in the instance and can later be used by the SDK.
Downside is that now we allocate the LinkProvider and also the Func<> for each Span.

@lmolkova
Copy link
Author

lmolkova commented Oct 15, 2019

Noop impl will ignore func. Sdk will invoke lambda to make sampling decision - you have to know links to make it.

The only thing lambda protects from is perf impact for noop case.

@bruno-garcia
Copy link
Member

Right. But one time we'll give the Func<> to ISampler. That'll invoke it to decide whether to sample or not.

Then it returns true and we need to populate the Span.Links so we'll need to invoke the Func<> again.

@SergeyKanzhelev
Copy link
Member

SergeyKanzhelev commented Oct 28, 2019

@lmolkova @bruno-garcia Is there anything left here after #282?

@SergeyKanzhelev SergeyKanzhelev added the pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package label Oct 28, 2019
@SergeyKanzhelev SergeyKanzhelev modified the milestones: API v0.1 complete, v0.3 Oct 28, 2019
@lmolkova
Copy link
Author

lmolkova commented Nov 8, 2019

this is done in #282.

@lmolkova lmolkova closed this as completed Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package
Projects
None yet
Development

No branches or pull requests

3 participants