Skip to content
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

feat(NODE-4696): add FaaS env information to client metadata #3626

Merged
merged 33 commits into from
Apr 12, 2023

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Apr 4, 2023

Description

What is changing?

  • Adds getFaasEnv() that gathers environment information as specified
  • Modifies makeClientMetadata to build the metadata document with an es map that checks the BSON size of every input preventing us from exceeding 512 bytes
    • Map offers a few conveniences over objects: key iteration, element count, set/delete is typical method syntax rather than bracket notation
  • Introduces an error if driverInfo properties name/version/platform use up the 512 bytes, this would prevent us from having a helloDoc.client.driver.name property which is required server side
  • if metadata document is at capacity, we will attempt to remove fields from os until it can fit, followed by the same logic for env
    • We do predict this is unlikely, but the hard restriction is that we never construct a document larger than 512 bytes.
Is there new documentation needed for these changes?

No mongodb manual changes.

I changed the API docs above MongoOptions, this is made public by the client.options getter and represents the parsed options, the type isn't necessarily an accurate representation of the runtime value because there are properties that are set after SRV lookup.

What is the motivation for this change?

FaaS environments have a unique impact on the driver's approach to it's distributed systems logic (SDAM). If we can better understand the environments & resources being used we can provide the best FaaS experience possible.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken nbbeeken added the wip label Apr 4, 2023
@nbbeeken
Copy link
Contributor Author

nbbeeken commented Apr 5, 2023

@nbbeeken nbbeeken force-pushed the NODE-4696-faas-metadata-handshake-5x-spec-updates branch from 9aee3fe to 871d208 Compare April 5, 2023 17:36
@nbbeeken nbbeeken changed the base branch from NODE-4696-faas-metadata-handshake-5x to main April 5, 2023 17:51
@nbbeeken nbbeeken changed the title refactor(NODE-4696): use an additive approach toward metadata limit feat(NODE-4696): add FaaS env information to client metadata Apr 5, 2023
@nbbeeken nbbeeken removed the wip label Apr 6, 2023
@nbbeeken nbbeeken marked this pull request as ready for review April 6, 2023 14:05
src/cmap/handshake/client_metadata.ts Show resolved Hide resolved
src/cmap/handshake/client_metadata.ts Outdated Show resolved Hide resolved
src/cmap/handshake/client_metadata.ts Outdated Show resolved Hide resolved
src/cmap/handshake/client_metadata.ts Show resolved Hide resolved
src/cmap/handshake/client_metadata.ts Show resolved Hide resolved
test/unit/cmap/handshake/client_metadata.test.ts Outdated Show resolved Hide resolved
test/unit/cmap/handshake/client_metadata.test.ts Outdated Show resolved Hide resolved
test/unit/cmap/handshake/client_metadata.test.ts Outdated Show resolved Hide resolved
@durran durran self-assigned this Apr 6, 2023
@durran durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Apr 6, 2023
@nbbeeken nbbeeken requested a review from durran April 6, 2023 15:20
@nbbeeken nbbeeken added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Apr 7, 2023
@dariakp dariakp self-requested a review April 7, 2023 15:11
src/cmap/handshake/client_metadata.ts Show resolved Hide resolved
test/tools/cmap_spec_runner.ts Outdated Show resolved Hide resolved
test/unit/cmap/handshake/client_metadata.test.ts Outdated Show resolved Hide resolved
test/unit/cmap/handshake/client_metadata.test.ts Outdated Show resolved Hide resolved
nbbeeken and others added 6 commits April 11, 2023 17:34
Co-authored-by: Daria Pardue <daria.pardue@mongodb.com>
Co-authored-by: Daria Pardue <daria.pardue@mongodb.com>
@nbbeeken nbbeeken requested a review from dariakp April 11, 2023 22:04
@durran
Copy link
Member

durran commented Apr 12, 2023

@dariakp Are your comments all adequately addressed? And @nbbeeken I think we have just one merge conflict now.

@durran durran merged commit 0424080 into main Apr 12, 2023
@durran durran deleted the NODE-4696-faas-metadata-handshake-5x-spec-updates branch April 12, 2023 16:00
W-A-James pushed a commit that referenced this pull request Apr 12, 2023
Co-authored-by: Bailey Pearson <bailey.pearson@mongodb.com>
Co-authored-by: Daria Pardue <daria.pardue@mongodb.com>
baileympearson added a commit that referenced this pull request Apr 13, 2023
Co-authored-by: Bailey Pearson <bailey.pearson@mongodb.com>
Co-authored-by: Daria Pardue <daria.pardue@mongodb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants