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

Zipkin exporter #495

Merged
merged 5 commits into from
Mar 11, 2020
Merged

Zipkin exporter #495

merged 5 commits into from
Mar 11, 2020

Conversation

krnowak
Copy link
Member

@krnowak krnowak commented Feb 24, 2020

This adds a zipkin exporter. For converting span data from otel to zipkin, I tried to follow https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/exporter-zipkin.md

The tests are still missing.
Added the tests, got ~85% coverage.

This PR implements both SpanSyncer and SpanBatcher interfaces as a separate types. Not sure if this is a way to go, but the code was simple enough to have both.
Implements now only the SpanBatcher interface.

One place I was stumped was when I wanted to convert otel trace ID (which is an array of 16 bytes) into zipkin trace ID (which is a pair of high and low uint64s). What endianness to use?
Using big endian.

Fixes #109.

@codeboten
Copy link
Contributor

It looks like it's using BigEndian according to some code in the repo:
https://github.com/openzipkin/zipkin-go/blob/62dc8b26c05e0e8b88eb7536eff92498e65bbfc3/proto/v2/encode_proto.go#L57-L58

@krnowak
Copy link
Member Author

krnowak commented Mar 10, 2020

Pushed an update. Still no tests, but added an example with a README. I couldn't get the zipkin collector to show the spans, though. Maybe it's because of missing endpoint information - the collector replies with HTTP 202 (which might mean that the sent data is okish, but not yet processed?).

@krnowak krnowak changed the title Initial zipkin exporter Zipkin exporter Mar 10, 2020
@jmacd
Copy link
Contributor

jmacd commented Mar 11, 2020

This looks good. I think we should try to have some sort of test, even if it's not fully validated against a real server at this time. Maybe just start an HTTP Server & check that you receive the expected data after a POST?

Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, looks good just missing a test or two.

exporters/trace/zipkin/zipkin.go Outdated Show resolved Hide resolved
exporters/trace/zipkin/zipkin.go Show resolved Hide resolved
@krnowak
Copy link
Member Author

krnowak commented Mar 11, 2020

Pushed some changes before I saw the review. Mostly fixes and tests.

krnowak added 5 commits March 11, 2020 22:22
The zipkin exporter implements the SpanBatcher interface. It follows
the current-at-the-time-of-writing document about conversion from
OpenTelemetry span data to Zipkin spans. Which means that endpoint
information is not yet filled.
The fixed paths should be prefixed with a slash. The "relative" paths
mean that git will ignore all the files that end with the path.
This sends span information to a locally running zipkin collector.
Currently I have a problem getting the collector to show me the spans
after accepting them with HTTP 202. Not sure if this is because of
missing endpoint information.
@krnowak
Copy link
Member Author

krnowak commented Mar 11, 2020

Updated again, added some docs.

Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:

@jmacd jmacd merged commit d9210f5 into open-telemetry:master Mar 11, 2020
@krnowak krnowak deleted the zipkin branch March 11, 2020 21:49
MikeGoldsmith pushed a commit to MikeGoldsmith/opentelemetry-go that referenced this pull request Mar 13, 2020
* Add zipkin exporter

The zipkin exporter implements the SpanBatcher interface. It follows
the current-at-the-time-of-writing document about conversion from
OpenTelemetry span data to Zipkin spans. Which means that endpoint
information is not yet filled.

* Fix typo in docs

* Add a zipkin example

This sends span information to a locally running zipkin collector.
Currently I have a problem getting the collector to show me the spans
after accepting them with HTTP 202. Not sure if this is because of
missing endpoint information.

* Make gitignore consistent

The fixed paths should be prefixed with a slash. The "relative" paths
mean that git will ignore all the files that end with the path.

* Add tests for zipkin exporter
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SpanData exporter implementation: Zipkin
5 participants