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

Drop spec matrix support of Java Thrift HTTP #2659

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

breedx-splk
Copy link
Contributor

This is related to an issue in the java sdk concerning removal of Java's Thrift+HTTP support for Jaeger.

The current Java Thrift+HTTP Jaeger exporter is built on top of the jaeger-client java library, which has recently been deprecated.

The existing exporter really only depends on the Jaeger client library to get the autogenerated Thrift classes. One could make the case that the java exporter could source the IDL itself directly from jaegertracing and autogenerate the classes at build time. That's definitely a possibility; however, I will make the case that reducing options will make for a clearer user experience. By dropping support, I think we also put forth a recommendation that is consistent with jaeger's messaging which now says

We urge all users to migrate to OpenTelemetry. Please refer to the notice in the documentation for details.

The docs site also claims native support for OTLP from the otel SDKs. This gives users a clear path forward without depending on the Thrift HTTP exporter.

Once we get spec support of this, we can begin the process of deprecating the exporter in the java sdk.

@breedx-splk breedx-splk requested review from a team July 11, 2022 22:29
@arminru arminru requested a review from yurishkuro July 12, 2022 09:39
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm

@reyang reyang merged commit 8b40d6b into open-telemetry:main Jul 14, 2022
@jack-berg
Copy link
Member

Not sure this should have been merged. I think this PR was in response to this comment, but that the comment was actually describing removing this entire line from the spec compliance table, rather than just switching the java column from + to -.

@breedx-splk
Copy link
Contributor Author

Not sure this should have been merged. I think this PR was in response to this comment, but that the comment was actually describing removing this entire line from the spec compliance table, rather than just switching the java column from + to -.

@jack-berg You're right, it was in response to that comment...so I guess I misunderstood the intended scope. I will follow up with another PR that removes the line completely and we can chat about it there. Thanks for pointing this out! 👍🏻

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.

6 participants