[NDMII-3058] Add synchronous rDNS lookup to rdnsquerier component #30002
+430
−58
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vicweiss
changed the title
vicweiss
commented
Oct 9, 2024
pkg/collector/corechecks/snmp/internal/report/report_device_metadata.go
Outdated
Show resolved
Hide resolved
Regression Detector |
vicweiss
commented
Oct 10, 2024
GustavoCaso
reviewed
Oct 11, 2024
pkg/collector/corechecks/snmp/internal/report/report_device_metadata.go
Outdated
Show resolved
Hide resolved
Test changes on VMUse this command from test-infra-definitions to manually test this PR changes on a VM: inv create-vm --pipeline-id=47487109 --os-family=ubuntuNote: This applies to commit c704f60 |
vicweiss
changed the title
jmw51798
reviewed
Oct 25, 2024
vickenty
dismissed
their stale review
This PR no longer includes parts that were requested to change
ken-schneider
dismissed
buraizu’s stale review
The part of this PR that adds this to SNMP has been removed, this piece doesn't need a release note.
jmw51798
approved these changes
Oct 25, 2024
|
/merge |
|
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR modifies the reverse DNS querier component to allow for synchronous calls.
Motivation
While this component was originally written with Netflow in mind, hence the asynchronous implementation, having a reverse DNS lookup with caching is also desired for 2 other use cases:
Describe how to test/QA your changes
Ensure that the asynchronous call still works and Netflow reverse DNS data is still being fetched properly. Changes to that were limited to renaming the function call.
Currently there's not a good way to QA the synchronous endpoint as this is just the addition of the synchronous call. Separate PRs will be created that actually use this code and impact the functionality of the agent.
Possible Drawbacks / Trade-offs
This isn't a perfect implementation. With more time, I would investigate refactoring the cache implementation to be more friendly to a synchronous approach. This would allow us to avoid having all of the channels to receive results.
Additional Notes