Skip to content

add docker swarm support#45

Open
darkeagle1337 wants to merge 3 commits intoumputun:masterfrom
darkeagle1337:master
Open

add docker swarm support#45
darkeagle1337 wants to merge 3 commits intoumputun:masterfrom
darkeagle1337:master

Conversation

@darkeagle1337
Copy link

Solving the following problem:

#8

Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

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

couple of critical issues:

  1. regex false positives(.*)\.(\d+)\.(.*) matches any container name with dots and digits, not just swarm containers. names like my.app.v2 get mangled. a tighter pattern like ^(.+)\.(\d+)\.[a-z0-9]{25}$ would match actual swarm task IDs, or the original com.docker.swarm.task.id label approach could be used as primary detection

  2. Actor.Attributes ≠ container Labelsactivate() passes dockerEvent.Actor.Attributes to buildContainerName, but Actor Attributes don't contain custom container labels like logger.container.name. the label override only works in emitRunningContainers (initial scan), not for live events

  3. no tests for buildContainerName/buildGroupName — these have non-trivial regex + label fallback logic that needs coverage

  4. scope creeplogger.group.name/logger.container.name labels and APP_UID=995 in 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

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.

3 participants