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

Jaeger/Zipkin/Opencensus exporters should populate service_name from Span Resource #1685

Closed
lzchen opened this issue Mar 10, 2021 · 6 comments
Labels
release:required-for-ga To be resolved before GA release

Comments

@lzchen
Copy link
Contributor

lzchen commented Mar 10, 2021

Currently, the three exporters that require service_name collect them all in different ways.

Jaeger -> after this pr, will be populated by global tracer_provider -> resource.
Opencensus -> passes in constructor
Zipkin -> Populated through global tracer_provider -> resource code

Following from @owais 's comment, users can instantiate multiple TracerProviders, which could possibly have different Resource attached to them, and therefore the service_name s could be different.

Dot net SDK has a reference to the creating tracer_provider in their exporters (which is weird), and Java SDK simply uses the currently exporting Span 's resource to populate service_name. I propose that we do something similar to Java for our exporters.

@lzchen lzchen added the 1.0.0rc2 release candidate 2 for tracing GA label Mar 10, 2021
@lzchen lzchen mentioned this issue Mar 10, 2021
11 tasks
@dmarar
Copy link
Contributor

dmarar commented Mar 11, 2021

In a batch will all the spans have the same resource?
As per the spec https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md

When used with distributed tracing, a resource can be associated with the TracerProvider when the TracerProvider is created. That association cannot be changed later. When associated with a TracerProvider, all Spans produced by any Tracer from the provider MUST be associated with this Resource.

Assuming that all spans from a tracer will have same resource we could do something similar to
#1607 (comment)

just a thought.

@owais
Copy link
Contributor

owais commented Mar 11, 2021

@dmarar Yes, this makes a lot of sense to me. Span translator should copy resource attributes to process tags (jaeger) and span tags (zipkin).

@srikanthccv
Copy link
Member

I am not sure if you can just take the 0th span and make it's resource as whole batch resource. I haven't closely looked at the span processor code so I don't know yet if it is possible for batch to have spans originating from different resources. Would it be more better if we group them by their resource and make proto.PostSpansRequest/thrift.Batch export for each of them?

@lzchen
Copy link
Contributor Author

lzchen commented Mar 11, 2021

From SIG meeting 03/11/2021 -> We will be restricting users to have a new instance of spanprocessor for each TracerProvider they create, so we can go with taking the 0th span to populate service_name for whole batch.

@lzchen
Copy link
Contributor Author

lzchen commented Mar 17, 2021

Work finished for Zipkin and Jaeger.

@lzchen lzchen removed their assignment Mar 17, 2021
@lzchen lzchen removed the 1.0.0rc2 release candidate 2 for tracing GA label Mar 17, 2021
@codeboten
Copy link
Contributor

Created a separate issue specifically for the opencensus exporter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:required-for-ga To be resolved before GA release
Projects
None yet
Development

No branches or pull requests

5 participants