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

logs: allow disabling log collection in jobspec #16962

Merged
merged 1 commit into from
Apr 24, 2023
Merged

logs: allow disabling log collection in jobspec #16962

merged 1 commit into from
Apr 24, 2023

Conversation

tgross
Copy link
Member

@tgross tgross commented Apr 21, 2023

Some Nomad users ship application logs out-of-band via syslog or the like. For these users having logmon (and docker_logger) running is unnecessary overhead. Allow disabling the logmon and pointing the task's stdout/stderr to /dev/null.

This changeset is the first of several incremental improvements to log collection short of full-on logging plugins. The next step will likely be to extend the internal-only task driver configuration so that cluster administrators can turn off log collection for the entire driver (as described in #15686 and #14636)


Fixes: #11175

Notes to reviewers:

  • In addition to the unit tests here, I've verified this works as expected with docker and raw_exec tasks, with and without the logs.enabled set, including requiring destructive updates to the alloc if we change the value.
  • Although we have an existing disable_logging_collection field for the driver, I went with enabled here to be more consistent with other jobspec fields and our inclusive language guidelines. I suspect most folks won't touch it, in any case.
  • Once this is merged I'll open a follow-up issue for disabling the logs UI if LogConfig.Enabled = false. But I've verified it doesn't cause any weird crashing behavior, the panel is just empty.

@tgross tgross force-pushed the null-logger branch 2 times, most recently from d3e9c5a to 0bf0a91 Compare April 21, 2023 18:16
@tgross tgross added this to the 1.5.x milestone Apr 21, 2023
@tgross tgross marked this pull request as ready for review April 21, 2023 20:44
@tgross tgross requested review from jrasell and schmichael April 21, 2023 20:44
.changelog/16962.txt Outdated Show resolved Hide resolved
client/allocrunner/taskrunner/logmon_hook.go Show resolved Hide resolved
@@ -3,7 +3,7 @@ layout: docs
page_title: logs Block - Job Specification
description: |-
The "logs" block configures the log rotation policy for a task's stdout and
stderr. Logging is enabled by default with sane defaults. The "logs" block
stderr. Logging is enabled by default with reasonable defaults. The "logs" block
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@jrasell jrasell left a comment

Choose a reason for hiding this comment

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

LGTM; just a question inline which is only because I don't know the answer.

client/allocrunner/taskrunner/logmon_hook.go Outdated Show resolved Hide resolved
client/allocrunner/taskrunner/logmon_hook.go Outdated Show resolved Hide resolved
client/allocrunner/taskrunner/logmon_hook.go Outdated Show resolved Hide resolved
Some Nomad users ship application logs out-of-band via syslog. For these users
having `logmon` (and `docker_logger`) running is unnecessary overhead. Allow
disabling the logmon and pointing the task's stdout/stderr to /dev/null.

This changeset is the first of several incremental improvements to log
collection short of full-on logging plugins. The next step will likely be to
extend the internal-only task driver configuration so that cluster
administrators can turn off log collection for the entire driver.

---

Fixes: #11175
Co-authored-By: Thomas Weber <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not spawn logmon if user wants to collect/ship log via other tools
3 participants