Conversation
umputun
left a comment
There was a problem hiding this comment.
couple of critical issues:
-
regex false positives —
(.*)\.(\d+)\.(.*)matches any container name with dots and digits, not just swarm containers. names likemy.app.v2get mangled. a tighter pattern like^(.+)\.(\d+)\.[a-z0-9]{25}$would match actual swarm task IDs, or the originalcom.docker.swarm.task.idlabel approach could be used as primary detection -
Actor.Attributes ≠ container Labels —
activate()passesdockerEvent.Actor.AttributestobuildContainerName, but Actor Attributes don't contain custom container labels likelogger.container.name. the label override only works inemitRunningContainers(initial scan), not for live events -
no tests for
buildContainerName/buildGroupName— these have non-trivial regex + label fallback logic that needs coverage -
scope creep —
logger.group.name/logger.container.namelabels andAPP_UID=995in Dockerfile are separate features/fixes not related to issue #8. would be better as separate PRs
the overall approach of normalizing swarm names in both code paths is correct though
Solving the following problem:
#8