-
Notifications
You must be signed in to change notification settings - Fork 245
DRIVERS-2985: Allow on-demand client metadata updates after MongoClient initialization. #1798
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
Changes from 6 commits
026c92b
a9eddb4
324019d
ab28ac3
03b5c1d
450528c
5ce8b92
c6a042b
e02503b
1faef56
8fc6f1c
c572c4d
0b7fbfa
dfde2d1
9f58bfb
b4a4b0e
4be80aa
f0315f4
da284da
72f1150
ae795a6
a34752b
43dea26
944329d
37456ca
c77aa66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -405,15 +405,25 @@ class DriverInfoOptions { | |
} | ||
``` | ||
|
||
Note that how these options are provided to a driver is left up to the implementer. | ||
Note that how these options are provided to a driver during `MongoClient` initialization is left up to the implementer. | ||
|
||
If provided, these options MUST NOT replace the values used for metadata generation. The provided options MUST be | ||
appended to their respective fields, and be delimited by a `|` character. For example, when | ||
### Metadata updates after MongoClient initialization | ||
|
||
Drivers MUST provide an API that allows to append `DriverInfoOptions` to a MongoClient instance after initialization. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "...that allows to append..." reads weirdly to me? not sure if its just me or not though. It's possible that my brain just isn't parsing it correctly? I think my brain wants it to be "...that allows to append..." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, you're right,
Did you mean something like 'that allows appending' or 'that allows [users/wrapping libraries] to append'? I currently changed it to Changed in 5ce8b92 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this is much better. (upon re-reading, realized my suggestion at the end makes no sense because i wrote the same thing HAHA sorry for the confusion) thanks! |
||
|
||
After client metadata update, drivers MUST apply updated metadata to newly created connections and MUST NOT apply it to | ||
already established connections. | ||
|
||
### Appending metadata | ||
|
||
If `DriverInfoOptions` are provided during or after MongoClient initialization, these options MUST NOT replace any | ||
existing metadata values, including driver-generated metadata and previously provided options. The provided options MUST | ||
be appended to their respective fields, and be delimited by a `|` character. For example, when | ||
[Motor](https://www.mongodb.com/docs/drivers/motor/) wraps PyMongo, the following fields are updated to include Motor's | ||
"driver info": | ||
|
||
```typescript | ||
{ | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this white space change intentional? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed in 5ce8b92, thanks! |
||
client: { | ||
driver: { | ||
name: "PyMongo|Motor", | ||
|
@@ -534,6 +544,7 @@ support the `hello` command, the `helloOk: true` argument is ignored and the leg | |
|
||
## Changelog | ||
|
||
- 2025-05-07: Add requirement to allow appending to client metadata after MongoClient initialization. | ||
- 2024-11-05: Move handshake prose tests from spec file to prose test file. | ||
- 2024-10-09: Clarify that FaaS and container metadata must both be populated when both are present. | ||
- 2024-08-16: Migrated from reStructuredText to Markdown. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,3 +82,89 @@ the following sets of environment variables: | |
2. Create and connect a `Connection` object that connects to the server that returns the mocked response. | ||
|
||
3. Assert that no error is raised. | ||
|
||
## Client Metadata Update Prose Tests | ||
|
||
The driver **MAY** implement the following tests. Drivers that do not emit events for commands issued as part of the | ||
jyemin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
handshake with the server will need to create a test-only backdoor mechanism to intercept the handshake `hello` command | ||
for verification purposes. | ||
|
||
### Test 1: Test that the driver updates metadata | ||
|
||
Drivers should verify that metadata provided after `MongoClient` initialization is appended, not replaced, and is | ||
visible in the `hello` command of new connections. | ||
|
||
1. Create a `MongoClient` instance with the following: | ||
|
||
- `maxIdleTimeMS` set to `1ms` | ||
|
||
- Wrapping library metadata: | ||
|
||
| Field | Value | | ||
| -------- | ---------------- | | ||
| name | library | | ||
| version | 1.2 | | ||
| platform | Library Platform | | ||
|
||
2. Send a `ping` command to the server and verify: | ||
|
||
- The command succeeds. | ||
- The wrapping library metadata is appended to the respective `client.driver` fields of the `hello` command. | ||
- Save intercepted `client` document as `initialClientMetadata`. | ||
|
||
3. Wait 5ms for the connection to become idle. | ||
|
||
4. Append the following `DriverInfoOptions` to the `MongoClient` metadata: | ||
|
||
| Field | Value | | ||
| -------- | ------------------ | | ||
| name | framework | | ||
| version | 2.0 | | ||
| platform | Framework Platform | | ||
|
||
5. Send `ping` command to the server and verify: | ||
|
||
- The command succeeds. | ||
|
||
- The framework metadata is appended to the existing `DriverInfoOptions` in the `client.driver` fields of the `hello` | ||
command. | ||
|
||
| Field | Value | | ||
| -------- | ------------------------------------ | | ||
| name | library\|framework | | ||
| version | 1.2\|2.0 | | ||
| platform | Library Platform\|Framework Platform | | ||
|
||
- All other subfields in the client document remain unchanged from `initialClientMetadata`. | ||
|
||
### Test 2: Test that metadata is not updated on established connections | ||
|
||
Drivers should verify that appending metadata after `MongoClient` initialization does **not** close existing | ||
connections, and that no new `hello` command is sent. | ||
|
||
1. Create a `MongoClient` instance with wrapping library metadata: | ||
|
||
| Field | Value | | ||
| -------- | ---------------- | | ||
| name | library | | ||
| version | 1.2 | | ||
| platform | Library Platform | | ||
|
||
2. Send a `ping` command to the server and verify: | ||
|
||
- The command succeeds. | ||
- The wrapping library metadata is appended to the respective `client.driver` fields of the `hello` command. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we simplify this test by removing the "library metadata" in step 1 and removing this assert from step 2? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch - I missed removing the assertions in test 3. Removed assertions and library metadata in f0315f4, thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, couldn't this particular test be a unified spec test? It doesn't rely on any mocking. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, good point - it makes sense to convert this to a unified test. I’ve updated it accordingly. I also added an assertion to verify no new connection is created. Since unified tests don’t assume a test backdoor, we can’t directly assert on handshake commands. However, verifying that no new connection is established indirectly ensures no hello was sent during the handshake. Any subsequent hello's over an existing connection would appear as regular commands and cause the test to fail. |
||
|
||
3. Append the following `DriverInfoOptions` to the `MongoClient` metadata: | ||
|
||
| Field | Value | | ||
| -------- | ------------------ | | ||
| name | framework | | ||
| version | 2.0 | | ||
| platform | Framework Platform | | ||
|
||
4. Send another `ping` command to the server and verify: | ||
|
||
- The command succeeds. | ||
- No `hello` command is sent. | ||
- No `ConnectionClosedEvent` is emitted. |
Uh oh!
There was an error while loading. Please reload this page.