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

Flow Windows service: Support environment variables #5762

Merged
merged 15 commits into from
Nov 22, 2023

Conversation

jkroepke
Copy link
Contributor

@jkroepke jkroepke commented Nov 13, 2023

PR Description

Windows support environments for global, per user and per service. All three os native methods have downsides

  • Global scope exposing environment variables to all program
  • Per User: It's easy for human users to change own environment variables, but hard too for system users.
  • Per Service: If the service is using a local system account, then environment variables will be read once from registry. Changes requires a reboot.

I copied the replicate the Arguments logic for Environment. Environment are needed to inject configuration from outside. Define configuration via CLI argument is not supported (see: grafana/alloy#308)

Which issue(s) this PR fixes

Notes to the Reviewer

Tested on Windows Server 2022.

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

Signed-off-by: Jan-Otto Kröpke <[email protected]>
@jkroepke jkroepke requested review from clayton-cornell and a team as code owners November 13, 2023 21:19
@mattdurham mattdurham self-assigned this Nov 13, 2023
cmd/grafana-agent-service/config_windows.go Outdated Show resolved Hide resolved
Comment on lines +66 to +67
* `Arguments` (Type `REG_MULTI_SZ`) Each value represents a binary argument for grafana-agent-flow binary.
* `Environment` (Type `REG_MULTI_SZ`) Each value represents a environment value `KEY=VALUE` for grafana-agent-flow binary.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think people might find it useful if we include a regedit printout of an example configuration. I don't insist on it, will leave it up to you to decide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the output of reg query ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess so? I've only used reg export and I suppose reg query does a similar job. As long as people can see the values in a format they can recognise, it's ok.

Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

Thank you!

cmd/grafana-agent-service/config_windows.go Outdated Show resolved Hide resolved
cmd/grafana-agent-service/config_windows.go Show resolved Hide resolved
@jkroepke
Copy link
Contributor Author

Is there anything from my side todo here?

Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

Looking good from my end

@jkroepke
Copy link
Contributor Author

I fixed the Changelog entry, since 0.38 is released.

@mattdurham mattdurham enabled auto-merge (squash) November 22, 2023 18:52
@mattdurham mattdurham merged commit 8f5a8aa into grafana:main Nov 22, 2023
8 checks passed
@jkroepke jkroepke deleted the win-environment-service branch November 22, 2023 22:12
@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Nov 23, 2023
hainenber pushed a commit to hainenber/agent that referenced this pull request Nov 25, 2023
* Implement windows service environment variabls

Signed-off-by: Jan-Otto Kröpke <[email protected]>

* Changelog

Signed-off-by: Jan-Otto Kröpke <[email protected]>

* Add NSIS argument

* Always override Environment, since its an expected user input

Signed-off-by: Jan-Otto Kröpke <[email protected]>

* Update cmd/grafana-agent-service/config_windows.go

Co-authored-by: Robert Fratto <[email protected]>

* Update cmd/grafana-agent-service/config_windows.go

* Update docs/sources/flow/setup/install/windows.md

Co-authored-by: Clayton Cornell <[email protected]>

* Update CHANGELOG.md

---------

Signed-off-by: Jan-Otto Kröpke <[email protected]>
Co-authored-by: mattdurham <[email protected]>
Co-authored-by: Robert Fratto <[email protected]>
Co-authored-by: Clayton Cornell <[email protected]>
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 21, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants