[SVLS-8800] fix: hold back cold-start invocation metric until durable tag is known#1301
[SVLS-8800] fix: hold back cold-start invocation metric until durable tag is known#1301lym953 wants to merge 18 commits into
Conversation
…g is known In On-Demand mode, the Extensions API's `/next` poll (driving on_invoke_event) consistently beats the buffered Telemetry API's platform.initStart (which sets the durable_function tag), so the first invocation's aws.lambda.enhanced.invocations metric almost always missed durable_function:true. Hold the sandbox's first invocation metric in a pending field and flush it once platform.initStart has set the durable tag, with fallbacks on the next invoke or shutdown so the metric is never dropped if initStart never arrives.
This comment has been minimized.
This comment has been minimized.
Deploys a real durable-configured Lambda alongside a plain guard function and asserts the cold-start aws.lambda.enhanced.invocations metric carries durable_function:true only for the durable one, exercising the fix in a905347 end-to-end. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…field platform.initStart not arriving before the next invocation is not a realistic scenario, so drop that fallback flush and the test covering it. Renamed pending_first_invocation_metric to pending_first_invocation_metric_timestamp to reflect that it stores a timestamp, and clarified "cold-start invocation metric in On-Demand mode" comments. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
If the Invoke event reaches the processor after platform.initStart (the reverse of the usual On-Demand race), on_invoke_event would still see an empty context_buffer and hold the metric back a second time, with nothing left to flush it until shutdown. Track whether platform.initStart already ran and skip the hold-back in that case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…already ran" This reverts commit 35ee072.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Invoke both functions a second time after the cold start and check the tag in a separate time window, to confirm the tag isn't only set on invocation #1 but persists across warm invocations. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces the two-window/two-5-min-indexing-wait design with one window and one wait, comparing tagged vs. total invocation counts (new getMetricCountWithTag helper) instead of checking existence per window. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e checks Each function is invoked exactly twice, so assert toBe(2) instead of a loose lower bound. Also add a guideline to AGENTS.md against unnecessary defensive checks/tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a tagging gap in Bottlecap’s enhanced invocation metric for Durable Functions by delaying the first aws.lambda.enhanced.invocations emission until platform.initStart has been processed (so the durable_function:true tag can be applied), with a shutdown fallback. It also adds an integration test and the associated CDK stack to validate the tag is present for all durable invocations and absent for non-durable ones.
Changes:
- Hold back the first On-Demand invocation metric until
platform.initStart(or flush at shutdown as fallback). - Add an integration test plus a new CDK stack deploying durable vs non-durable Lambdas to validate tagging.
- Extend Datadog test utilities with a helper to query metric counts filtered by a tag.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
bottlecap/src/lifecycle/invocation/processor.rs |
Holds back the first invocation metric and flushes it on platform.initStart or shutdown; adds a unit test for the race. |
integration-tests/tests/utils/datadog.ts |
Adds getMetricCountWithTag helper for tag-filtered metric count queries. |
integration-tests/tests/durable-cold-start.test.ts |
New integration test validating durable_function:true tagging behavior across cold + warm invocations. |
integration-tests/lib/stacks/durable-cold-start.ts |
New CDK stack deploying durable and non-durable functions for the integration test. |
integration-tests/bin/app.ts |
Registers the new DurableColdStart stack in the integration test app. |
.gitlab/datasources/test-suites.yaml |
Adds the durable-cold-start suite to CI test suite definitions. |
AGENTS.md |
Adds a small code-style guideline note for agents. |
|
Do you think we should add a comment to agents.md about the ordering of lambda events so agents are aware and proactively catch issues before they are bugs? |
| /// `platform.initStart` sets the durable tag so it's not missed on invocation #1. | ||
| /// | ||
| /// This assumes `platform.initStart` always arrives after the Invoke event, which is what | ||
| /// we've observed in production. |
There was a problem hiding this comment.
Is this actually a guarantee? That platform.initStart must appear after the first invoke event? Or is is just likely to appear after the first invoke event?
There was a problem hiding this comment.
It's not a guarantee. This is just the order I consistently observed from my tests, and I didn't want to over-engineer.
Since you raised this, let me make code robust by handling both cases.
…er emptiness context_buffer.is_empty() is also true between warm invocations once platform.report is processed (an unordered, separate path from on_invoke_event), so it could wrongly re-trigger the cold-start hold-back on later invocations and drop their metric. Track it explicitly instead. Also fix a test that recomputed `now` after on_invoke_event to derive the expected metric bucket, which could mismatch the actual timestamp if the call straddled a 10s boundary; capture the held-back timestamp instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dy processed Handles both orderings of the Invoke event and platform.initStart. If initStart has already resolved the durable tag by the time invocation #1 arrives, there's nothing to wait for, so emit the metric right away instead of holding it back (which previously would only be flushed at shutdown, since platform.initStart doesn't fire again). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces pending_first_invocation_metric_timestamp, first_on_demand_invocation_seen, and platform_init_start_processed with one FirstInvocationMetric enum (Pending/InitStartSeen/HeldBack/Resolved), making the state machine explicit instead of spread across three separate fields. Also drops the struct_excessive_bools allow, since the struct is back to 3 bool fields. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tatus Field/enum name now reflects that it tracks a status, not the metric value itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…omment Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Oh no, I see duplicate logs in Datadog. Let me fix it. |
That's unrelated to this PR. Marking the PR as Ready for Review again. |
Problem
For durable functions, the
aws.lambda.enhanced.invocationsmetric for the first invocation (after cold start) doesn't have the tagdurable_function:trueas expected.This is because the metric is emitted in
on_invoke_event(), which, from what we observed, always happens before the extension receivesplatform.initStartevent, which carries the information of whether the function is a durable function. As a result, thedurable_function:truetag is only set on the metric for subsequent invocations but not for the first one.Fix
Delay the flushing of
aws.lambda.enhanced.invocationsmetric for the first invocation, if the extension doesn't yet know whether the function is a durable function.Use an enum to track the state of the first invocation:
This covers both cases about the order of
on_invoke_event()event andplatform.initStartevent:on_invoke_event()comes first, thenplatform.initStart:Pendingon_invoke_event()comes: hold metric and move toHeldBackplatform.initStartcomes: flush metric and move toResolvedplatform.initStartcomes beforeon_invoke_event()Pendingplatform.initStartcomes:InitStartSeenon_invoke_event()comes: flush metric immediately and move toHeldBackAlso, flush the metric at shutdown, in case
platform.initStartis never received for whatever reason.Testing
Manual testing
For a durable function, all data points for the

aws.lambda.enhanced.invocationsmetric have the tagdurable_function:trueset.Automated testing
Passed the added integration test.