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-5188): add alternative runtime detection to client metadata #3636

Merged
merged 8 commits into from Apr 14, 2023

Conversation

nbbeeken
Copy link
Contributor

Description

What is changing?

Changes platform metadata based on globals in the driver's runtime environement.

What is the motivation for this change?

Node.js compatibility in Deno and Bun have made it possible to run the driver, it is useful to denote the difference between the runtimes in the platform information.

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 requested a review from alexbevi April 12, 2023 19:45
@nbbeeken
Copy link
Contributor Author

Server log lines:

{"t":{"$date":"2023-04-12T15:53:58.738-04:00"},"s":"I",  "c":"NETWORK",  "id":51800,   "ctx":"conn6","msg":"client metadata","attr":{"remote":"127.0.0.1:64794","client":"conn6","doc":{"driver":{"name":"nodejs","version":"5.2.0"},
"platform":"Deno v1.32.3, LE",
"os":{"name":"darwin","architecture":"arm64","version":"22.4.0","type":"Darwin"}}}}

{"t":{"$date":"2023-04-12T16:03:31.216-04:00"},"s":"I",  "c":"NETWORK",  "id":51800,   "ctx":"conn18","msg":"client metadata","attr":{"remote":"127.0.0.1:59421","client":"conn18","doc":{"driver":{"name":"nodejs","version":"5.2.0"},
"platform":"Bun v0.5.4, LE",
"os":{"name":"darwin","architecture":"arm64","version":"22.4.0","type":"Darwin"}}}}

alexbevi
alexbevi previously approved these changes Apr 12, 2023
Copy link
Contributor

@alexbevi alexbevi left a comment

Choose a reason for hiding this comment

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

LGTM

@durran durran self-assigned this Apr 13, 2023
@durran durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Apr 13, 2023
src/cmap/handshake/client_metadata.ts Outdated Show resolved Hide resolved
src/cmap/handshake/client_metadata.ts Outdated Show resolved Hide resolved
@nbbeeken nbbeeken requested a review from durran April 13, 2023 15:43
@nbbeeken nbbeeken requested a review from durran April 13, 2023 18:33
@durran durran merged commit 2a26de3 into main Apr 14, 2023
16 of 21 checks passed
@durran durran deleted the NODE-5188-deno-bun branch April 14, 2023 12:59
nbbeeken added a commit that referenced this pull request Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
4 participants