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 ZipkinSpanExporter option to add resource attributes to spans #3009

Open
edwardxia opened this issue Mar 11, 2021 · 7 comments
Open

Add ZipkinSpanExporter option to add resource attributes to spans #3009

edwardxia opened this issue Mar 11, 2021 · 7 comments
Labels
blocked:spec blocked on open or unresolved spec Feature Request Suggest an idea for this project

Comments

@edwardxia
Copy link

edwardxia commented Mar 11, 2021

Is your feature request related to a problem? Please describe.

OT has the concept of ResourceAttributes but they are not tagged to spans.

For example, if I have:
-Dotel.resource.attributes=service.name=cats,service.namespace=mammals

Currently only service.name will be used as localEndpoint's service_name. However, I'd like to have service.namespace=mammals to be tagged to all spans, so that I can search spans by service.namespace=mammals.

Describe the solution you'd like

A configuration option, -Dotel.traces.resource.attributes, which is a comma separated list. User can specify either just names, or name value pairs. If a list item is just a name, the value would be read from -Dotel.resource.attributes. All the pairs will be tagged to all the spans.

For example:

-Dotel.resource.attributes=service.namespace=mammals
-Dotel.traces.resource.attributes=service.namespace,cloud.provider=aws

With these options, it will tag all spans with service.namespace=mammals (value is not specified directly, getting it from declared ResourceAttributes), and cloud.provider=aws (value is specified directly).

Describe alternatives you've considered

Manually adding the same tag to all spans during code execution and instrumentation turns out to be very difficult, especially for client side of a PRC call under auto-instrumentation.

Theoretically, this can also be achieved by writing customized exporter, however:

  1. All the exporters in OT are final, so that they are not overridable.
  2. Ideally 99% of the generateSpan logic in exporters can be reused, but this method is package-private, making it a headache to maintain a fork rather than just a call and further customization.

Given these conditions, I think it's better for OT to provide an option that automatically tagging user picked ResourceAttributes to all spans, and should OT handles this internally either during instrumentation or exporter.

@edwardxia edwardxia added the Feature Request Suggest an idea for this project label Mar 11, 2021
@anuraaga
Copy link
Contributor

Hi @edwardxia - are you using Zipkin? We currently don't have a spec on how to map resource to spans because of the potential cost explosion.

open-telemetry/opentelemetry-specification#823

But it is important to get that solved eventually.

In the meantime, while the exporters are final you should be able to delegate them with a wrapping exporter. We have some classes in sdk-extensions/trace-incubator that might help with it. Would it work?

@edwardxia
Copy link
Author

Are you using Zipkin?

Yes.

We currently don't have a spec on how to map resource to spans because of the potential cost explosion.

I understand tagging every single resource attribute would be too expensive, which is why I'm proposing let user explicitly choose which ones to be tagged. And of course, by default nothing would be tagged.

In the meantime, while the exporters are final you should be able to delegate them with a wrapping exporter. We have some classes in sdk-extensions/trace-incubator that might help with it. Would it work?

Not really, take ZipkinSpanExporter as an example:

public CompletableResultCode export(final Collection<SpanData> spanDataList) {
List<byte[]> encodedSpans = new ArrayList<>(spanDataList.size());
for (SpanData spanData : spanDataList) {
encodedSpans.add(encoder.encode(generateSpan(spanData)));
}

If you look at the quoted lines above, you will see that there is no way I can reuse export method to customize a Span, as Spans are built internally in package-private static generateSpan method. That means I cannot just build a wrapper, instead, I have to copy that whole class and then I can modify generateSpan method, this is not a very good idea.

@anuraaga
Copy link
Contributor

you will see that there is no way I can reuse export method to customize a Span,

The span is generated from SpanData. The wrapper could add attributes to SpanData, probably using DelegatingSpanData.

@edwardxia
Copy link
Author

The span is generated from SpanData. The wrapper could add attributes to SpanData, probably using DelegatingSpanData.

Ok, it will work but I will have to wrap SpanData in the spanDataList with a DelegatingSpanData. It's somewhat ugly but I can live with it for now.

@jkwatson jkwatson added the blocked:spec blocked on open or unresolved spec label May 5, 2021
@jkwatson
Copy link
Contributor

jkwatson commented May 5, 2021

If we want to add this feature, it will need to go through the spec process so it's not a java-only option and so that the configuration is standard across all languages. I've added the blocked:spec tag until that happens. @edwardxia Are you interested in working to make this part of the official specification?

@ecourreges-orange
Copy link

In the mean time of solving the spec (I think I saw a similar proposal elsewhere), I think we need to put a warning in the doc of opentelemetry-java-instrumentation that the zipkin exporter ignores all resource attributes except for service.name.
I lost a bit of time, noticed a otel.dropped_attributes_count in my spans, and figured that in the code of the exporter.
For example in opentelemetry-python with zipkin exporter, the attributes are sent, so it added to my confusion.
I think exporters should have consistent behavior across languages, companies are more likely to have multiple coding languages but a single span protocol (in my case zipkin for http simplicity and backwards compat).
I will start tomorrow with a pull request for the doc warnings.

@jkwatson
Copy link
Contributor

jkwatson commented Sep 23, 2021

Currently the spec does not say what is supposed to happen with the Resource and zipkin: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk_exporters/zipkin.md

TBD : This is work in progress document and it is currently doesn't specify mapping for these fields:

OpenTelemetry fields:

Resource attributes
Tracestate
Links
Zipkin fields:

local_endpoint
debug
shared

@jack-berg jack-berg changed the title New configuration option to tag ResourceAttributes to spans Add ZipkinSpanExporter option to add resource attributes to spans Jul 20, 2023
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 Feature Request Suggest an idea for this project
Projects
None yet
Development

No branches or pull requests

4 participants