Repository navigation
[SVLS-9300] ecs-fargate uninstrument command - #2505
ojproductions merged 3 commits into
Conversation
domalessi
left a comment
domalessi
left a comment
There was a problem hiding this comment.
Left some editorial feedback, but approving so you're not blocked on me!
23b7b7c to
f3527aa
Compare
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 1 job - 1 passed on retry 🔗 Commit SHA: 019b037 | Docs | View more details | Give us feedback! |
f3527aa to
74f96fc
Compare
ojproductions
added
the
serverless
1ba378a to
16a69f6
Compare
b7b545e to
a38dc36
Compare
a38dc36 to
da22cf5
Compare
stzou
left a comment
stzou
left a comment
There was a problem hiding this comment.
Looks good. Left three non-blocking comments on efficiency/reuse - happy to see them addressed here or in a follow-up.
| */ | ||
| export const resolveApps = async ( | ||
| client: ECSClient, | ||
| config: EcsFargateConfigOptions, |
There was a problem hiding this comment.
describeService sends one service per DescribeServicesCommand, even though the API accepts up to 10. Batching --ecs-service names in groups of 10 would cut API calls and rate-limit risk on large fleets.
| // The driver routes to the log router, which is being removed, so a container left with it would | ||
| // start no tasks. What it was configured with before log collection was turned on is recorded | ||
| // nowhere, so the container is left with none rather than with a guess. | ||
| const firelens = container.logConfiguration?.logDriver === AWSFIRELENS_LOG_DRIVER |
There was a problem hiding this comment.
This drops any awsfirelens container as if it must route through datadog-log-router, but it could be a customer's own Firelens setup. Reuse isDatadogFirelens() (checks options.Name === DATADOG_FLUENTBIT_OUTPUT) so we don't delete and mis-attribute a config we didn't add.
| /** | ||
| * The sidecars instrumentation adds, which are keyed by name. | ||
| */ | ||
| const isSidecar = (container: ContainerDefinition): boolean => |
There was a problem hiding this comment.
The sidecar name set is defined three times: SIDECAR_CONTAINER_NAMES (line 678), the inline checks in instrumentTaskDefinition's flatMap (lines 854/858), and isSidecar here. Worth having all three reuse SIDECAR_CONTAINER_NAMES so adding a sidecar can't miss one spot.
Drarig29
left a comment
Drarig29
left a comment
There was a problem hiding this comment.
can you rename the PR so it looks better in the changelog? 🙏
ojproductions
changed the title
da22cf5 to
5e50c79
Compare
Apply the README editorial comments and unblock CI by matching the plugin version and dry-run snapshot to current instrumentation output. Co-authored-by: Cursor <cursoragent@cursor.com>
5e50c79 to
904d2c8
Compare
4 participants
What and why?
Adds
datadog-ci ecs-fargate uninstrument, which removes Datadog instrumentation from an ECS Fargate task definition: it strips changes added byinstrumentadded and registers the result as a new revision. Running tasks do not change until that revision is deployed.How?
Same targeting and rollout as
instrument(--task-definition,--region,--profile,--ecs-service,--cluster,--dry-run,datadog-ci.json). Thetransform drops:
datadog-agentanddatadog-log-routersidecarsdd-socketsvolume and its mountsDD_-prefixed environment variable and secret, plusECS_FARGATEcom.datadoghq.tags.service/env/versionDocker labels (Autodiscoverylabels are left alone)
service,env,version, anddd_sls_cirevision tags--env-varsnames extra variables to remove. Fields declared by containersthemselves keep their position and value. Uninstrumenting a revision this
command registered should revert back to the task definition it was registered from.
A FireLens log configuration is not restored: the original driver is recorded nowhere, so the container is left with none and the command warns.
A task definition with nothing Datadog-owned to remove is reported as such and no revision is registered.
Beta: requires
DD_BETA_COMMANDS_ENABLED=1.Review checklist