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

Apache and Jersey Client Inconsistencies #24

Closed
srapp opened this issue Jul 16, 2014 · 1 comment · Fixed by #27
Closed

Apache and Jersey Client Inconsistencies #24

srapp opened this issue Jul 16, 2014 · 1 comment · Fixed by #27

Comments

@srapp
Copy link

srapp commented Jul 16, 2014

I was messing a bit more with the Apache and Jersey implementations in Brave and noticed a few discrepancies between the two. Most notably, the headers are calculated differently and the attributes associated with the spans were inconsistent as well. It seems like it would be good for the client side tracing to be as consistent as possible between the libraries Brave offers, and I'd like to offer a plan.

We could introduce a shared 'brave-client' module that contains some interceptors which do the appropriate work of computing the header and span info, and also sends the CR and CS events, agnostic to the web client implementation. The implementation of adding a header to a request differs on which client library your using, but we could add RequestAdaptor interface that the shared module delegates to, and then the Jersey and Apache libraries can implement that accordingly.

I admittedly have started down this path in code to see if it could pan out, and I think there's some real promise with it, but I'd like to see what you thought first. I have a branch up in my fork called refactor-clients that has my current progress on the idea and may provide a bit more context about my proposal.

@kristofa
Copy link
Member

I agree that it is probably the right time to implement an abstraction on top of the existing ClientTracer and ServerTracer. It should make supporting additional libraries easier, with less code, and more consistent.

I'll do my best to have a closer look in next few days.

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 a pull request may close this issue.

2 participants