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.

Sign up for GitHub

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

Document special Zipkin conversion cases #566

Merged
merged 6 commits into from
Apr 28, 2020
Merged

Document special Zipkin conversion cases #566

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3265160
Document special Zipkin conversion cases
pavolloffay Apr 16, 2020
1d7d32d
Fix typos
pavolloffay Apr 16, 2020
dd298fd
Fix lint
pavolloffay Apr 16, 2020
1df8d80
Change heading and add explanation for special cases
pavolloffay Apr 23, 2020
5d5444d
Reference zipkin v1 converter
pavolloffay Apr 24, 2020
6da1181
Fix review comments
pavolloffay Apr 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions specification/trace/sdk_exporters/zipkin.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,17 @@ omitted from the payload when they are empty in the OpenTelemetry `Span`.

For example, an OpenTelemetry `Span` without any `Event` should not have an
`annotations` field in the Zipkin payload.

## Considerations for Legacy (v1) Format

Zipkin's v2 [json](https://github.com/openzipkin/zipkin-api/blob/master/zipkin2-api.yaml) format was defined in 2017, followed up by a [protobuf](https://github.com/openzipkin/zipkin-api/blob/master/zipkin.proto) format in 2018.

Frameworks made before then use a more complex v1 [Thrift](https://github.com/openzipkin/zipkin-api/blob/master/thrift/zipkinCore.thrift) or [json](https://github.com/openzipkin/zipkin-api/blob/master/zipkin-api.yaml) format that notably differs in so far as it uses terminology such as Binary Annotation, and repeats endpoint information on each attribute.

Consider using [V1SpanConverter.java](https://github.com/openzipkin/zipkin/blob/master/zipkin/src/main/java/zipkin2/v1/V1SpanConverter.java) as a reference implementation for converting v1 model to OpenTelemetry.

Follows a list of cases that require special handling:
Copy link
Member

Choose a reason for hiding this comment

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

no harm in listing these, of course, but not much benefit either imo - if people do look at V1SpanConverter code, those cases would be handled there already.

Copy link

Choose a reason for hiding this comment

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

PS even in v2 these cases can happen. For example, people reporting data in chunks.

The what's notably different is that in v1, these might be resolvable at ingest time using annotations. If we are going to say anything we should probably say more specific, or not say anything as it is misleading to say missing timestamp happens here (it implies it can't happen otherwise)

Ex.

Suggested change
Follows a list of cases that require special handling:
The span timestamp and duration were late additions to the V1 format. As in the code link above, it is possible to heuristically derive them from annotations.

It is things like this, that even doing this part will take some back and forth to get the words leading in the right direction, why I mention it is hard doc labor. What I pasted is ok if you remove the other text here. Just I would prefer you use this as one example as opposed to hinting that this doc will enumerate all things, such as the bullets below do.

I know intentions are good, just let's be careful not to dig a hole, basically.

Copy link
Member Author

Choose a reason for hiding this comment

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

no harm in listing these, of course, but not much benefit either imo - if people do look at V1SpanConverter code, those cases would be handled there already.

From glancing at the java code it might not be obvious that timestamps/duration might be missing.

You are right there are probably several heuristics that can be applied during the conversion. For the OTEL scope we are probably interested in those that can happen at ingestion time.
I know only a small subset from what we do in Zipkin->Jaeger converter. I have applied your change. The goal here is to document that special cases exist and if it is convenient document which ones.


* Missing start time
* Missing duration
* TBD