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

Document Windows NPM setup and make sure it starts #761

Merged
merged 6 commits into from
Nov 17, 2022

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Nov 2, 2022

What does this PR do?

  • Add documentation about the need to use windows_npm_install when setting up NPM on Windows.
  • Restart Datadog Agent service on Windows on system probe configuration changes.

Motivation

  • Flag was undocumented and we did not mention this explicitly anywhere.
  • The NPM driver did not start since the system probe class was applied after the service start.

Additional Notes

It's difficult to do this automatically since we would have to call the datadog::datadog_agent class on the datadog::system_probe one, so instead we are just documenting this.

Describe your test plan

Docs only change.

@mx-psi mx-psi requested review from a team as code owners November 2, 2022 12:17
Copy link
Contributor

@buraizu buraizu left a comment

Choose a reason for hiding this comment

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

Approving, but left a small suggestion for consistency with other parameter descriptions

README.md Outdated Show resolved Hide resolved
mx-psi and others added 2 commits November 3, 2022 09:06
Co-authored-by: Bryce Eadie <bryce.eadie@datadoghq.com>
@mx-psi mx-psi mentioned this pull request Nov 17, 2022
@mx-psi mx-psi force-pushed the mx-psi/document-sysprobe-setup branch from 5e00d4f to 640f5ca Compare November 17, 2022 10:57
@mx-psi mx-psi changed the title Add documentation about Windows NPM setup Document Windows NPM setup and make sure it starts Nov 17, 2022
Copy link
Contributor

@KSerrania KSerrania left a comment

Choose a reason for hiding this comment

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

I've left some comments about the documentation, but otherwise this looks great!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@mx-psi mx-psi requested a review from KSerrania November 17, 2022 13:58
@mx-psi mx-psi merged commit 0f4573d into main Nov 17, 2022
@mx-psi mx-psi deleted the mx-psi/document-sysprobe-setup branch November 17, 2022 14:17
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.

3 participants