Skip to content

feat(mongodb): Add MongoDB client handshake metadata #625

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

alexbevi
Copy link

@alexbevi alexbevi commented Apr 23, 2025

The PR addresses #626 by incorporating MongoDB's wrapping client library specification for the connection handshake to allow library details to be included in the metadata written to mongos or mongod logs.

For example, this change would allow server-side logs such as the following:

{"t":{"$date":"2024-01-27T23:10:40.108Z"},"s":"I","c":"NETWORK","id":51800,"ctx":"conn16235","msg":"client metadata","attr":{"remote":"127.0.0.1:1094","client":"conn16235","doc":{"driver":{"name":"nodejs|unstorage","version":"4.16.0"},"platform":"Node.js v18.18.2, LE","os":{"name":"linux","architecture":"x64","version":"5.15.133+","type":"Linux"}}}}

For anyone hosting clusters with connections coming from different applications this can help differentiate connections and facilitate log analysis.

@alexbevi alexbevi requested a review from pi0 as a code owner April 23, 2025 19:50
@alexbevi alexbevi changed the title feat(mongodb):Add MongoDB client handshake metadata feat(mongodb): Add MongoDB client handshake metadata Apr 23, 2025
mongoOptions.driverInfo = {
name: "unstorage",
};
const mongoClient = new MongoClient(opts.connectionString, mongoOptions);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should instead support generic mongoOptions as a driver option so you can set it to { driverInfo: { name: "..." } } based on your needs

Copy link
Member

@43081j 43081j Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i agree. we could default if it helps:

// in the top of the file
const defaultMongoOptions: MongoClientOptions = {
  driverInfo: {
    name: "unstorage"
  }
};

// here (where i'm commenting in this PR)
const mongoOptions: MongoClientOptions = {
  ...defaultMongoOptions, // don't bother deep-merging, people can override `driverInfo` as a whole
  ...opts.clientOptions
};

then update the MongoDbOptions to have:

clientOptions?: MongoClientOptions;

and either way, we should strongly type this instead of any

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestions! I've worked these into the PR

Co-authored-by: James Garbutt <[email protected]>
@43081j
Copy link
Member

43081j commented Apr 24, 2025

Looks good to me 👍

@pi0 I'll leave you to merge once you've had time to read through

Copy link

codecov bot commented Apr 24, 2025

Codecov Report

Attention: Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.

Project coverage is 59.90%. Comparing base (70310f9) to head (b0cd37a).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
src/drivers/mongodb.ts 54.54% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #625      +/-   ##
==========================================
- Coverage   59.99%   59.90%   -0.10%     
==========================================
  Files          42       42              
  Lines        3657     3736      +79     
  Branches      590      600      +10     
==========================================
+ Hits         2194     2238      +44     
- Misses       1460     1495      +35     
  Partials        3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants