-
Notifications
You must be signed in to change notification settings - Fork 888
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
Remove the Jaeger Exporter #3964
Conversation
@marcalff Can you double-check please? |
I tried to remove file See #3567 (comment) Please check with @yurishkuro |
|
To clarify, I am fine with this cleanup, which I support, as the Jaeger exporter is no longer used. However, I am not in a position to formally approve it, having no voice for specs. |
In the Collector, we removed the Jaeger exporter on v0.86.0, specifically on open-telemetry/opentelemetry-collector-contrib/pull/26546 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the situation today is that because in the past OTEL needed to transform to Jaeger format we wrote this transformation spec and there is a transformer in collector-contrib matching that spec. Even though the exporters in both SDK and collector may be safely removed now, the transformer is still actively used by Jaeger itself. So the simplest solution is to move the transformer spec to the transformer dir in collector-contrib. Longer term we can consider migrating that code completely to Jaeger, but the transformer is technically a public API in the contrib, we don't know if other people are also using it.
Requesting changes.
@open-telemetry/collector-contrib-maintainer, what do you think of @yurishkuro proposal: #3964 (review)? Are you fine moving the spec to collector-contrib? If so then I think this PR should blocked until the spec is added to collector. |
Per our CODEOWNERS |
I'd be fine with hosting the lib as a standalone repo in the jaeger org. At some point, if we convert the main jaeger repo into multi-module monorepo we could move it there. |
Ready to review as open-telemetry/opentelemetry-collector-contrib#32187 is already merged. |
Fixes #3980
Follows #3551
Leftover after #3567: