-
Notifications
You must be signed in to change notification settings - Fork 14
wip: datadog-container-id header when running system tests locally #6098
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
base: main
Are you sure you want to change the base?
Conversation
|
|
cbeauchesne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor cosmetic requests changes
| volumes: dict | None = None, | ||
| working_dir: str | None = None, | ||
| pid_mode: str | None = None, | ||
| cgroupns_mode: str | None = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you keep this identical to docker run method :
| cgroupns_mode: str | None = None, | |
| cgroupns: Literal['private', 'host'] | None = None, |
| name=self.container_name, | ||
| hostname=self.name, | ||
| environment=self.environment, | ||
| # Build run arguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By keeping the cgroupns identical to the one used by run method, you can keep the current code design.
| # Set cgroupns to host mode to ensure container ID is visible in /proc/self/cgroup | ||
| # This is particularly important for Docker Desktop on macOS where the default | ||
| # private cgroup namespace shows "/" as the cgroup path | ||
| self.weblog_container.cgroupns_mode = "host" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be in DefaultScenario.__init__
Motivation
I am using the following test:
Changes
Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present