openzipkin-contrib / zipkin-otel

Reporters and collectors for use in OpenTelemetry
Apache License 2.0
4 stars 2 forks source link

Add OTLP/HTTP collector #13

Closed making closed 2 days ago

making commented 2 weeks ago

This pull request adds an OTLP/HTTP implementation of the Zipkin Collector.

It mostly follows the following document, but does not implement fallback for remoteEndpoint name and deprecated attribute names have been changed to new ones. https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk_exporters/zipkin.md

Also, the mapping of resource attributes is TBD in this document, and is not implemented in this PR, but I would like to open a separate issue to discuss the mapping of resource attributes.

making commented 2 weeks ago

Lookin good. did you test integrating the module yet?

Not yet. I don't think I understand the concept of modules yet 😅

codefromthecrypt commented 2 weeks ago

Not yet. I don't think I understand the concept of modules yet 😅

no worries that part is simpler for me to help with vs what you've done!

codefromthecrypt commented 2 weeks ago

ps when in doubt direction matters

in collector we are receiving data in a foreign format and putting it into ours. since we define our api and format, it follows that if plain comma separated is a norm in zipkin, we should coerce to that (it is)

on the other side if we are exporting to a foreign format, we should follow that format's conventions as much as possible for the same albeit visa versa reason. (e.g. bytesmessageformat for brave)

hope this helps

codefromthecrypt commented 2 days ago

thanks for all the hard work @making and also thanks for the help reviewing @kojilin and @minwoox!

making commented 2 days ago

Thanks all!