Skip to content

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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

p12tic
Copy link
Contributor

@p12tic p12tic commented May 26, 2025

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

Copy link
Contributor

openshift-ci bot commented May 26, 2025

[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.

Needs approval from an approver in each of these files:

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

@p12tic
Copy link
Contributor Author

p12tic commented May 26, 2025

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
Copy link
Member

Luap99 commented May 27, 2025

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
Copy link
Contributor Author

p12tic commented May 27, 2025

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
Copy link
Contributor Author

p12tic commented May 27, 2025

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
Copy link
Member

Luap99 commented May 27, 2025

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
Copy link
Contributor Author

p12tic commented May 27, 2025

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
Copy link
Member

mheon commented May 27, 2025

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
Copy link
Member

Luap99 commented May 27, 2025

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
Copy link
Contributor Author

p12tic commented May 27, 2025

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 p12tic changed the title libpod: Send container labels to conmon using --log-label libpod: Implement --log-opt labels=LABEL=Value May 29, 2025
This allows things like compose project names to be associated with log
messages and later used in log processing and analysis.

Signed-off-by: Povilas Kanapickas <[email protected]>
@p12tic
Copy link
Contributor Author

p12tic commented May 29, 2025

@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
Copy link
Contributor Author

p12tic commented May 29, 2025

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

@p12tic p12tic changed the title libpod: Implement --log-opt labels=LABEL=Value libpod: Implement --log-opt label=LABEL=Value May 29, 2025
@p12tic
Copy link
Contributor Author

p12tic commented May 29, 2025

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.

Copy link

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

1 similar comment
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants