[SVLS-9300] add ecs-fargate instrument command by ojproductions · Pull Request #2463 · DataDog/datadog-ci

ojproductions · 2026-08-21T20:36:40Z

What and why?

Adds datadog-ci ecs-fargate instrument, which instruments an AWS ECS Fargate task
definition with Datadog: it adds the datadog-agent sidecar, gives the application
containers the environment their tracers read, and registers the result as a new
revision. Container images are left untouched.

Fargate users currently have to hand-edit task definition JSON to add the sidecar, which
is easy to get wrong and hard to keep consistent across services.

How?

New plugin-ecs-fargate package plus the base command definition, following the usual
base/plugin split. A run describes each task definition, transforms it, and registers
the new revision.

Targeting: --task-definition (family, family:revision, or ARN), --region,
--profile, and a datadog-ci.json config file.
Rollout: --ecs-service points services at the revision that was just registered, so
the change reaches running tasks without a manual deployment. --cluster is only
needed when the services are not named by full ARN. Services are updated only after
every task definition in the run is instrumented, so a partial failure does not leave
half the fleet on a new revision.
API key: --api-key-secret-arn keeps the key out of the task definition; falling back
to DD_API_KEY writes it in plain text, which is warned about and validated first.
Idempotency: the Agent container is matched by name and the dd_sls_ci version tag is
excluded from the comparison, so re-running (or upgrading the CLI) does not burn a
revision.
--dry-run prints the diff and registers nothing.

Unified service tagging and the product toggles follow in the next PR; Windows tasks in
the one after. The @aws-sdk/client-ecs version is pinned to the one the other AWS
packages in the repo already use, so the lockfile addition stays small.

Review checklist

Feature or bugfix MUST have appropriate tests (unit, integration)

datadog-official · 2026-08-21T20:37:12Z

✅ All CI checks and tests passed.

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 56e9fc9 | Docs | View more details | Give us feedback!}

ava-silver · 2026-08-21T20:52:58Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ecdd3181c0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-21T20:55:57Z

+  if (settings.apiKeySecretArn && !taskDefinition.executionRoleArn) {
+    warnings.push(
+      `Task definition ${family} has no executionRoleArn, which ECS needs to read ${settings.apiKeySecretArn}. Tasks started from this revision will fail until the task definition has an execution role granting secretsmanager:GetSecretValue on that secret.`
+    )


Stop deployment when the secret has no execution role

When --api-key-secret-arn is used on a task definition without executionRoleArn, this only adds a warning even though the code itself establishes that tasks from the resulting revision cannot start. If --ecs-service was also supplied, the command proceeds to register and deploy that unusable revision, potentially replacing healthy tasks with tasks that fail at startup; this condition must block registration or at least the service update.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-08-21T20:55:57Z

+        const service = await describeService(client, cluster, name)
+        const family = taskDefinitionFamily(service.taskDefinition)
+        const target = instrumented.find((candidate) => candidate.family === family)


Reject duplicate task-definition families before deployment

When multiple --task-definition values resolve to different revisions of the same family, find() silently selects whichever result was listed first. For example, if an already-instrumented older revision is listed before a newer revision that the service currently runs, deployment points the service back to the older ARN instead of the newly registered revision; reject duplicate families or otherwise disambiguate the revision rather than selecting the first match.

Useful? React with 👍 / 👎.

OliviaShoup

this looks great! left a few comments but nothing blocking

OliviaShoup · 2026-08-21T21:24:35Z


+#### `ecs-fargate`
+
+<sub>**README:** [📚](/packages/plugin-ecs-fargate) | **Documentation:** [🔗](https://docs.datadoghq.com/integrations/ecs_fargate/) | **Plugin:** `@datadog/datadog-ci-plugin-ecs-fargate`</sub>


this link 301-redirects to /integrations/aws-fargate/ (confirmed via curl). it still works, but it's not the canonical URL

Suggested change

<sub>**README:** [📚](/packages/plugin-ecs-fargate) | **Documentation:** [🔗](https://docs.datadoghq.com/integrations/ecs_fargate/) | **Plugin:** `@datadog/datadog-ci-plugin-ecs-fargate`</sub>

<sub>**README:** [📚](/packages/plugin-ecs-fargate) | **Documentation:** [🔗](https://docs.datadoghq.com/integrations/aws-fargate/) | **Plugin:** `@datadog/datadog-ci-plugin-ecs-fargate`</sub>

OliviaShoup · 2026-08-21T21:24:36Z

+<!-- BEGIN_USAGE:instrument -->
+| Argument | Shorthand | Description | Default |
+| -------- | --------- | ----------- | ------- |
+| `--dry` or `--dry-run` | `-d` | Preview changes running command would apply | `false` |


OliviaShoup · 2026-08-21T21:30:53Z

+
+#### Deploying the new revision
+
+Pass `--ecs-service` for each service that should run the revision the command just registered, and `--cluster` if those services are not in the `default` cluster. A service named by its full ARN already says which cluster it runs in, so `--cluster` can be left off; passing one that the ARN contradicts is an error rather than a silent choice between them. A run updates services in a single cluster, so ARNs naming more than one are reported too. Each service is matched to the task definition family it currently runs, so a run over several task definitions points each service at its own new revision, and a service already running the instrumented revision is left alone rather than redeployed. Updating a service starts an ECS deployment: the command returns as soon as ECS accepts it, and the rollout follows your service's deployment configuration.


"a silent choice between them" is a little hard to parse on first read. the antecedent for "them" (ARN's cluster vs. flag's cluster) isn't obvious. so suggesting this but no pressure

Suggested change

Pass `--ecs-service` for each service that should run the revision the command just registered, and `--cluster` if those services are not in the `default` cluster. A service named by its full ARN already says which cluster it runs in, so `--cluster` can be left off; passing one that the ARN contradicts is an error rather than a silent choice between them. A run updates services in a single cluster, so ARNs naming more than one are reported too. Each service is matched to the task definition family it currently runs, so a run over several task definitions points each service at its own new revision, and a service already running the instrumented revision is left alone rather than redeployed. Updating a service starts an ECS deployment: the command returns as soon as ECS accepts it, and the rollout follows your service's deployment configuration.

Pass `--ecs-service` for each service that should run the revision the command just registered, and `--cluster` if those services are not in the `default` cluster. A service named by its full ARN already says which cluster it runs in, so `--cluster` can be left off; passing a --cluster that contradicts the ARN's cluster is an error, not a silent override. A run updates services in a single cluster, so ARNs naming more than one are reported too. Each service is matched to the task definition family it currently runs, so a run over several task definitions points each service at its own new revision, and a service already running the instrumented revision is left alone rather than redeployed. Updating a service starts an ECS deployment: the command returns as soon as ECS accepts it, and the rollout follows your service's deployment configuration.

ava-silver

Automated AI Review (human curated)

The command needs stronger safeguards around deployable task definitions, MFA-backed AWS profiles, and duplicate task-definition families.

ava-silver · 2026-08-24T21:41:50Z

+
+  // ECS resolves secrets through the task's execution role, so a reference without a role in place
+  // registers a revision whose tasks cannot start.
+  if (settings.apiKeySecretArn && !taskDefinition.executionRoleArn) {


P1: Reject this configuration instead of warning. ECS resolves container secrets through the task execution role, so this revision cannot start without executionRoleArn; with --ecs-service, the command proceeds to deploy that unusable revision.

ava-silver · 2026-08-24T21:41:50Z

+/**
+ * Returns the credentials loaded from the given AWS named profile.
+ */
+export const getAWSProfileCredentials = async (profile: string): Promise<AwsCredentialIdentity | undefined> => {


P1: Support MFA-backed named profiles here. The analogous Lambda credential helper supplies mfaCodeProvider, but this fromIni setup does not, so --profile fails for profiles configured with mfa_serial. Please share or reproduce the Lambda credential setup, including its MFA callback.

ava-silver · 2026-08-24T21:41:50Z

+      try {
+        const service = await describeService(client, cluster, name)
+        const family = taskDefinitionFamily(service.taskDefinition)
+        const target = instrumented.find((candidate) => candidate.family === family)


P2: Do not select the first result when multiple targets resolve to the same family. For example, --task-definition app:1 --task-definition app:2 registers both revisions but deploys the first one, which can roll a service back. Reject duplicate resolved families or define an explicit selection rule.

ava-silver

Automated AI Review (human curated)

Two additional parity gaps surfaced when comparing this command with the corresponding Terraform module and CDK construct.

ava-silver · 2026-08-24T22:09:02Z

+    // The Agent's own trace intake, which is a separate switch from the tracers' `DD_TRACE_ENABLED`.
+    [DD_APM_ENABLED_ENV_VAR]: 'true',
+    [DD_USE_DOGSTATSD_ENV_VAR]: 'true',
+    [DD_ECS_TASK_COLLECTION_ENABLED_ENV_VAR]: 'true',


P2: Enabling ECS task collection requires permissions that this command neither supplies nor checks. The Terraform module adds ecs:ListClusters, ecs:ListContainerInstances, and ecs:DescribeContainerInstances to the task role, and the CDK construct adds the same policy. Without those permissions, ECS task collection is incomplete. Avoid enabling it automatically, or make the required task-role policy explicit to the user.

ava-silver · 2026-08-24T22:09:02Z

+    [SITE_ENV_VAR]: settings.site,
+    // The Agent's own trace intake, which is a separate switch from the tracers' `DD_TRACE_ENABLED`.
+    [DD_APM_ENABLED_ENV_VAR]: 'true',
+    [DD_USE_DOGSTATSD_ENV_VAR]: 'true',


P2: Match the DogStatsD origin-tagging defaults used by the other ECS instrumentation tools. The Terraform module defaults origin detection to enabled and cardinality to orchestrator (configuration, Agent environment); the CDK construct injects the same variables. Add DD_DOGSTATSD_ORIGIN_DETECTION=true, DD_DOGSTATSD_ORIGIN_DETECTION_CLIENT=true, and DD_DOGSTATSD_TAG_CARDINALITY=orchestrator so custom metrics retain equivalent origin tags and cardinality.

stzou

Thanks for adding ECS fargate support - just a couple comments around logging and traces.

APM/DogStatsD: right now app containers reach the Agent over the implicit localhost:8126 tracer default, which only works because awsvpc shares one network namespace. Can we follow the Terraform module's pattern here?

mount a shared dd-sockets volume
set DD_TRACE_AGENT_URL/DD_DOGSTATSD_URL to the unix socket by default and fall back to UDP (DD_AGENT_HOST=127.0.0.1) only when the user opts out of the socket.

Logs: there's currently no path to get application logs into Datadog. Could we add an opt-in Fluent Bit sidecar (datadog-log-router) using the awsfirelens log driver, matching the Terraform module's dd_log_collection config, so app containers can route logs through it to Datadog's log intake.

Could also be worth adding DD_INSTALL_INFO_TOOL / _TOOL_VERSION / _INSTALLER_VERSION so we can track usage of this cli. If there's another way this is being tracked, feel free to leave it out.

Where possible, we should try to have parity between this onboarding tool and existing ones so the onboarding experience is consistent regardless of method used.

ava-silver · 2026-08-28T19:25:18Z

+
+import {AWS_SHARED_CREDENTIALS_FILE_ENV_VAR, EXPONENTIAL_BACKOFF_RETRY_STRATEGY} from './constants'
+
+// TODO: the two credential helpers below are duplicated from plugin-lambda's `functions/commons.ts`.


yeah the tricky thing is we don't want to pull the AWS SDK into the base package -- we can revisit this if need be in the future

ava-silver · 2026-08-28T19:25:42Z

+    return await credentialsProvider()
+  } catch (err) {
+    if (err instanceof Error) {
+      throw Error(`Couldn't set AWS profile credentials. ${err.message}`)


Suggested change

throw Error(`Couldn't set AWS profile credentials. ${err.message}`)

throw Error(`Couldn't get AWS profile credentials. ${err.message}`)

ava-silver · 2026-08-28T20:14:52Z

+      return 1
+    }
+
+    const region = config.region ?? AWS_REGION_ENV_VARS.map((envVar) => process.env[envVar]).find((value) => !!value)


would it be worth supporting multiple regions here (based on the task definition ARN)? although either way that'd be better as a follow-up PR

yup I think followup PR is better

ava-silver · 2026-08-28T20:32:30Z

+
+    if (failed) {
+      // Deploying changes what is running, so a run that could not instrument every task definition
+      // stops here rather than pointing some of the services at a new revision.


I'm not sure I understand why we'd want to do this rather than roll out the successful ones -- lets say they have 99 task definitions succeed, but one fails, we'd fail the entire fleet and nothing would get rolled out

I feel like we should probably be in line with the other commands and roll out the successful ones, so we could do each instrumentation in parallel and just wait for all the operations at the same time.

ava-silver · 2026-08-28T20:33:15Z

+    const instrumented: InstrumentedRevisions = new Map()
+    let failed = false
+    for (const taskDefinition of config.taskDefinitions ?? []) {
+      const revision = await this.instrument(client, taskDefinition, settings, services.length > 0)


this should probably use a Promise.all() to do these in parallel, right? (Assuming the ECS client has throttling/retry handling builtin)

ava-silver · 2026-08-28T20:36:10Z

+    instrumented: InstrumentedRevisions
+  ): Promise<boolean> {
+    let deployed = true
+    for (const name of services) {


same with this for loop, it'd be great to parallelize these, although if we refactor to make each app individual then we probably get this for free

ojproductions · 2026-09-01T20:52:36Z

@stzou I've added the dd-sockets change, though I think the logging changes would be better added to this PR (the next PR in the stack) where I add other configuration flags.
Regarding DD_INSTALL_INFO_TOOL / _TOOL_VERSION / _INSTALLER_VERSION variables, we're already using the dd_sls_ci tag... should we be adding these new vars? if so this may also be a separate PR as none of the other cli commands add this.
thoughts?

stzou

LGTM

For the install info vars, dd_sls_ci covers our need to track CLI-driven installs so we're fine leaving those out.

ava-silver

a couple last things i missed in my initial review now after solidifying on command guidance: #2500

ava-silver · 2026-09-08T18:37:55Z

+    [DD_ECS_TASK_COLLECTION_ENABLED_ENV_VAR]: 'true',
+    ...(settings.apiKey ? {[API_KEY_ENV_VAR]: settings.apiKey} : {}),
+  },
+  defaults: {


The lifecycle guidance says omitted configuration must resolve to an explicit default or absence rather than preserve remote Datadog state. These values -- along with DD_TRACE_ENABLED and DD_LOGS_INJECTION below -- are defaults, so the same invocation can retain an earlier false or low value and yield a different Datadog configuration. Please either manage these values or add explicit options whose omitted values resolve deterministically.

ava-silver · 2026-09-08T18:37:55Z

+    ...(settings.apiKey ? {[API_KEY_ENV_VAR]: settings.apiKey} : {}),
+  },
+  defaults: {
+    [DD_DOGSTATSD_ORIGIN_DETECTION_ENV_VAR]: 'true',


For ECS Fargate, Datadog's DogStatsD UDS documentation requires pidMode: "task" when DD_DOGSTATSD_ORIGIN_DETECTION=true. This transform enables origin detection but leaves pidMode unchanged, so metrics may not receive the task/container origin tags described in the README. Please apply pidMode: "task" as part of this desired state.

ava-silver · 2026-09-08T18:37:55Z

+
+  const segments = service.split('/')
+
+  return segments.length === 3 ? segments[1] : undefined


This returns no cluster for a legacy short service ARN (...:service/name). ECS then resolves it in the default cluster, so a service in another cluster cannot be found despite the README saying a full ARN does not need --cluster. Please require --cluster for short-format ARNs and reserve automatic cluster extraction for long-format ARNs.

… app individually + parallelism

ojproductions requested review from a team as code owners August 21, 2026 20:36

ojproductions requested a review from ava-silver August 21, 2026 20:36

ava-silver added the serverless Related to [aas, cloud-run, lambda, stepfunctions, ecs-fargate] label Aug 21, 2026

ojproductions changed the title ~~Add ecs-fargate instrument~~ [svls-9300] add ecs-fargate instrument command Aug 21, 2026

ojproductions changed the title ~~[svls-9300] add ecs-fargate instrument command~~ [SVLS-9300] add ecs-fargate instrument command Aug 21, 2026

ojproductions added the enhancement New feature or request label Aug 21, 2026

chatgpt-codex-connector Bot reviewed Aug 21, 2026

View reviewed changes

OliviaShoup approved these changes Aug 21, 2026

View reviewed changes

ava-silver reviewed Aug 24, 2026

View reviewed changes

ojproductions requested a review from ava-silver August 27, 2026 17:55

stzou reviewed Aug 27, 2026

View reviewed changes

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch from 03d44ec to dd3edda Compare August 27, 2026 20:11

ava-silver reviewed Aug 28, 2026

View reviewed changes

ava-silver approved these changes Sep 2, 2026

View reviewed changes

ojproductions requested a review from stzou September 2, 2026 18:59

stzou approved these changes Sep 2, 2026

View reviewed changes

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch from bf06453 to 8c8913c Compare September 4, 2026 21:55

ava-silver reviewed Sep 8, 2026

View reviewed changes

ava-silver mentioned this pull request Sep 10, 2026

chore: minorly update serverless guidance #2500

Merged

1 task

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch 2 times, most recently from 20d0db4 to ea77750 Compare September 10, 2026 14:47

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch from 42d33cd to b9298ab Compare September 10, 2026 16:26

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch from b9298ab to 6ed66ca Compare September 10, 2026 20:12

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch from 6ed66ca to 5e2bbd7 Compare September 11, 2026 14:12

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch from 5e2bbd7 to 790c5e6 Compare September 14, 2026 18:10

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch 2 times, most recently from 83e1dca to 9e8877e Compare September 15, 2026 19:44

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch from a2d7c20 to b640b09 Compare September 21, 2026 16:38

ojproductions added 7 commits September 21, 2026 12:40

Add ecs-fargate instrument

8d31b98

pr feedback

9af14cf

use unix socket transport by default + add install info

3d24c57

dont block succesful apps, if some apps fail + instrument/deploy each…

3e57de2

… app individually + parallelism

drop tool info vars

bc88a2e

yarn lint

253aa59

update package version

56e9fc9

ojproductions force-pushed the onzia/ecs-instrument-command-02 branch from b640b09 to 56e9fc9 Compare September 21, 2026 16:40

ojproductions merged commit b12eceb into master Sep 25, 2026
36 checks passed

ojproductions deleted the onzia/ecs-instrument-command-02 branch September 25, 2026 18:31

Drarig29 mentioned this pull request Oct 6, 2026

v5.25.0 #2545

Merged


		#### `ecs-fargate`

		<sub>README: [📚](/packages/plugin-ecs-fargate) \| Documentation: [🔗](https://docs.datadoghq.com/integrations/ecs_fargate/) \| Plugin: `@datadog/datadog-ci-plugin-ecs-fargate`</sub>

	\| `--dry` or `--dry-run` \| `-d` \| Preview changes running command would apply \| `false` \|
	\| `--dry` or `--dry-run` \| `-d` \| Preview the changes the command would apply \| `false` \|


		#### Deploying the new revision

		Pass `--ecs-service` for each service that should run the revision the command just registered, and `--cluster` if those services are not in the `default` cluster. A service named by its full ARN already says which cluster it runs in, so `--cluster` can be left off; passing one that the ARN contradicts is an error rather than a silent choice between them. A run updates services in a single cluster, so ARNs naming more than one are reported too. Each service is matched to the task definition family it currently runs, so a run over several task definitions points each service at its own new revision, and a service already running the instrumented revision is left alone rather than redeployed. Updating a service starts an ECS deployment: the command returns as soon as ECS accepts it, and the rollout follows your service's deployment configuration.


		import {AWS_SHARED_CREDENTIALS_FILE_ENV_VAR, EXPONENTIAL_BACKOFF_RETRY_STRATEGY} from './constants'

		// TODO: the two credential helpers below are duplicated from plugin-lambda's `functions/commons.ts`.

	throw Error(`Couldn't set AWS profile credentials. ${err.message}`)
	throw Error(`Couldn't get AWS profile credentials. ${err.message}`)


		const segments = service.split('/')

		return segments.length === 3 ? segments[1] : undefined

Conversation

ojproductions commented Aug 21, 2026

What and why?

How?

Review checklist

Uh oh!

datadog-official Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ava-silver commented Aug 21, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

OliviaShoup left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ava-silver left a comment

Choose a reason for hiding this comment

Automated AI Review (human curated)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ava-silver left a comment

Choose a reason for hiding this comment

Automated AI Review (human curated)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stzou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ojproductions commented Sep 1, 2026

Uh oh!

stzou left a comment

Choose a reason for hiding this comment

Uh oh!

ava-silver left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ava-silver Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

datadog-official Bot commented Aug 21, 2026 •

edited

Loading

ava-silver left a comment •

edited

Loading

ava-silver Sep 8, 2026 •

edited

Loading