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(service-provider-server): log more detailed information about DNS lookups #1191

Merged
merged 2 commits into from
Jan 17, 2022

Conversation

addaleax
Copy link
Contributor

In particular, include whether os-dns-native made lookups
using the native OS DNS APIs or not.

…S lookups

In particular, include whether os-dns-native made lookups
using the native OS DNS APIs or not.
@@ -42,7 +42,8 @@ const config = {
'mongodb-client-encryption': 'commonjs2 mongodb-client-encryption',
kerberos: 'commonjs2 kerberos',
snappy: 'commonjs2 snappy',
interruptor: 'commonjs2 interruptor'
interruptor: 'commonjs2 interruptor',
'os-dns-native': 'commonjs2 os-dns-native',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gribnoysup Does this look right to you? I’m guessing it’s enough to make this work in mongosh, since we also do the same for kerberos, but I’d want to be sure :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, looks good! I know that I said that we should externalize it here (and it's totally the correct thing to do) but what I forgot about is due the way we connect this runtime in Compass strictly speaking it's not even required as we connect the app through dataService first (where os-dns-native will already do all the work) and then we pass the driver connection options to this module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right -- but currently, we're still getting a "Missing optional dependency" warning fro mongosh in Compass because of this. I think it might be nice to avoid that :)

@addaleax addaleax merged commit 9b01e48 into main Jan 17, 2022
@addaleax addaleax deleted the surface-os-dns-native-lookupmode branch January 17, 2022 18:36
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.

2 participants