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

[NDM] Add netflow troubleshooting packet drops section #23962

Merged
merged 15 commits into from
Jul 15, 2024

Conversation

TCheruy
Copy link
Contributor

@TCheruy TCheruy commented Jul 2, 2024

What does this PR do? What is the motivation?

Add a section to Netflow docs on troubleshooting UDP packet drops
Preview here
NDMII-2844

Merge instructions

  • Please merge after reviewing

Additional notes

Copy link
Contributor

github-actions bot commented Jul 2, 2024

Preview links (active after the build_preview check completes)

Modified Files

@TCheruy TCheruy marked this pull request as ready for review July 2, 2024 12:39
@TCheruy TCheruy requested a review from a team as a code owner July 2, 2024 12:39
@hestonhoffman hestonhoffman added the editorial review Waiting on a more in-depth review label Jul 2, 2024
@hestonhoffman
Copy link
Contributor

Opened DOCS-8368 for someone on the docs team to take a look

Copy link
Contributor

@aliciascott aliciascott left a comment

Choose a reason for hiding this comment

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

Hi @TCheruy thanks for adding this! I have a few suggestions (we want to keep NetFlow as camel case to align with how it is in the app and the rest of the document), a few other minor things , also can you indent the steps (# 1, # 2, etc), 2 spaces to the right and the code samples need indented 4 spaces to the right for formatting so they align? Here is an example of how that would look:

https://docs.datadoghq.com/network_monitoring/devices/guide/device_profiles/#add-tabular-tags

Also you need to click the button to merge master into this branch as there is now a merge conflict (which was introduced by me in another PR, sorry about that!)

content/en/network_monitoring/devices/netflow.md Outdated Show resolved Hide resolved
content/en/network_monitoring/devices/netflow.md Outdated Show resolved Hide resolved
content/en/network_monitoring/devices/netflow.md Outdated Show resolved Hide resolved
content/en/network_monitoring/devices/netflow.md Outdated Show resolved Hide resolved
content/en/network_monitoring/devices/netflow.md Outdated Show resolved Hide resolved
content/en/network_monitoring/devices/netflow.md Outdated Show resolved Hide resolved
content/en/network_monitoring/devices/netflow.md Outdated Show resolved Hide resolved
content/en/network_monitoring/devices/netflow.md Outdated Show resolved Hide resolved
content/en/network_monitoring/devices/netflow.md Outdated Show resolved Hide resolved
Copy link
Contributor

@aliciascott aliciascott left a comment

Choose a reason for hiding this comment

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

Hi @TCheruy thanks for the updates, looks good! did you want to adjust that one sentence per @FlorianVeaux comment to state it's specific to Linux only? I am fine with it either way, let me know and then I can get this merged for you!

@TCheruy
Copy link
Contributor Author

TCheruy commented Jul 9, 2024

Thanks a lot @aliciascott ! just pushed 8bd8371 to specify that it's linux only, let me know what you think 🙇‍♂️

@aliciascott
Copy link
Contributor

Thanks a lot @aliciascott ! just pushed 8bd8371 to specify that it's linux only, let me know what you think 🙇‍♂️

Looks good! @TCheruy let me know if this is ok to go ahead and merge ?

@TCheruy
Copy link
Contributor Author

TCheruy commented Jul 9, 2024

@aliciascott Let's wait for a final review from Florian when he's back on Friday, will let you know when we're good 🙇‍♂️

@aliciascott aliciascott merged commit 5774a1f into master Jul 15, 2024
15 checks passed
@aliciascott aliciascott deleted the ThibaudC/netflow-troubleshoot-packet-drops branch July 15, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial review Waiting on a more in-depth review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants