libpod: Implement --log-opt label=LABEL=Value

[p12tic]

Currently, Compose on Podman has a significant limitation compared to Compose on Docker: there's no way to include container labels such as the Compose project name in the logs. This issue exists regardless of the underlying Compose provider.

This PR implements support for passing container labels to conmon via new --log-label option. The container labels are supported only on journald log driver and are exposed via --log-opt label=KEY=VALUE syntax. The option can be repeated multiple times to set multiple labels.

Does this PR introduce a user-facing change?

The `--log-opt` option for `podman create` and `podman run` now supports the `label` option to set additional labels that are attached to the log messages when using journald driver

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: p12tic
Once this PR has been reviewed and has the lgtm label, please assign l0rd for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[p12tic]

Tests are failing due to --log-label not being supported in conmon yet.

As for minimum required conmon version, some code existed to check that, but it was removed in 20ad122. I think proper solution would be to resurrect part of it and disable the new code path if old conmon is detected.

[Luap99]

What is your actual end goal here? Why would you want the labels as part of every single journald message. That seems wasteful in terms of IO and storage. What does docker compose do? Also adding labels like this means you have a hard dependency on the journald logging driver while that is normally something that is configured by end users so I doubt podman-compose would want to have a hard dependency on a particular log driver?

Doing version detection is generally something we avoid because it adds additional overhead to every single container start.

[p12tic]

Thanks for your comments. There are many questions, I will try to answer to each separately:

What does docker compose do?

Docker/podman compose is a tool to orchestrate running multiple related local containers. It can be thought as a very simple single node kubernetes.

What is your actual end goal here?

The end goal is to be able to understand container logs in more complex scenarios.

Docker supports logging drivers which allows to ship logs from multiple hosts to a single host for processing and analysis, using tools like Grafana Loki. There's no such thing on Podman. The best what exists is to ship logs to journald and then read them using daemons such as Promtail which ship the logs to the host that does log processing.

Unfortunately even in this case current capability is insufficient. A single application is composed of multiple containers most of the time and it's useful to be able to see all logs from a particular application. Unfortunately, currently only container ID and name are available in journald, so this is not possible. Having container labels allows to access labels such as com.docker.compose.project and com.docker.compose.service and then group logs based on that.

Why would you want the labels as part of every single journald message.

This allows messages to be associated with a particular grouping. Note that there are already 28 fields associated with each single journald message coming out of conmon already. These are all normal fields in a journald message, I already excluded the fields that are handled in a special way such as timestamps.

That seems wasteful in terms of IO and storage.

Journald does string interning, so repeating log labels have very small incremental cost. Note how many fields are always included already.

However I agree that it probably does not make sense to include all labels. Probably it makes sense to configure that how log tag can be configured via --log-opt tag=....

Also adding labels like this means you have a hard dependency on the journald logging driver while that is normally something that is configured by end users so I doubt podman-compose would want to have a hard dependency on a particular log driver?

The logging drivers expose different data in the logs already. Journald provides most complete logging information, so adding labels to it does not make it a more hard dependency compared to how it is already.

Doing version detection is generally something we avoid because it adds additional overhead to every single container start.

Agreed, this is a very strong argument why this should not be enabled by default.

[p12tic]

List of journald fields coming out of conmon by default: : _EXE CONTAINER_ID _HOSTNAME MESSAGE _TRANSPORT CODE_FILE _AUDIT_SESSION _SOURCE_REALTIME_TIMESTAMP _GID _SYSTEMD_SLICE _SELINUX_CONTEXT PRIORITY _RUNTIME_SCOPE _UID CONTAINER_NAME CODE_FUNC CODE_LINE _SYSTEMD_INVOCATION_ID _CMDLINE _SYSTEMD_CGROUP _MACHINE_ID SYSLOG_IDENTIFIER _PID _SYSTEMD_UNIT _AUDIT_LOGINUID _COMM _CAP_EFFECTIVE CONTAINER_ID_FULL

[Luap99]

What does docker compose do?

Docker/podman compose is a tool to orchestrate running multiple related local containers. It can be thought as a very simple single node kubernetes.

My question was about how docker labels the logs? Dumping all labels by default to each log line seems really wasteful.

 Journald does string interning, so repeating log labels have very small incremental cost. Note how many fields are always included already.

Oh that was a misunderstanding on my part, I thought the log is written append only line by line basically but it seems there is quite some logic in there to link to previous entries via hash tables so I guess you are right it would not be so bad.

However I agree that it probably does not make sense to include all labels. Probably it makes sense to configure that how log tag can be configured via --log-opt tag=....

I think that would make the most sense, if we have --log-opt label=key=val then it would be up to the end user to decide what gets put into the journal and what not. And then there is also no need to feature check anything as we can just error if conmon doesn't have the option.

Also adding labels like this means you have a hard dependency on the journald logging driver while that is normally something that is configured by end users so I doubt podman-compose would want to have a hard dependency on a particular log driver?

The logging drivers expose different data in the logs already. Journald provides most complete logging information, so adding labels to it does not make it a more hard dependency compared to how it is already.

From how I understood your PR description I thought your plan was to use that in podman-compose directly and depend on the labels in journald somehow for some feature?! If that is not the case sure then it is nor a problem.

[p12tic]

My question was about how docker labels the logs? Dumping all labels by default to each log line seems really wasteful.

Sorry, misunderstanding on my side.

Docker itself does not need to log labels for its own purposes because it just need to know which container a log came from and then attach labels according to that. The interesting part becomes when the logs are shipped to external log systems. This is implemented by docker logging drivers. For example, Grafana Loki driver will ship the values of com.docker.swarm.service.name, com.docker.stack.namespace, com.docker.compose.service, com.docker.compose.project if present.

I think that would make the most sense, if we have --log-opt label=key=val then it would be up to the end user to decide what gets put into the journal and what not. And then there is also no need to feature check anything as we can just error if conmon doesn't have the option.

Sounds like a plan. I will get back with WIP implementation.

From how I understood your PR description I thought your plan was to use that in podman-compose directly and depend on the labels in journald somehow for some feature?! If that is not the case sure then it is nor a problem.

Indeed, this is correct. The only plan is to allow users to configure centralized logging.

[mheon]

We are getting a number of complaints about Podman being overly spammy in the journal logs, so while I'm not opposed to this, I'd want a way of turning it off (the --log-opt flag seems most logical, but we'd need to pipe that into containers.conf as well so it could be set via config file... that could be done outside of this PR though).

[Luap99]

but we'd need to pipe that into containers.conf as well so it could be set via config file... that could be done outside of this PR though)

I am not to sure we need this a containers.conf option. I rather wait for some use case for this rather than adding any possible option in containers.conf which always complicates things.

[p12tic]

Docker supports setting a list of interesting labels to be always logged as part of their journald logging driver https://docs.docker.com/engine/logging/drivers/journald/. These are both settable from per-container and global configuration. I agree that this is out of scope of this PR.

[p12tic]

@Luap99 @mheon I updated PR with explicit --log-opt label=KEY=VALUE option. VALUE now can be rendered just like VALUE in --log-opt tag=VALUE can.

I chose exposing this as repeating --log-opt label=KEY=VALUE instead of single --log-opt labels=KEY=VALUE,KEY2=VALUE2 option because the latter introduces more complex parsing and additional limitations on what symbols the values may contain. Right now everything after the first = is a value. Supplying multiple labels via single option causes values not being able to contain at least = and , characters. The downside of the current approach is that labels need to be parsed earlier into a separate map within LogConfig.

[p12tic]

Currently tests fail due to new Podman logs [It] using journald labels test expecting a new conmon.

[p12tic]

In the end I also implemented containers.conf option, as the implementation was just 20 lines of code excluding tests. At least for me the use case would be to avoid setting `--log-opt 'label=COMPOSE_PROJECT={{index .Config.Labels "io.podman.compose.project"}}' in every container in every podman-compose file if I want to gather logs into centralized logging analytics service.

However this is out of scope of this PR, because first common library needs to be updated.

[packit-as-a-service]

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

[packit-as-a-service]

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

[github-actions]

A friendly reminder that this PR had no activity for 30 days.

[p12tic]

PR is waiting for review and conmon release.

[p12tic]

@Luap99 @mheon I think I addressed all review comments that I could address.

[p12tic]

@Luap99 I think I addressed all review comments that I could address again.

I apologize for needing so much hand holding..

[p12tic]

@Luap99 Just a friendly ping :-)

We stopped at this comment. Please let me know which solution you prefer and I will implement that. All other problems have been solved.

[p12tic]

@Luap99 This PR should be good to go now.

There are failures in CI, but it seems they are caused by general instability, because unrelated tests fail. I would schedule to rerun the failing tests, but don't have enough permissions.

[Luap99]

LGTM

[Luap99]

I restarted the flakes

[Luap99]

@containers/podman-maintainers PTAL

[mheon]

LGTM

[skoppe]

This is really awesome work and exactly what the doctor ordered.