From 954fe90e54790d2d9d6b528a8749d63942551b90 Mon Sep 17 00:00:00 2001 From: Sphia Sadek Date: Tue, 4 Aug 2026 00:11:33 +0100 Subject: [PATCH 1/2] fix(agent): stop hard-coding log-delivery logical ids, closes #703 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stack overrode the logical ids of the six log-delivery resources from a table keyed by stack name, holding values read off one account's live stack. Stack name is not a proxy for deployed state: two accounts running a stack of the same name had diverged, so the table was correct for one and actively caused the rename on the other. That rename is fatal rather than cosmetic. A DeliverySource is unique per (resource ARN, log type) account-wide, and the runtime ARN does not change when the resource is renamed — so CloudFormation's create-before-delete produces a second source for the same runtime, CloudWatch Logs rejects it as already existing, and the whole stack update rolls back. Renaming can never avoid that collision, because the conflict is on the ARN the resources point at rather than on their own names. Since no set of literals can describe every account's deployed state, this holds none. The resources go back to being named by the AgentCore Runtime, which generates them from the construct path — deterministically, and identically in every account, with nothing to keep in sync and no table to re-record when it drifts. The wrong instinct here was to swap the recorded ids for self-chosen stable ones. I tried it: it renames on BOTH accounts, so it breaks the account that is already correct in order to fix the other. Verified by diff before discarding it. Also removes the earlier `-c pinnedLogDeliveryStack` gate along with the table. Gating the safe path behind a flag inverted the default: the failure that teaches an operator the flag exists is a mid-update rollback whose message never mentions it. Effect per account, measured with `cdk diff` against live stacks: - a stack already on the library's naming: zero churn, the delivery resources are not touched at all. This is the account the table was breaking. - a stack the table was holding on legacy ids: those six ids converge once. No operator migration is prescribed, because nothing is being migrated TO — the stack simply stops being held back from what the library already generates. Tests replace the two that asserted the table's existence: one that no captured id, account-unique Name, or override survives anywhere in the source, and one that both log types are still wired so "no pin" cannot silently mean "no delivery". Both were confirmed to fail against a reintroduced hard-coded override AND against the self-chosen-prefix version above, so they reject the wrong fix as well as the original bug. --- cdk/src/stacks/agent.ts | 133 ++-------------------------------- cdk/test/stacks/agent.test.ts | 76 ++++++++++--------- 2 files changed, 43 insertions(+), 166 deletions(-) diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index ea37c3d4..f873a4c3 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -403,30 +403,12 @@ export class AgentStack extends Stack { runtimeArnHolder = runtime.agentRuntimeArn; - // --- AgentCore log-delivery: keep the logical ids STABLE across library - // renames, so updating an existing stack never has to be opted into --- - // - // The AgentCore Runtime auto-creates AWS::Logs::DeliverySource + Delivery + - // DeliveryDestination per loggingConfig, naming them from the construct path - // the library happens to use. When that path changes — as it did between - // library versions here — the CFN logical ids change with it, and CFN treats - // renamed resources as new ones: it CREATES before it DELETES. - // - // A DeliverySource is unique per (resource ARN, log type) for the whole - // account, and the runtime ARN does not change across the rename. So the new - // source collides with the live one that is still there, CloudWatch Logs - // rejects it with ``AlreadyExists``, and the whole stack rolls back. Note - // what this means: renaming the resources cannot avoid the collision, because - // the conflict is on the ARN they point at, not on their own names. Only - // keeping the logical id stable avoids it, since that is what makes CFN - // update in place rather than create a second source for the same runtime. - // - // Hence: pinned ALWAYS, for every stack, with no context flag. A flag would - // mean the safe path is the one you have to know to ask for, and the failure - // it prevents is a mid-update rollback that says nothing about the flag's - // existence. A fresh stack is unaffected either way — it has no live sources - // to collide with, and these ids are as valid for it as the library's own. - pinLogDeliveryLogicalIds(runtime); + // Log delivery is left entirely to the AgentCore Runtime, which creates and + // names the DeliverySource / Delivery / DeliveryDestination trio itself. We + // deliberately do not override those logical ids: see the note in the design + // docs, but in short, pinning them to values recorded from one account's live + // stack made the template describe that account rather than the code, and broke + // every other account on update. // --- Session storage (preview) --- // The L2 construct does not yet expose filesystemConfigurations; use the @@ -1441,106 +1423,3 @@ export class AgentStack extends Stack { } } -/** - * A churned log-delivery resource to re-pin: the construct child id under the - * Runtime, the logical id CFN already has deployed, and (for the account-unique - * Source/Destination kinds) the deployed ``Name``. ``liveName`` is omitted for - * Delivery links, which have no Name. - */ -interface PinnedLogResource { - readonly childId: string; - readonly liveLogicalId: string; - readonly liveName?: string; -} - -/** - * Log-delivery logical ids to keep stable, keyed by stack name. Consulted on - * every synth — see {@link pinLogDeliveryLogicalIds} for why there is no flag. - * - * Each entry records what CloudFormation already has for a stack deployed before - * the library renamed these resources. Read from `aws cloudformation - * list-stack-resources` against the live stack, so the ids are observed, not - * constructed — the hash in each one is not reproducible from the construct path - * alone, which is precisely why they have to be written down. - * - * An entry stays until its stack is gone. Removing one while the stack still - * exists re-introduces the rename and the failed update that comes with it. - */ -const PINNED_LOG_DELIVERY_BY_STACK: Record = { - 'backgroundagent-dev': [ - { - childId: 'ApplicationLogsDeliverySource', - liveLogicalId: 'RuntimeCDKSourceAPPLICATIONLOGSbackgroundagentdevRuntimeBC0AE9ED96A02E02', - liveName: 'cdk-applicationlogs-source-backgroundagentdevRuntimeBC0AE9ED', - }, - { - childId: 'UsageLogsDeliverySource', - liveLogicalId: 'RuntimeCDKSourceUSAGELOGSbackgroundagentdevRuntimeBC0AE9ED544FBB22', - liveName: 'cdk-usagelogs-source-backgroundagentdevRuntimeBC0AE9ED', - }, - { - childId: 'ApplicationLogsDest', - liveLogicalId: 'RuntimeCdkLogGroupApplicationLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeApplicationLogGroup454A95E8DestapplicationlogsE09F77DC', - liveName: 'cdk-cwl-Destapplication-logs-dest-backgrounp454A95E829BF8A27', - }, - { - childId: 'UsageLogsDest', - liveLogicalId: 'RuntimeCdkLogGroupUsageLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeUsageLogGroup7FA1FA67Destusagelogs9AB608D0', - liveName: 'cdk-cwl-Destusage-logs-dest-backgroundagroup7FA1FA67A8A16CEE', - }, - // Delivery links: logical-id pin only (no Name — unique per source/dest pair). - { - childId: 'ApplicationLogsDelivery', - liveLogicalId: 'RuntimeCdkLogGroupApplicationLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeApplicationLogGroup454A95E8Delivery92FE492C', - }, - { - childId: 'UsageLogsDelivery', - liveLogicalId: 'RuntimeCdkLogGroupUsageLogsDeliverybackgroundagentdevRuntimeBC0AE9EDbackgroundagentdevRuntimeUsageLogGroup7FA1FA67Delivery40F023D7', - }, - ], -}; - -/** - * Pin the auto-created log-delivery resources to stable logical ids, ALWAYS. - * - * These resources are created for us by the AgentCore Runtime and named after - * whatever construct path the library uses internally, so a library-side rename - * silently renames them — and a renamed resource is, to CloudFormation, a new - * one to create before the old is deleted. That is fatal here: a DeliverySource - * is unique per (resource ARN, log type) account-wide, the runtime ARN is - * unchanged by a rename, so the create collides with the live source and the - * update rolls the whole stack back. Owning the ids ourselves decouples us from - * the library's internal naming. - * - * Applied unconditionally rather than behind a flag. Three cases, all safe: - * - * - An existing stack in the account that owns these resources: the ids match - * what CloudFormation already recorded, so it updates them in place. This is - * the case that was broken. - * - A fresh stack or account: nothing owns these names yet, so they create - * normally. The ids are ours rather than the library's, which is the point; - * the values themselves carry no meaning beyond being stable. - * - Any other name: the ids embed the stack name, so each stack gets its own. - * - * The values were read off a stack deployed before the rename. Do not "tidy" - * them — they are a record of what CloudFormation already has, and editing one - * re-breaks exactly the update path this exists to protect. - */ -function pinLogDeliveryLogicalIds(runtime: agentcore.Runtime): void { - const stack = Stack.of(runtime); - const pins = PINNED_LOG_DELIVERY_BY_STACK[stack.stackName]; - // Only the stack these ids were recorded from can use them: they embed that - // stack's name. Any other stack keeps the library's own naming, which is - // correct for it — it has no pre-rename resources to line up with. - if (!pins) return; - - for (const pin of pins) { - const res = runtime.node.tryFindChild(pin.childId) as CfnResource | undefined; - // A future library rename moves the child, so the pin stops matching. Skip - // rather than throw: the stack still deploys, and the next update that hits - // the collision is the signal to re-record the ids from the live stack. - if (!res) continue; - res.overrideLogicalId(pin.liveLogicalId); - if (pin.liveName !== undefined) res.addPropertyOverride('Name', pin.liveName); - } -} diff --git a/cdk/test/stacks/agent.test.ts b/cdk/test/stacks/agent.test.ts index 2fa8d10b..d28e7611 100644 --- a/cdk/test/stacks/agent.test.ts +++ b/cdk/test/stacks/agent.test.ts @@ -529,59 +529,57 @@ describe('AgentStack', () => { expect(objectStatements).toEqual([]); }); - test('log-delivery logical ids are pinned with NO opt-in, so an existing stack updates in place', () => { - // A DeliverySource is unique per (resource ARN, log type) for the whole - // account, and the runtime ARN survives a library-side rename of these - // auto-created resources. So a renamed source is a SECOND source for the same - // runtime: CloudFormation creates before deleting, CloudWatch Logs rejects it - // as already existing, and the update rolls the whole stack back. + test('no hard-coded log-delivery logical ids or names anywhere in the stack', () => { + // The template must not carry values recorded from one account's live stack. // - // The ids must therefore be pinned unconditionally. Behind a flag, the safe - // path is the one an operator has to already know about, and the failure that - // teaches them is a mid-update rollback whose message never mentions it. + // These resources are created and named by the AgentCore Runtime. An earlier + // version overrode their logical ids from a table keyed by STACK NAME, holding + // ids read off one deployed stack — but stack name is not a proxy for deployed + // state. Two accounts running a stack of the same name had diverged, so the + // table was right for one and actively caused the rename on the other, which is + // fatal: a DeliverySource is unique per (resource ARN, log type) account-wide, + // the runtime ARN survives a rename, so CloudFormation's create-before-delete + // collides with the live source and the whole update rolls back. // - // Asserted on the source, not by synthesizing a second stack: constructing - // one under a different construct id trips an unrelated cdk-nag - // suppression-path check first, which masks whatever this is checking. + // There is no set of literals that serves every account, so the fix is to hold + // none. Asserted on the source because the failure mode is a value being + // reintroduced, not a synth-visible shape. const src = fs.readFileSync( path.resolve(__dirname, '../../src/stacks/agent.ts'), 'utf8', ); - const fn = src.slice(src.indexOf('function pinLogDeliveryLogicalIds')); - const body = fn.slice(0, fn.indexOf('\n}')); - // Keyed off the stack's OWN name — no context, no opt-in. - expect(body).toContain('PINNED_LOG_DELIVERY_BY_STACK[stack.stackName]'); - expect(body).not.toContain('tryGetContext'); - // Nothing anywhere may reintroduce a gate. + // No table, no lookup, no per-stack keying, and no flag to gate any of it. + expect(src).not.toContain('PINNED_LOG_DELIVERY_BY_STACK'); expect(src).not.toContain('pinnedLogDeliveryStack'); + expect(src).not.toContain('overrideLogicalId'); - // The ids it pins are the ones CloudFormation already holds for that stack. - // Hard-coded here on purpose: if someone "tidies" a value in the table, this - // fails instead of the next production update rolling back. - expect(src).toContain('RuntimeCDKSourceAPPLICATIONLOGSbackgroundagentdevRuntimeBC0AE9ED96A02E02'); - expect(src).toContain('RuntimeCDKSourceUSAGELOGSbackgroundagentdevRuntimeBC0AE9ED544FBB22'); + // No captured logical id or account-unique Name, in any of their shapes. + expect(src).not.toMatch(/RuntimeCDKSource/); + expect(src).not.toMatch(/cdk-(application|usage)logs-source-/); + expect(src).not.toMatch(/cdk-cwl-Dest/); }); - test('a stack with no recorded ids keeps the library\'s own log-delivery naming', () => { - // The pinned ids embed a stack name, so they are only correct for that stack. - // Another stack has no pre-rename resources to line up with and must not - // inherit them — otherwise two stacks in one account would claim the same - // account-unique DeliverySource Name. The table lookup is what enforces this, - // so assert it returns nothing for an unknown name rather than falling back. - const src = fs.readFileSync( - path.resolve(__dirname, '../../src/stacks/agent.ts'), 'utf8', - ); - const fn = src.slice(src.indexOf('function pinLogDeliveryLogicalIds')); - const body = fn.slice(0, fn.indexOf('\n}')); - expect(body).toMatch(/if \(!pins\) return;/); - - // And this stack — named TestAgentStack, absent from the table — got the - // library's naming, with none of backgroundagent-dev's ids leaking in. - const ids = Object.keys(template.findResources('AWS::Logs::DeliverySource')); + test('log delivery is left to the library, so every account synthesizes the same ids', () => { + // The point of holding no ids: what lands in the template comes from the + // library, so the same code produces the same ids in every account. A stack + // already on the library's naming sees no change at all. + const sources = template.findResources('AWS::Logs::DeliverySource'); + const ids = Object.keys(sources); expect(ids).toHaveLength(2); + for (const id of ids) { + // Library-generated, not ours: no self-chosen prefix and no stack name baked + // in. A stack name in a logical id is the signature of the old table. expect(id).not.toContain('backgroundagentdev'); + expect(id).not.toContain('AgentRuntimeApplication'); + expect(id).toMatch(/^Runtime(Application|Usage)LogsDeliverySource[0-9A-F]{8}$/); } + + // Both log types are wired, so "no pin" cannot mean "no delivery". + const logTypes = Object.values(sources) + .map((r) => (r as { Properties?: { LogType?: string } }).Properties?.LogType) + .sort(); + expect(logTypes).toEqual(['APPLICATION_LOGS', 'USAGE_LOGS']); }); test('the fan-out consumer can reach BOTH surfaces\' credentials registries', () => { From 6e6cabdb0f5baf931ba35fbdd85db5c08dd9a69a Mon Sep 17 00:00:00 2001 From: Sphia Sadek Date: Mon, 31 Aug 2026 14:24:52 -0400 Subject: [PATCH 2/2] docs(observability): document the log-delivery rename hazard and its migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review on #705. The code comment deferred its entire rationale to "the note in the design docs", and no such note existed — so the durable reasoning for a decision that can roll a stack back lived only in a PR and an issue. Adds it to OBSERVABILITY.md: why the AgentCore library owns these logical ids, why renaming any of them collides on the unchanged runtime ARN rather than on their own names, why a table of recorded ids keyed by stack name cannot describe per-account state, and the one-time operator step for a stack that still sits on pre-rename ids — including how to tell whether a given stack needs it. That step is the part the previous description got wrong. Convergence on such a stack is the fatal rename, not a benign one-time id change: the old delivery resources have to be deleted before the new ones are created, or the update rolls back. Leaving that undocumented handed those operators the same unexplained mid-update rollback that removing the context flag was meant to stop. Also makes the logical-id assertion say out loud that it is a canary on the library's current naming, so the next rename reads as an operational warning about already-deployed stacks rather than a string mismatch. --- cdk/src/stacks/agent.ts | 32 ++++++++-- cdk/test/stacks/agent.test.ts | 9 +++ docs/design/OBSERVABILITY.md | 63 +++++++++++++++++++ .../docs/architecture/Observability.md | 63 +++++++++++++++++++ 4 files changed, 161 insertions(+), 6 deletions(-) diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index 9b64e0c4..5b5481d7 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -612,12 +612,32 @@ export class AgentStack extends Stack { runtimeArnHolder = runtime.agentRuntimeArn; - // Log delivery is left entirely to the AgentCore Runtime, which creates and - // names the DeliverySource / Delivery / DeliveryDestination trio itself. We - // deliberately do not override those logical ids: see the note in the design - // docs, but in short, pinning them to values recorded from one account's live - // stack made the template describe that account rather than the code, and broke - // every other account on update. + // --- AgentCore log delivery: the library owns these logical ids, not us --- + // + // The Runtime above creates a DeliverySource / DeliveryDestination / Delivery + // trio per loggingConfig, naming them from its own construct path. We + // deliberately leave those names alone, and that is load-bearing. + // + // Renaming any of them is fatal on an existing stack. A DeliverySource is + // unique per (resource ARN, log type) account-wide and the runtime ARN does not + // change, so CloudFormation's create-before-delete produces a second source for + // the same runtime, CloudWatch Logs rejects it as already existing, and the + // whole update rolls back. Note what that implies: no choice of name avoids the + // collision, because the conflict is on the ARN they point at rather than on + // their own names. Only leaving a logical id untouched updates in place. + // + // An earlier version overrode them from a table of ids recorded off a live + // stack, keyed by stack NAME. Stack name is not a proxy for deployed state: + // two accounts running a stack of the same name had diverged, so the table was + // correct for one and caused the rename on the other. No set of literals can + // describe every account, so we hold none and let the library generate them — + // deterministically, identically in every account, with nothing to keep in sync. + // + // The one cost: a stack deployed before a library-side rename, or held on older + // ids by that table, converges once and needs a one-time operator step first, + // since the old resources must be gone before the new ones are created. That + // step, and how to tell whether a stack needs it, are in + // docs/design/OBSERVABILITY.md ("AgentCore log delivery"). // --- Session storage (preview) --- // The L2 construct does not yet expose filesystemConfigurations; use the diff --git a/cdk/test/stacks/agent.test.ts b/cdk/test/stacks/agent.test.ts index 219a5b19..d37e3cd7 100644 --- a/cdk/test/stacks/agent.test.ts +++ b/cdk/test/stacks/agent.test.ts @@ -820,6 +820,15 @@ describe('AgentStack', () => { // in. A stack name in a logical id is the signature of the old table. expect(id).not.toContain('backgroundagentdev'); expect(id).not.toContain('AgentRuntimeApplication'); + // Deliberately the library's CURRENT id shape, so this assertion doubles as a + // canary: the next time the AgentCore library renames these resources, this + // fails here rather than in a deploy. Read the failure as an operational + // warning, not a string mismatch — every already-deployed stack will try to + // rename its DeliverySource on the next update, and a renamed source collides + // with the live one on the unchanged runtime ARN and rolls the stack back. + // Update the regex, and add the new shape to the migration note in + // docs/design/OBSERVABILITY.md ("AgentCore log delivery") so operators can + // tell which side of the rename their stack is on. expect(id).toMatch(/^Runtime(Application|Usage)LogsDeliverySource[0-9A-F]{8}$/); } diff --git a/docs/design/OBSERVABILITY.md b/docs/design/OBSERVABILITY.md index b6db7a8d..52824aca 100644 --- a/docs/design/OBSERVABILITY.md +++ b/docs/design/OBSERVABILITY.md @@ -179,6 +179,69 @@ For post-mortems, eval-harness input, and compliance export, the API exposes a s Fields whose source did not run for a given task are returned `null`/empty (e.g. no `--trace` → `trace_uri: null`), so the schema is stable for consumers. +## AgentCore log delivery + +The agent runtime's application and usage logs reach CloudWatch through a trio of resources per log type — `AWS::Logs::DeliverySource`, `AWS::Logs::DeliveryDestination`, and an `AWS::Logs::Delivery` link joining them. The stack does not declare these. The AgentCore `Runtime` L2 construct creates them from the `loggingConfigs` passed to it, and names them from its own internal construct path. + +**The stack deliberately does not override their logical ids.** That is a load-bearing decision rather than an oversight, because the obvious alternatives are worse in a way that is not obvious until a deployment fails. + +### Why a rename of these resources is fatal + +A `DeliverySource` is unique per `(resource ARN, log type)` for the whole account. The agent runtime's ARN does not change when the delivery resources are renamed, so: + +1. Changing a delivery resource's logical id makes CloudFormation treat it as a new resource, which it **creates before deleting** the old one. +2. The new source points at the same runtime ARN as the live one, which still exists at that moment. +3. CloudWatch Logs rejects it — `AlreadyExists`, "This ResourceId has already been used in another Delivery Source in this account." +4. The whole stack update rolls back. + +The counter-intuitive consequence: **no choice of name avoids this.** The conflict is on the ARN the sources point at, not on their own names, so renaming them to library-generated ids, to hand-picked stable ids, or to anything else collides identically. Only leaving a logical id untouched avoids it, because that is what makes CloudFormation update in place instead of creating a second source for the same runtime. + +### Why the ids are not pinned in the template + +An earlier version of the stack held a table of logical ids to override, keyed by stack name, with values read off a live stack. This does not work, because **stack name is not a proxy for deployed state.** Two accounts running a stack of the same name can sit on different library versions' naming, so one set of literals is correct for one account and actively causes the fatal rename on the other. Re-recording the table inverts which account breaks rather than fixing either. + +Since no set of literals can describe every account's deployed state, the stack holds none. Log delivery is left to the library, which generates the ids from the construct path — deterministically, identically in every account, with nothing to keep in sync. + +The cost of this choice is bounded and one-time: a stack deployed before a library-side rename, or held on older ids by the pin table, must converge once. See the migration below. The cost of the alternative was unbounded — a hand-maintained table that silently breaks a different account every time any library version or any deployment moves. + +### Migrating a stack that predates the current naming + +Fresh deployments need nothing here. A stack whose live delivery resources already match what the library generates needs nothing either, and `cdk diff` will show the six resources untouched. + +A stack still on older ids has to converge, and because the create-before-delete collision above applies, the old resources must be gone before the new ones are created. Check first: + +```bash +aws cloudformation list-stack-resources \ + --stack-name backgroundagent-dev \ + --query "StackResourceSummaries[?contains(ResourceType,\ +'Logs::Delivery')].LogicalResourceId" --output text +``` + +Logical ids of the form `RuntimeApplicationLogsDeliverySource` are current — nothing to do. Ids carrying a `CDKSource` segment or the stack name (`RuntimeCDKSourceAPPLICATIONLOGSRuntime`) predate the rename and need the one-time step below. + +Delete the delivery configuration out of band, then deploy. Deliveries first — they reference the source and destination: + +```bash +REGION=us-east-1 +for d in $(aws logs describe-deliveries --region "$REGION" \ + --query 'deliveries[].id' --output text); do + aws logs delete-delivery --region "$REGION" --id "$d" +done +for s in $(aws logs describe-delivery-sources --region "$REGION" \ + --query 'deliverySources[].name' --output text); do + aws logs delete-delivery-source --region "$REGION" --name "$s" +done +for t in $(aws logs describe-delivery-destinations --region "$REGION" \ + --query 'deliveryDestinations[].name' --output text); do + aws logs delete-delivery-destination --region "$REGION" --name "$t" +done +mise //cdk:deploy +``` + +The loops above delete **every** delivery configuration in the account and region, which is what you want on a dedicated deployment account and is not what you want anywhere else. On a shared account, filter to the six resources the previous command listed. + +The deploy then creates the delivery trio under the library's naming and drops the old logical ids, whose underlying resources are already gone — CloudFormation treats a delete of an absent resource as done. Agent logs stop being delivered between the deletion and the end of the deploy; nothing else is affected, and no log data already in CloudWatch is touched. + ## Deployment safety Agent sessions run for up to 8 hours. CDK deployments replace Lambda functions, which can orphan in-flight orchestrator executions. The platform handles this through multiple mechanisms: diff --git a/docs/src/content/docs/architecture/Observability.md b/docs/src/content/docs/architecture/Observability.md index e032fd65..0e50cda7 100644 --- a/docs/src/content/docs/architecture/Observability.md +++ b/docs/src/content/docs/architecture/Observability.md @@ -183,6 +183,69 @@ For post-mortems, eval-harness input, and compliance export, the API exposes a s Fields whose source did not run for a given task are returned `null`/empty (e.g. no `--trace` → `trace_uri: null`), so the schema is stable for consumers. +## AgentCore log delivery + +The agent runtime's application and usage logs reach CloudWatch through a trio of resources per log type — `AWS::Logs::DeliverySource`, `AWS::Logs::DeliveryDestination`, and an `AWS::Logs::Delivery` link joining them. The stack does not declare these. The AgentCore `Runtime` L2 construct creates them from the `loggingConfigs` passed to it, and names them from its own internal construct path. + +**The stack deliberately does not override their logical ids.** That is a load-bearing decision rather than an oversight, because the obvious alternatives are worse in a way that is not obvious until a deployment fails. + +### Why a rename of these resources is fatal + +A `DeliverySource` is unique per `(resource ARN, log type)` for the whole account. The agent runtime's ARN does not change when the delivery resources are renamed, so: + +1. Changing a delivery resource's logical id makes CloudFormation treat it as a new resource, which it **creates before deleting** the old one. +2. The new source points at the same runtime ARN as the live one, which still exists at that moment. +3. CloudWatch Logs rejects it — `AlreadyExists`, "This ResourceId has already been used in another Delivery Source in this account." +4. The whole stack update rolls back. + +The counter-intuitive consequence: **no choice of name avoids this.** The conflict is on the ARN the sources point at, not on their own names, so renaming them to library-generated ids, to hand-picked stable ids, or to anything else collides identically. Only leaving a logical id untouched avoids it, because that is what makes CloudFormation update in place instead of creating a second source for the same runtime. + +### Why the ids are not pinned in the template + +An earlier version of the stack held a table of logical ids to override, keyed by stack name, with values read off a live stack. This does not work, because **stack name is not a proxy for deployed state.** Two accounts running a stack of the same name can sit on different library versions' naming, so one set of literals is correct for one account and actively causes the fatal rename on the other. Re-recording the table inverts which account breaks rather than fixing either. + +Since no set of literals can describe every account's deployed state, the stack holds none. Log delivery is left to the library, which generates the ids from the construct path — deterministically, identically in every account, with nothing to keep in sync. + +The cost of this choice is bounded and one-time: a stack deployed before a library-side rename, or held on older ids by the pin table, must converge once. See the migration below. The cost of the alternative was unbounded — a hand-maintained table that silently breaks a different account every time any library version or any deployment moves. + +### Migrating a stack that predates the current naming + +Fresh deployments need nothing here. A stack whose live delivery resources already match what the library generates needs nothing either, and `cdk diff` will show the six resources untouched. + +A stack still on older ids has to converge, and because the create-before-delete collision above applies, the old resources must be gone before the new ones are created. Check first: + +```bash +aws cloudformation list-stack-resources \ + --stack-name backgroundagent-dev \ + --query "StackResourceSummaries[?contains(ResourceType,\ +'Logs::Delivery')].LogicalResourceId" --output text +``` + +Logical ids of the form `RuntimeApplicationLogsDeliverySource` are current — nothing to do. Ids carrying a `CDKSource` segment or the stack name (`RuntimeCDKSourceAPPLICATIONLOGSRuntime`) predate the rename and need the one-time step below. + +Delete the delivery configuration out of band, then deploy. Deliveries first — they reference the source and destination: + +```bash +REGION=us-east-1 +for d in $(aws logs describe-deliveries --region "$REGION" \ + --query 'deliveries[].id' --output text); do + aws logs delete-delivery --region "$REGION" --id "$d" +done +for s in $(aws logs describe-delivery-sources --region "$REGION" \ + --query 'deliverySources[].name' --output text); do + aws logs delete-delivery-source --region "$REGION" --name "$s" +done +for t in $(aws logs describe-delivery-destinations --region "$REGION" \ + --query 'deliveryDestinations[].name' --output text); do + aws logs delete-delivery-destination --region "$REGION" --name "$t" +done +mise //cdk:deploy +``` + +The loops above delete **every** delivery configuration in the account and region, which is what you want on a dedicated deployment account and is not what you want anywhere else. On a shared account, filter to the six resources the previous command listed. + +The deploy then creates the delivery trio under the library's naming and drops the old logical ids, whose underlying resources are already gone — CloudFormation treats a delete of an absent resource as done. Agent logs stop being delivered between the deletion and the end of the deploy; nothing else is affected, and no log data already in CloudWatch is touched. + ## Deployment safety Agent sessions run for up to 8 hours. CDK deployments replace Lambda functions, which can orphan in-flight orchestrator executions. The platform handles this through multiple mechanisms: