daemon/logger/loggerutils: ParseLogTag: fix panic and image ID format, improve test-coverage#52343
Merged
thaJeztah merged 4 commits intomoby:masterfrom Apr 13, 2026
Merged
Conversation
Update the GoDoc for various methods to be less specific about the underlying implementation. This struct is currently passed as-is to loggers to allow setting a templated "tag", but holds various fields that are not documented for this purpose, and which do not have an explicit method or template-function. We may want to reimplement that code to be more deliberate on fields and/or methods intended for templating, but let's start with updating the docs. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added
area/logging
status/2-code-review
impact/changelog
kind/bugfix
- add consts for well-known templating options - improve test coverage, and use more realistic values in tests Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- fix a potential panic when using `ID()` / `{{.ID}}` or `ImageID()` / `{{.ImagID}}`
if the Info was empty. In practice, this shouldn't happen because it's always
initialized.
- fix `ImageID()` / `{{.ImageID}}` ("short ID") not trimming the algorithm.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Add fast-path for well-known options and skip templating. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
commented
Apr 11, 2026
Comment on lines
+13
to
+22
| const ( | ||
| ctrShortID = "{{.ID}}" | ||
| ctrFullID = "{{.FullID}}" | ||
| ctrName = "{{.Name}}" | ||
| ctrCommand = "{{.Command}}" | ||
| imgShortID = "{{.ImageID}}" | ||
| imgFullID = "{{.ImageFullID}}" | ||
| imgName = "{{.ImageName}}" | ||
| hostName = "{{.Hostname}}" | ||
| ) |
Member
Author
There was a problem hiding this comment.
We could consider having consts defined in the client to help creating a template (haven't thought yet what the right shape would be for that).
2 tasks
vvoland
reviewed
Apr 13, 2026
| @@ -10,6 +10,17 @@ import ( | |||
| // DefaultTemplate defines the defaults template logger should use. | |||
| const DefaultTemplate = "{{.ID}}" | |||
Contributor
There was a problem hiding this comment.
Suggested change
| const DefaultTemplate = "{{.ID}}" | |
| const DefaultTemplate = ctrShortID |
should we reuse the const instead?
Member
Author
There was a problem hiding this comment.
I think I kept it like this, so that it shows up in docs (pkg.go.dev), otherwise it shows as some un-exported var, and I thought it'd be more useful to know "what's this thing" 😂
vvoland
approved these changes
Apr 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
daemon/logger: Info: touch-up godoc
Update the GoDoc for various methods to be less specific about the underlying
implementation. This struct is currently passed as-is to loggers to allow
setting a templated "tag", but holds various fields that are not documented
for this purpose, and which do not have an explicit method or template-function.
We may want to reimplement that code to be more deliberate on fields and/or
methods intended for templating, but let's start with updating the docs.
daemon/logger/loggerutils: ParseLogTag: improve test-coverage
daemon/logger/loggerutils: ParseLogTag: fix panic and image ID
ID()/{{.ID}}orImageID()/{{.ImagID}}if the Info was empty. In practice, this shouldn't happen because it's always
initialized.
ImageID()/{{.ImageID}}("short ID") not trimming the algorithm.daemon/logger/loggerutils: ParseLogTag: add fast path
Add fast-path for well-known options and skip templating.
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)