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

Add Resource attributes to Zipkin exporter #1970

Closed
iNikem opened this issue Nov 2, 2020 · 8 comments
Closed

Add Resource attributes to Zipkin exporter #1970

iNikem opened this issue Nov 2, 2020 · 8 comments
Labels
blocked:spec blocked on open or unresolved spec

Comments

@iNikem
Copy link
Contributor

iNikem commented Nov 2, 2020

Specification says:

OpenTelemetry Span and Resource Attribute(s) MUST be reported as tags to Zipkin.

But ZipkinSpanExporter currently translates only service.name resource attribute and completely ignores all others.

@iNikem iNikem added the Bug Something isn't working label Nov 2, 2020
@Oberon00
Copy link
Member

Oberon00 commented Nov 2, 2020

Hmm, this specification seems a bit dubious to me. There can be a lot of resource attributes. Do we really want to duplicate them on each span?

@iNikem
Copy link
Contributor Author

iNikem commented Nov 2, 2020

Do we have any other option? We don't want to loose them completely, do we?

@jkwatson jkwatson added release:required-for-ga Required for 1.0 GA release priority:p2 Medium priority issues and bugs. labels Nov 2, 2020
@anuraaga
Copy link
Contributor

anuraaga commented Nov 3, 2020

I never noticed Resource there for some reason - that does seem quite problematic since I think most zipkin deployments will be destroyed if we send all the resource attributes in every span. Considering Zipkin exporter will commonly be used with Zipkin (collector will generally be used with OTLP I guess?), I think we are effectively required to have a configuration knob in the SDK to filter attributes if we do indeed add these.

open-telemetry/opentelemetry-specification#823 was pushed to post-GA but it makes me want to remove Resource from the zipkin exporter spec until post-GA too since I think they both need to be considered together.

@anuraaga
Copy link
Contributor

anuraaga commented Nov 3, 2020

Sent open-telemetry/opentelemetry-specification#1185 let's see what people think

@jkwatson
Copy link
Contributor

jkwatson commented Nov 4, 2020

Marking this as blocked by the spec until 1185 is resolved.

@Oberon00
Copy link
Member

open-telemetry/opentelemetry-specification#1185 has been merged, removing resources from Zipkin for now, so I'm removing the "Bug" label since we are now conformant.

The follow-up open-telemetry/opentelemetry-specification#1196 is only "allowed-for-ga", so I think we should not have this issue required-for-ga eiher.

@Oberon00 Oberon00 changed the title Zipkin exporter ignores Resource attributes Add Resource attributes to Zipkin exporter Dec 18, 2020
@jkwatson jkwatson removed priority:p2 Medium priority issues and bugs. release:required-for-ga Required for 1.0 GA release labels Dec 18, 2020
@jkwatson
Copy link
Contributor

I wonder if we can just close this and open a new issue if/when 1196 gets resolved in the spec?

@jkwatson
Copy link
Contributor

jkwatson commented Mar 5, 2021

closing due to inactivity, and no support in the specs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked:spec blocked on open or unresolved spec
Projects
None yet
Development

No branches or pull requests

4 participants