-
Notifications
You must be signed in to change notification settings - Fork 525
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
Integrate Jaeger gRPC collector #2976
Conversation
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.
Looking good!
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.
After looking more into what is missing, I now went down the road of not using the jaegerreceiver directly, but implementing the jaeger protobuf contract and starting a grpc server on our own. We still use some open-telemetry functions, but have full control over the grpc server. This is still only a draft, to see which direction we should go.
@axw please let me know what you think
(GitHub won't let me respond inline...)
I think that's the best option, at least for the near term.
Co-Authored-By: Andrew Wilkins <[email protected]>
This reverts commit 3ccd8ff.
This reverts commit d7ec4f6.
Codecov Report
@@ Coverage Diff @@
## jaeger #2976 +/- ##
=========================================
Coverage ? 78.01%
=========================================
Files ? 93
Lines ? 4735
Branches ? 0
=========================================
Hits ? 3694
Misses ? 1041
Partials ? 0
|
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.
LGTM! Just a handful of very minor cosmetic issues, and what I guess is an accidental use of legacy golang.org/x/net/context
.
Co-Authored-By: Andrew Wilkins <[email protected]>
Co-Authored-By: Andrew Wilkins <[email protected]>
* Integrate Jaeger gRPC collector (#2976) Add support for Jaeger gRPC Trace Intake Collector. The gRPC endpoint collects monitoring metrics and supports TLS communication, by reusing the `apm-server.ssl.*` configuration. By default the gRPC endpoint is disabled. closes #2962 Co-Authored-By: Andrew Wilkins <[email protected]> * [Jaeger] Add otel consumer converting batches to Elastic APM events (#3066) Add consumer converting incoming otel batches to Elastic APM format. Add integration tests covering incoming gRPC requests being transformed to beat events. partially implements #3307 * Jaeger http thrift (#3081) Add an HTTP handler, muxer, and server, in beater/jaeger for accepting Thrift-encoded trace data over HTTP. Refactor beater/jaeger.GRPCServer into Server, which now encapsulates both gRPC and HTTP servers. Move beater/api/jaeger code into beater/jaeger, which is the only user of GRPCCollector. If the beater/jaeger code grows significantly, we might consider having subpackages like beater/jaeger/grpc, beater/jaeger/http, etc. * [jaeger] Convert Timeevents to errors (#3085) * [jaeger] Convert Timeevents to errors Parse Timeevents from Jaeger spans and convert to elastic error events if they describe an error. Fixes #3007 * Add experimental flag to Jaeger integration (#3121) * tests/system: system test for Jaeger Thrift/HTTP (#3114) * tests/system: system test for Jaeger Thrift/HTTP * tests/system: system test for Jaeger gRPC * processor/otel: update approvals Co-authored-by: Silvia Mitter <[email protected]>
Add support for Jaeger gRPC Trace Intake Collector. The gRPC endpoint collects monitoring metrics and supports TLS communication, by reusing the `apm-server.ssl.*` configuration. By default the gRPC endpoint is disabled. closes elastic#2962 Co-Authored-By: Andrew Wilkins <[email protected]>
* Integrate Jaeger gRPC collector (#2976) Add support for Jaeger gRPC Trace Intake Collector. The gRPC endpoint collects monitoring metrics and supports TLS communication, by reusing the `apm-server.ssl.*` configuration. By default the gRPC endpoint is disabled. closes #2962 Co-Authored-By: Andrew Wilkins <[email protected]> * [Jaeger] Add otel consumer converting batches to Elastic APM events (#3066) Add consumer converting incoming otel batches to Elastic APM format. Add integration tests covering incoming gRPC requests being transformed to beat events. partially implements #3307 * Jaeger http thrift (#3081) Add an HTTP handler, muxer, and server, in beater/jaeger for accepting Thrift-encoded trace data over HTTP. Refactor beater/jaeger.GRPCServer into Server, which now encapsulates both gRPC and HTTP servers. Move beater/api/jaeger code into beater/jaeger, which is the only user of GRPCCollector. If the beater/jaeger code grows significantly, we might consider having subpackages like beater/jaeger/grpc, beater/jaeger/http, etc. * [jaeger] Convert Timeevents to errors (#3085) * [jaeger] Convert Timeevents to errors Parse Timeevents from Jaeger spans and convert to elastic error events if they describe an error. Fixes #3007 * Add experimental flag to Jaeger integration (#3121) * tests/system: system test for Jaeger Thrift/HTTP (#3114) * tests/system: system test for Jaeger Thrift/HTTP * tests/system: system test for Jaeger gRPC Co-authored-by: Andrew Wilkins <[email protected]>
fixes #2962
TODO:
Switched to implement the jaeger collector ourselves instead of using otel jaegerreceiver, allowing us to have more control over the gRPC server.
Points to consider from Jaeger collectorjaegerreceiverStartTraceReception
always starts agent. For now just put a patch into the vendored file, will be fixed with open-telemetry PR#434startCollector always starts all receiver protocols, independent of configuration. Needs to be fixed.Improve how configuration can be created for jaeger receiver. If not using CreateDefaultConfig() all names, protocols etc need to be duplicated, since not exported and no supporter methods available.*~ Logger is unused. Figure out how to integrate with our logger once this is fixed.~
Context is unused