-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Migrate Jaeger internal packages to OTEL contrib and update import paths #37999
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
Conversation
receiver/jaegerreceiver/internal/cmd/customtransport/buffered_read_transport_test.go
Outdated
Show resolved
Hide resolved
) | ||
|
||
// ConnBuilder Struct to hold configurations | ||
type ConnBuilder struct { |
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.
Why is this needed at all? It's for communicating out of the agent to the collector, such function does not exist in OTEL receiver
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.
Hey,
The test file thrift_processor_test.go
uses a real UDP receiver and grpc collector to perform tests.
Could you suggest a way to deal with this part if I were to remove reporter
?
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.
that test is too complex. It makes sense in the context of Jaeger where the agent is its own binary that forwards the data to collector, but in the context of OTEL Collector the agent is not a binary, it's just a library that receives the data and hands it off to the rest of the OTEL Collector pipeline. So the test needs to have the agent's server and receiver, and the processor, but it does not need the collector part, the collector part should be mocked by a Processor that just accumulates data in memory.
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.
Hey,
I made changes to the thrift_processor_test.go
to use a mock collector. Please look into it.
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
This has been addressed in #38655. Thanks for trying. |
TODO : Fix metrics -> use OTEL SDK for metrics
Description
Refer to this jaeger issue : jaegertracing/jaeger#6704. Author of the issue: yurishkuro
This PR is a follow-up to the PR - #37956 -> It deviates too much from it's initial description.
This PR moves the necessary Jaeger internal code, into the OTEL collector contrib repository in reference to the mentioned Jaeger Issue. The goal is to move certain dependencies as mentioned in the above issue from jaeger to this codebase :
Code Migration:
Files moved to
receiver/jaegerreceiver/internal
andcmd
,internal
,pkg
directories contain the files from jaeger codebase. The choice of the folder names and structure was in conjecture to the format used in jaeger codebase.The files were then imported to
receiver/jaegerreceiver/jaeger_agent_test.go
andreceiver/jaegerreceiver/trace_receiver.go
Import Path Updates:
All import statements have been modified as needed to make it work smoothly.
Dependency Cleanup:
Removing dependencies on Jaeger’s internal packages, thereby resolving build issues related to importing internal modules from an external repository.
Coordination with Jaeger:
This change is part of a broader effort affecting both Jaeger and OTEL contrib. The corresponding Jaeger PR that updates its references to the new OTEL contrib location is tracked separately.
Link to tracking issue
Fixes Jaeger issue #6408
Testing
Documentation