enhancement(vrl): added reverse_dns remap function #8717
Conversation
Rebases #5647 Signed-off-by: Jesse Szwedko <[email protected]>
|
✔️ Deploy Preview for vector-project canceled. 🔨 Explore the source changes: 2f0f62d 🔍 Inspect the deploy log: https://app.netlify.com/sites/vector-project/deploys/6154d077bbbfb90007da8384 |
|
Possibly relevant: DataDog/datadog-agent#7941 |
|
To what extent would it be possible to release this function as is, and keeping the internal caching mechanism as an improvement on the roadmap? The docs already make it clear that the function can slow things down, and then it's up to the end user whether the performance penalty is acceptable or not. |
|
Hi @jeromekleinen-kbc . We discussed this and would like to implement a function local cache before releasing. We realize we could just document it is slow, but we have some concerns about it being a bit of a slippery slope for additional functions and it hurting the user experience for VRL. However, I think we can merge this without the documentation for now so that you don't need to maintain a custom build and we can add the caching later. |
Signed-off-by: Jesse Szwedko <[email protected]>
Signed-off-by: Jesse Szwedko <[email protected]>
Signed-off-by: Jesse Szwedko <[email protected]>
Signed-off-by: Jesse Szwedko <[email protected]>
|
It appears the |
| want: Ok(value!("localhost")), | ||
| } | ||
|
|
||
| google { |
There was a problem hiding this comment.
This could be fairly brittle as it relies on a number of external factors. It's probably important to bench this though, so I think we should keep it.
I wonder if 8.8.8.8 is the best to use since a lot of systems may be setup to use it for DNS so it may already be cached by the OS.
There was a problem hiding this comment.
Yeah, true, though these DNS resolvers IPs also seem to be the least likely to change 😄 Also, if the OS is caching these lookups, it'll be cached after the first benchmark iteration anyway?
lib/vrl/stdlib/src/reverse_dns.rs
Outdated
| tdef: TypeDef::new().fallible().bytes(), | ||
| } | ||
|
|
||
| localhost { |
There was a problem hiding this comment.
Is it worth putting in a test for ::1?
There was a problem hiding this comment.
I ended up dropping the localhost tests since Windows doesn't return localhost. I did add an IPv6 test for dns.google though in 6fe11b7
Awesome, works for me! :) |
…on Windows Signed-off-by: Jesse Szwedko <[email protected]>
Signed-off-by: Jesse Szwedko <[email protected]>
Signed-off-by: Jesse Szwedko <[email protected]>
Rebases #5647
Re-opening as a draft for discussion. We have two users interested in seeing this introduced.
Local benchmarks show this at ~200 us for me. It does seem like we'll want caching here. I'm wondering if we could just do a function local cache for now until we are able to introduce a more comprehensive VRL cache. Thoughts @JeanMertz @StephenWakely ?
Signed-off-by: Jesse Szwedko [email protected]