[WIP] Mongodb otel refactor #40472
[WIP] Mongodb otel refactor #40472
Conversation
Instead of storing the full mongo event object, we only save command name and the command itself
As suggested by @brunobat, moving the implementation from mongo-client to opentelemetry extension
Otherwise OOM would happen atm
Fixing failing windows tests
The mongo dependency is optional and we must include mongo tracing only if mongo dependency is present and mongo tracing is enabled Also moving the build item to TracerProcessor
|
/cc @radcortez (opentelemetry) |
|
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
The endpoints called from blockingClientError or reactiveClientError tests will return 500 because of invalid mongo query. We assert that the invalid query was added to the traces
Verify that mongo span has parent span id matching root span id
|
@brunobat I've added docs and parent-child relationship tests, which shows that in reactive case the mongo span is not added to the parent, as the listener is executed in a separate thread from vert.x event loop thread containing parent span. Any hints how to propagate the parent context to the listener. I've tried a few things but it didn't work |
|
🙈 The PR is closed and the preview is expired. |
To correctly implement opentelemetry traces in mongodb listeners we need to propagate traces with the help of ReactiveContextProvider, which will create request context to be used in mongo command listener implementations
If mongo request context contains trace context then we must use it as parent, otherwise mongo span would not be the child of the parent span
|
Let me think about this... Please park it for a couple days. |
| @@ -659,6 +659,7 @@ Additional exporters will be available in the Quarkiverse https://docs.quarkiver | |||
| * https://quarkus.io/guides/scheduler[`quarkus-scheduler`] | |||
| * https://quarkus.io/guides/smallrye-graphql[`quarkus-smallrye-graphql`] | |||
| * https://quarkus.io/extensions/io.quarkus/quarkus-messaging[`quarkus-messaging`] | |||
| * https://quarkus.io/extensions/io.quarkus/quarkus-mongodb-client[`quarkus-mongodb-client`] | |||
There was a problem hiding this comment.
In needs to be inserted above quarkus-messaging
| @@ -26,6 +26,11 @@ | |||
| <groupId>io.quarkus</groupId> | |||
| <artifactId>quarkus-vertx</artifactId> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>io.quarkus</groupId> | |||
| <artifactId>quarkus-mongodb-client</artifactId> | |||
There was a problem hiding this comment.
Looking at how messy the de code will be, with many property overrides, I think your original implementation is better and I was wrong when I asked you to move it to the OpenTelemetry extension.
Please ignore that request. Sorry.
| public CompletionStage<List<Book>> getBooksError() { | ||
| BsonDocument query = new BsonDocument(); | ||
| query.put("$invalidop", new BsonDouble(0d)); | ||
| return getCollection().find(query).collect().asList().subscribeAsCompletionStage(); |
There was a problem hiding this comment.
The context propagation will not happen when calling the MongoDB directly.
Please check the injected reactive client, it should provide the required propagation: https://quarkus.io/guides/mongodb#reactive.
There was a problem hiding this comment.
I've fixed reactive context propagation by a registering com.mongodb.reactivestreams.client.ReactiveContextProvider.
Otherwise we get circular dependency
Initially #40191 added mongo db commands to otel traces. This PR moves the implementation from mongodb-client extension to opentelemetry