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

Jump to bottom

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

Let client override the service name
26f96ad 
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
@kristofa
Merge pull request #19 from eirslett/let-client-submit-service-name
c409375 
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.

@kristofa kristofa mentioned this pull request Sep 7, 2014
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@eirslett @kristofa @noroute