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

Adds zipkin http transport and moves off deprecated collector #2151

Closed
wants to merge 1 commit into from
Closed

Adds zipkin http transport and moves off deprecated collector #2151

wants to merge 1 commit into from

Conversation

codefromthecrypt
Copy link

This allows you to use zipkin's http endpoint, and any modern transport.
This is important as scribe is deprecated. Notably, this doesn't change
internal plumbing to use new Brave Tracer api, though that can be done
in a later change.

@codefromthecrypt
Copy link
Author

This came out of a request by @TimurShakirov

@davsclaus
Copy link
Contributor

Thanks a lot. I think I spotted a new source file which did not have the ASF license header.

@codefromthecrypt
Copy link
Author

ok all the headers and import related distractions should be gone now

@davsclaus
Copy link
Contributor

Thanks @adriancole have you btw had any chance to try to example/camel-example-zipkin with these new changes and see if that example still works.

And maybe that example needs some updates as well?

However this work looks great and we will surely merge it. Thanks for the contribution.

@codefromthecrypt
Copy link
Author

codefromthecrypt commented Dec 15, 2017 via email

This allows you to use zipkin's http endpoint, and any modern transport.
This is important as scribe is deprecated. Notably, this doesn't change
internal plumbing to use new Brave Tracer api, though that can be done
in a later change.
@codefromthecrypt
Copy link
Author

ok examples work, but I'm not sure of the data. Going to compare vs the old one

@codefromthecrypt
Copy link
Author

OK the data matches the old version. I think there is some follow-up work to do around modeling, which hasn't to do with this change, but should be done. That said, I have to budget time to work through that later as I have to round-robin through other project maintenance.

PS @davsclaus if you are up to it we have a meeting in may (obv we can work through things prior) https://docs.google.com/document/d/1u5sKZkhLux10bEZuXMhYILkigBtx4vIgfr1WRLq3ZfY/edit?ts=5a329ada#heading=h.siw588q01xt9

@davsclaus
Copy link
Contributor

@adriancole thanks merging this, but the OSGi stuff dont work. If we had a $ for every time a JAR had problems with its OSGi MANIFEST.MF we would be rich.

You can run this tests in

camel/tests/camel-itest-karaf (master)/$ mvn test -Dtest=CamelZipkinTest

And then fiddle with the platforms/karaf/features/.../features.xml file where the karaf osgi file is, and adjust the camel-zipkin feature to see if you can get the osgi stuff working.

We will have to disable it and log a JIRA.

@davsclaus davsclaus closed this in 7de1986 Dec 17, 2017
@codefromthecrypt
Copy link
Author

codefromthecrypt commented Dec 18, 2017 via email

@davsclaus
Copy link
Contributor

Yes thanks @adriancole

And thanks for the heads-up on that meeting. I dont know my calendar at that time in 2018.

codefromthecrypt pushed a commit to openzipkin/zipkin that referenced this pull request Dec 19, 2017
This fixes packaging where zipkin2 types were in the zipkin 1 jar.
It also restores the missing MANIFEST.MF file needed for OSGi to
work properly.

See apache/camel#2151
codefromthecrypt pushed a commit to openzipkin/zipkin that referenced this pull request Dec 19, 2017
This fixes packaging where zipkin2 types were in the zipkin 1 jar.
It also restores the missing MANIFEST.MF file needed for OSGi to
work properly.

See apache/camel#2151
codefromthecrypt pushed a commit to openzipkin/brave that referenced this pull request Dec 20, 2017
This removes the need to share internal packages between the brave 3 and
4 apis and zipkin. In doing so, it unlocks some OSGi problems.

This also makes it easier to transition to zipkin2 by allowing reporter2
as a Brave.Builder.spanReporter arg.

See apache/camel#2151
codefromthecrypt pushed a commit to openzipkin/brave that referenced this pull request Dec 20, 2017
This removes the need to share internal packages between the brave 3 and
4 apis and zipkin. In doing so, it unlocks some OSGi problems.

This also makes it easier to transition to zipkin2 by allowing reporter2
as a Brave.Builder.spanReporter arg.

See apache/camel#2151
@codefromthecrypt codefromthecrypt deleted the zipkin-spanreporter branch December 20, 2017 09:32
abesto pushed a commit to abesto/zipkin that referenced this pull request Sep 10, 2019
This fixes packaging where zipkin2 types were in the zipkin 1 jar.
It also restores the missing MANIFEST.MF file needed for OSGi to
work properly.

See apache/camel#2151
thgoexpt added a commit to thgoexpt/Java-distributed-tracing-implementation that referenced this pull request Apr 7, 2022
This removes the need to share internal packages between the brave 3 and
4 apis and zipkin. In doing so, it unlocks some OSGi problems.

This also makes it easier to transition to zipkin2 by allowing reporter2
as a Brave.Builder.spanReporter arg.

See apache/camel#2151
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants