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

#429 issue - OpenTrace zipKin Support #472

Merged
merged 14 commits into from
Oct 29, 2018
Merged

Conversation

galen0624
Copy link
Collaborator

"OpenTracing zipKin Support. Reference - Issue #429

Co-authored-by: Jeremy White jeremy.white@target.com
Co-authored-by: Kristina Fischer Kristina.Fischer@target.com
Co-authored-by: Michael Murphy Michael.Murphy@target.com
Co-authored-by: Nathan West nathan.west@target.com
Co-authored-by: Austin Hartzheim Austin.Hartzheim@target.com
Co-authored-by: Jacob Hansen Jacob.Hansen@target.com"

galen0624 and others added 3 commits March 20, 2018 13:25
Co-authored-by: Kristina Fischer <Kristina.Fischer@target.com>
Co-authored-by: Michael Murphy <Michael.Murphy@target.com>
Co-authored-by: Nathan West <nathan.west@target.com>
Co-authored-by: Austin Hartzheim <Austin.Hartzheim@target.com>
Co-authored-by: Jacob Hansen <Jacob.Hansen@target.com>
Co-authored-by: Kristina Fischer <Kristina.Fischer@target.com>
Co-authored-by: Michael Murphy <Michael.Murphy@target.com>
Co-authored-by: Nathan West <nathan.west@target.com>
Co-authored-by: Austin Hartzheim <Austin.Hartzheim@target.com>
Co-authored-by: Jacob Hansen <Jacob.Hansen@target.com>
@kmfischer3
Copy link
Contributor

kmfischer3 commented Mar 27, 2018

@magiconair We are working on fixing the Travis CI build, cleaning up the tracing code, and adding tests.

@galen0624
Copy link
Collaborator Author

@magiconair Any thoughts on this PR? Should we continue to work on this or should we go in a different direction?

@magiconair
Copy link
Contributor

Sorry, I just started a new job traveling between Amsterdam and Stockholm. I'll try to find some time to review this but I'd love to merge this.

@CLAassistant
Copy link

CLAassistant commented Aug 3, 2018

CLA assistant check
All committers have signed the CLA.

@kmfischer3
Copy link
Contributor

@magiconair We have added some tests for the tracing feature. Please let us know if there are any changes needed before this gets merged.

@magiconair
Copy link
Contributor

@kmfischer3 Sorry for the delay. Catching my breath after moving countries. Could you please rebase so that I can merge this? Thank you

@galen0624
Copy link
Collaborator Author

@magiconair I rebase lined this PR with the current 1.5.10 release.

@magiconair
Copy link
Contributor

Thank you.

@magiconair magiconair merged commit 57c76db into fabiolb:master Oct 29, 2018
@magiconair magiconair added this to the 1.5.11 milestone Oct 29, 2018
magiconair added a commit that referenced this pull request Oct 29, 2018
@lazystone
Copy link

I know that issue is closed but it looks that one must provide host and port of zipkin server in order to push spans there.

Will it be possible to use service discovery instead of that?
So you can provide tracing.connectString="http://zipkin-service" where zipkin-service is a service name in Consul instead of http:port pair?

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.

None yet

7 participants