spring-projects / spring-data-mongodb

Provides support to increase developer productivity in Java when using MongoDB. Uses familiar Spring concepts such as a template classes for core API usage and lightweight repository style data access.
https://spring.io/projects/spring-data-mongodb/
Apache License 2.0
1.62k stars 1.09k forks source link

Add support for "comment suppliers" #4581

Open onmishkin opened 10 months ago

onmishkin commented 10 months ago

Although Spring Data MongoDB has support for MongoDB comments (e.g., Query.comment(String) and @Meta(comment = "...")) I'd like to see that support extended to support a use case I'd like to implement:

My application is a web server that uses "request IDs" that are generated by nginx on all incoming HTTP requests and then carried through all of my server's logic and across threads and in any remote requests it itself makes. Request IDs appear as part of all log messages. Request IDs are stored in thread-local storage (not passed parametrically). I want these request IDs to be passed to MongoDB as "comments" so I can associate any DB issues that MongoDB logs to also contain my request IDs. This will let me figure out what incoming request (and other associated activity) led to the DB issue.

Spring Data MongoDB's support for comments isn't general enough to support the above. One idea I have is to extend MongoTemplate to have some sort of setCommentSupplier(Supplier<String>) method. The supplied supplier would be called whenever Spring Data MongoDB is about to send a request to MongoDB. I'm sure there are other (and maybe better) ways to extend Spring Data MongoDB to support my use case too.

mp911de commented 2 months ago

I understand that you want to feed in contextual data (correlation-id) that is taken from an incoming request (such as a web-request) and then feed into the MongoDB request.

ThreadLocal's work as long as you're residing on the same thread, using asynchronous or reactive API's would break as the data isn't propagated across threads.

You might want to have a look at MongoObservationCommandListener that makes use of Micrometer's context-propagation for tracing identifiers. With a similar approach, you could associate contextual identifiers with the command attached to CommandStartedEvent.

onmishkin commented 2 months ago

ThreadLocal's work as long as you're residing on the same thread, using asynchronous or reactive API's would break as the data isn't propagated across threads.

RIght, I'm aware of this issue. In our system we work around it using a mechanism that ensures that the Runnables that we pass to thread creations capture various bits of relevant context (like the request ID) from the thread-local state of the thread that creates the Runnable; the mechanism's Runnable.run() method reinstates that context into the thread-local state of the new thread.

You might want to have a look at MongoObservationCommandListener that makes use of Micrometer's context-propagation for tracing identifiers. With a similar approach, you could associate contextual identifiers with the command attached to CommandStartedEvent.

I'm unclear on what you're suggesting. I dug around a bit and learned that the Mongo Java Driver has the ability to associate a ContextProvider with a connection (via a MongoClientSettings) and (I think) that that mechanism is exposed through Spring Data. But it looks to me like there's not a way to leverage any of that to affect what ultimately gets sent on the wire to the Mongo server--e.g., to include the "comment" in what's sent, which is my actual goal. Am I missing something?

mp911de commented 2 months ago

But it looks to me like there's not a way to leverage any of that to affect what ultimately gets sent on the wire to the Mongo server--e.g., to include the "comment" in what's sent, which is my actual goal. Am I missing something?

You're right; Amending the CommandStartedEvent.command doesn't affect the command being sent to the server. I was under the assumption that it would work.

That brings us back to the point where a generic command hook would be helpful. Only the driver team could provide such a hook.

On the Spring Data side, effectively, only queries and aggregations would be associated with a comment. All other commands (collection creation, index creation, distinct, estimated document count, …) would remain uncorrelated.

We would love to have a general mechanism for attaching out-of-band data with a MongoDB command request to propagate tracing identifiers in general; however, that would require changes within MongoDB and its protocol.

onmishkin commented 2 months ago

On the Spring Data side, effectively, only queries and aggregations would be associated with a comment. All other commands (collection creation, index creation, distinct, estimated document count, …) would remain uncorrelated.

I understand your point but FWIW I'd take that "incomplete" coverage over no coverage.

That brings us back to the point where a generic command hook would be helpful. Only the driver team could provide such a hook.

FYI, I also opened an issue with the MongoDB Java Driver team last year: JAVA-5260. They haven't said "No" yet :-)