opentelemetry-java icon indicating copy to clipboard operation
opentelemetry-java copied to clipboard

Add ZipkinSpanExporter option to add resource attributes to spans

Open edwardxia opened this issue 3 years ago • 7 comments

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 avatar Mar 11 '21 01:03 edwardxia

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.

https://github.com/open-telemetry/opentelemetry-specification/issues/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?

anuraaga avatar Mar 11 '21 02:03 anuraaga

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:

https://github.com/open-telemetry/opentelemetry-java/blob/fc149c4d3802389ecbaa301e25a2489a787f1f39/exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporter.java#L203-L207

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.

edwardxia avatar Mar 11 '21 02:03 edwardxia

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.

anuraaga avatar Mar 11 '21 02:03 anuraaga

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.

edwardxia avatar Mar 11 '21 03:03 edwardxia

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?

jkwatson avatar May 05 '21 15:05 jkwatson

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.

ecourreges-orange avatar Sep 23 '21 15:09 ecourreges-orange

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

jkwatson avatar Sep 23 '21 19:09 jkwatson