Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

It is possible to report invalid spans (endpoint.serviceName == null) #45

Closed
codefromthecrypt opened this issue Sep 8, 2016 · 5 comments

Comments

@codefromthecrypt
Copy link
Contributor

In zipkin's thrift, Endpoint.serviceName is not an optional field. Conventionally, some set this to empty, but not serializing it is a mistake. I noticed this when using the jersey client, which might hint there's a bug there (as I did specify a serviceName).

This should be enforced in unit tests or similar as otherwise the only way to know is after it reaches zipkin.

@codefromthecrypt
Copy link
Contributor Author

The workaround I'm using now is the following in Span.java

peer = new Endpoint().setService_name("");

@yurishkuro
Copy link
Member

yurishkuro commented Sep 8, 2016

Would #46 fix it? Or is this for some other endpoint, e.g. when not setting peer.serviceName tag?

@codefromthecrypt
Copy link
Contributor Author

codefromthecrypt commented Sep 9, 2016 via email

@yurishkuro
Copy link
Member

yurishkuro commented Sep 9, 2016

I agree, peer.serviceName should not be required. I just popped another PR #47 to avoid null.

@yurishkuro
Copy link
Member

released as 0.8.1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants