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

try if trust-dns-resolver works or not #190

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Cindia-blue
Copy link

No description provided.

@Cindia-blue Cindia-blue requested a review from PiotrSikora as a code owner March 5, 2023 04:44
Copy link
Member

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

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

I'm generally fine with adding examples like that, but could you create a new example instead of overloading the existing one with unrelated test code?

Also, the outgoing DNS request won't be sent, since Proxy-Wasm doesn't yet allow TCP/UDP sidecalls. Did you try to use this code in Envoy or another host implementation?

Comment on lines +85 to +86
let io_loop = Runtime::new().unwrap();
let lookup = io_loop.block_on(lookup).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a bad idea, since each worker thread (in case of Envoy) is going to be blocked on this, and DNS resolution can potentially take hundreds of milliseconds... not to mention the overhead of starting Tokio runtime per each request.

Copy link
Author

Choose a reason for hiding this comment

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

Can we have a meeting about this? Can we mitigate the overhead per request by cache layer?

Copy link
Member

Choose a reason for hiding this comment

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

You could store the runtime in a global variable and share it across the callbacks, but I suspect that Tokio runtime couldn't make any progress without deep integration with Proxy-Wasm's callbacks.

Also, if you try to compile your example, you can see that Tokio async doesn't work in Wasm (yet):

compile_error!("Only features sync,macros,io-util,rt,time are supported on wasm.");

What are you trying to achieve? What's your end goal?

Copy link
Author

@Cindia-blue Cindia-blue Mar 10, 2023

Choose a reason for hiding this comment

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

yup... I can not compile through for that reason. I am trying to get cname chain in WASM. Could you recommend some feasible way? I saw there are some other lib besides Tokio, like RustDNS maybe others but not sure if they have similar issue. It will be nice if you can suggest

Copy link
Member

@PiotrSikora PiotrSikora Mar 14, 2023

Choose a reason for hiding this comment

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

Are you interested only in the final answer or the whole CNAME chain? If the former, and you're using Envoy, then there are envoy_resolve_dns / envoy_on_resolve_dns extensions that you could call and use Envoy's built-in DNS resolver... Note that they are not exposed in the Rust SDK yet.

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