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

[towe75]

Proposal

As a nomad user i want to be able to have full control over my log files. I do not want to be forced to waste resources by running a idle "nomad logmon" process for each of my tasks.

Use-cases

Sometimes we want to use promtail or another log aggregator to ship logs to a central server. For some users it might even be enough to examine the local journal. In such cases it's quite some overhead to manage dual-logging incl. rotation etc.

I'm not arguing against the nomad'isch way of logging - it's very nice for some cases but the user should have a choice.

There seems to be no generic way to accomplish this by configuration means right now. Only the docker driver has a explicit "disable_log_collection" option. It's not possible for external driver plugins, like https://github.com/hashicorp/nomad-driver-podman , to hook into this mechanism AFAIK (experimental internal driver capabilities).

Also it might be desired to enable/disable log collection per task.

What about extending the existing logs stanza with e.g. a "Enabled = false" attribute. Alternatively we could also interpret a magic logs.max_files=0 value.
A optimal solution should respect the setting also in ui/cli instead of simply showing an empty tab etc.

I'm willing to contribute a PR if we can get to some conclusion here.

[lgfa29]

Hi @towe75 👋

This sounds like an useful option. I will bring up with the rest of the team and let you know 🙂

[lgfa29]

After checking with the rest of the team we think this would be a nice feature to have 🙂

Per-task config sounds good. One idea that was brought up was to potentially have a plugin-level config, perhaps adding a new capability so it becomes a generic mechanism for plugins to tell Nomad that logging is disabled.

I'm willing to contribute a PR if we can get to some conclusion here.

Thank you ❤️

Let us know if you have any questions!

[kolotaev]

I don't want to clog the discussion, but since there are not many comments indicating that this feature is awaited I want to add my +1 for its demand. Looks like it won't be super-hard to implement.

[towe75]

Any thoughts on configuration properties? Can i go with logs { max_files=0 } or is it better to add a new bool property. IMHO max_files=0 is not so bad because it makes it explicit that Nomad will no longer consume or rotate the logs.

[kolotaev]

To my mind Enabled = false would be more explicit. Thus you will be able to just say:

logs {
  enabled = false
}

for a task and not bother with any other options of this stanza.

[lgfa29]

I think I would lean towards enabled = false as well. Seems more consistent with other fields in Nomad.

Another small technical detail with using max_files = 0 is that in Go 0 is the zero-value of int, which is what we use in that field (https://github.com/hashicorp/nomad/blob/v1.1.6/nomad/structs/structs.go#L6611), so it would be impossible to distinguish between the zero-value and an explicit max_files = 0.

To work around this, we use pointers so we can have 0 meaning the user explicitly set a value of 0, and nilto represent the user didn't pass any value.

Using max_files = 0 would require changing the type of this field to maintain backwards-compatibility, and this would be extra work.

[towe75]

I spent some time on this issue and went for the Enabled=false approach. It was simple to not start/stop logmon based on the new config entry.

Flipping the option and updating the job is another story. I am unsure how to implement it - it's my first core contribution.
For now it tried to implement the TaskUpdateHook intferface for the logmon hook but it feels awkward.

Can we maybe enforce a alloc restart in the case that the logger is enabled/disabled? It would surely simplify the lifecycle management for logmon.

Also we have to decide what to present to the actual task executor instead of the fifo files. For non-windows we could use /dev/null, maybe.

@lgfa29 can you give me some advice?

[towe75]

Pushed a draft now, see #11415
It will force a destructive update if the new LogConfig.Enabled option is changed.

[kolotaev]

Hello,
@towe75, thank you for the Draft implementation! As per discussion in the Draft PR, looks like there aren't any huge restrictions/concerns for further converting it into the mainstream PR for merging it into master. Are there any plans for doing this?

[kolotaev]

One more note from my side, which I didn't realise prior to looking into the code (which you definitely already aware of). There's really a driver's config (as part of the InternalCapabilitiesDriver interface implementation for a particular driver) that can disable spawning logmon on a driver level, but for some reason it has been implemented only for docker driver. Are there any reasons for missing implementations for other core drivers: e.g. exec, raw_exec, etc.

If there aren't concerns for implementing it for other drivers, then it'd be nice to have it as well. Thus we'll allow 2 levels of enabling/disabling logmon: driver level -> task level which is actually convenient.

It should be addressed in a separate issue, of course :)

[tgross]

Implemented in #16962. Disabling collection at the task driver level is described in #15686 and #14636, and I'll follow-up with that work shortly.

[github-actions]

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.