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

Update SpanProcessors to be invoked in order they are registered #1195

Closed
MrAlias opened this issue Sep 22, 2020 · 1 comment · Fixed by #1198
Closed

Update SpanProcessors to be invoked in order they are registered #1195

MrAlias opened this issue Sep 22, 2020 · 1 comment · Fixed by #1198
Assignees
Labels
area:trace Part of OpenTelemetry tracing help wanted Extra attention is needed pkg:SDK Related to an SDK package
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Sep 22, 2020

Currently a TracerProvicer stores known SpanProcessors in a map:

type spanProcessorMap map[SpanProcessor]*sync.Once

When a span is started or finished it iterates through the map and invokes the associated method of the SpanProcessor:

for sp := range sps {
sp.OnStart(span.data)
}

for sp := range sps {
sp.OnEnd(sd)
}

The specification specifies this execution to be ordered:

Span processors can be registered directly on SDK TracerProvider and they are invoked in the same order as they were registered.

Based on a recent issue it was made clear that this hasn't had any real effect because there is no direct linking between registered SpanProcessors. However, we should update the execution model and conform to the specified behaviour so we are compliant.

This will likely require changing the underlying spanProcessorMap type. It will need to still provide getter/setter functionality as the existing map does so the TracerProvider can correctly unregister any SpanProcessor, but it will need to also track order.

@MrAlias MrAlias added help wanted Extra attention is needed pkg:SDK Related to an SDK package area:trace Part of OpenTelemetry tracing release:required-for-ga labels Sep 22, 2020
@MrAlias MrAlias added this to the RC1 milestone Sep 22, 2020
@MrAlias MrAlias added this to To do in GA Burndown via automation Sep 22, 2020
@huikang
Copy link
Member

huikang commented Sep 23, 2020

Hi, @MrAlias , I'd like to give this a shot. Thanks.

@MrAlias MrAlias linked a pull request Sep 24, 2020 that will close this issue
GA Burndown automation moved this from To do to Done Sep 24, 2020
@MrAlias MrAlias added this to Done in OpenTelemetry Go RC Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:trace Part of OpenTelemetry tracing help wanted Extra attention is needed pkg:SDK Related to an SDK package
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants