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

Let client override the service name #19

Merged
merged 1 commit into from
Jun 3, 2014
Merged

Let client override the service name #19

merged 1 commit into from
Jun 3, 2014

Conversation

eirslett
Copy link
Contributor

When submitting and Endpoint, Zipkin expects the client to submit its own
host and port, but the service name of the service it's calling. This
commit adds a method to ClientTracer that lets you specify the name of the
service you're calling.

If no service name is specified, Brave will fallback to using the server's
service name when submitting the Endpoint.
@eirslett
Copy link
Contributor Author

See the commit message for more info.
This should also close #15.

@kristofa
Copy link
Member

Thanks Eirik. I'll probably only come to review this in detail next week.

@kristofa
Copy link
Member

kristofa commented Jun 3, 2014

I did a test and indeed the service name for the end points at client and server side should be the same. This makes the traces easier to read in zipkin-web and also the duration calculation makes more sense.

The ip addresses can remain as they are so we can see origin and destination for span.

Your changes look good, I'll merge PR and adapt the rest-easy and apache http client integration code. Thanks!

kristofa added a commit that referenced this pull request Jun 3, 2014
Let client override the service name. This allows setting endpoint service name at client and server the same which results in better readable traces in zipkin-web.
@kristofa kristofa merged commit c409375 into openzipkin:master Jun 3, 2014
@eirslett eirslett deleted the let-client-submit-service-name branch June 3, 2014 15:00
@noroute
Copy link

noroute commented Jul 7, 2014

Does this mean that it's not fixed in brave-jersey?

@kristofa
Copy link
Member

kristofa commented Jul 7, 2014

Yes that's right. brave-jersey is not adapted :(
You should be able to generate traces and visualise them but visualisation in zipkin-web can improve by applying similar change to jersey integration as I did to rest-easy and http client.

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.

3 participants