Skip to content

logging: Add container labels to log entries on journald #562

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

Merged
merged 1 commit into from
Jun 1, 2025

Conversation

p12tic
Copy link
Contributor

@p12tic p12tic commented May 26, 2025

At present it's not possible to ship properly labeled logs when using podman and tools like podman-compose. Container labels are lost which makes it much harder to understand where a particular log line originated from. Log processing and analysis is significantly more inconvenient as well, because it's hard to group related logs, e.g. coming from the same compose project.

This commit implements the parts necessary to annotate log messages with container labels. Each label and value pair is specified via --log-label LABEL=VALUE arguments.

This PR was initially based on #553 and ultimately was reworked enough that there's little semblance to the original PR anymore. Attribution is still preserved in commit message. The following is the changes from the 553 PR:

  • the labels are now passed via multiple input arguments. This avoids whole comma-delimited string parsing code and any additional limitations on the label values.
  • added checks against invalid labels or --log-label being used with k8s-file
  • added documentation
  • tested with patched podman and currently runs in production. Podman PR: libpod: Implement --log-opt label=LABEL=Value podman#26203.

Fixes #336.

@p12tic
Copy link
Contributor Author

p12tic commented May 26, 2025

Podman PR: containers/podman#26203

@p12tic p12tic force-pushed the journald-labels branch 4 times, most recently from fc7c6a6 to 10a3e75 Compare May 26, 2025 20:01
@p12tic
Copy link
Contributor Author

p12tic commented May 27, 2025

I think this PR fixes #336 after all.

@p12tic
Copy link
Contributor Author

p12tic commented May 30, 2025

@giuseppe you reviewed #553, maybe you would be interested in reviewing this PR which is evolution of that one? Thanks a lot!

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

nit, otherwise LGTM

@p12tic p12tic force-pushed the journald-labels branch from 10a3e75 to b151522 Compare May 31, 2025 21:44
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@p12tic p12tic force-pushed the journald-labels branch from b151522 to c241514 Compare May 31, 2025 21:47
Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

thanks,

LGTM

At present it's not possible to ship properly labeled logs when using
podman and tools like podman-compose. Container labels are lost which
makes it much harder to understand where a particular log line
originated from. Log processing and analysis is significantly more
inconvenient as well, because it's hard to group related logs, e.g.
coming from the same compose project.

This commit implements the parts necessary to annotate log messages with
container labels. Each label and value pair is specified via --log-label
LABEL=VALUE arguments.

Co-authored-by: OZoneGuy <[email protected]>
Signed-off-by: Povilas Kanapickas <[email protected]>
@p12tic p12tic force-pushed the journald-labels branch from c241514 to f37e9e7 Compare May 31, 2025 21:49
@rhatdan
Copy link
Member

rhatdan commented Jun 1, 2025

LGTM

@rhatdan rhatdan merged commit 9b74c32 into containers:main Jun 1, 2025
28 of 30 checks passed
@p12tic p12tic deleted the journald-labels branch June 4, 2025 06:21
@p12tic
Copy link
Contributor Author

p12tic commented Jun 4, 2025

@haircommander Would it be possible to release new version of conmon with this PR? I think containers/podman#26203 depends on new version of conmon. If new release is too early, I'm sorry for the noise and will wait for whenever a release will be made.

Comment on lines +161 to +168
if (!use_journald_logging) {
if (!tag) {
nexit("k8s-file doesn't support --log-tag");
}
if (!log_labels) {
nexit("k8s-file doesn't support --log-label");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks wrong to me...
It should be if (tag) { maybe?

Copy link
Member

Choose a reason for hiding this comment

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

yes that seems wrong, could you please open a PR so we can get it merged quickly?

@bitoku
Copy link
Contributor

bitoku commented Jun 4, 2025

cri-o is facing CI failure and seeing conmon failure.
cri-o/cri-o#9245

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

Successfully merging this pull request may close these issues.

CONTAINER_NAME doesn't output the correct "io.kubernetes.container.name" label
4 participants