Skip to content

[SVLS-8800] fix: hold back cold-start invocation metric until durable tag is known#1301

Open
lym953 wants to merge 17 commits into
mainfrom
yiming.luo/durable-metric-tag-cold-start
Open

[SVLS-8800] fix: hold back cold-start invocation metric until durable tag is known#1301
lym953 wants to merge 17 commits into
mainfrom
yiming.luo/durable-metric-tag-cold-start

Conversation

@lym953

@lym953 lym953 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Problem

For durable functions, the aws.lambda.enhanced.invocations metric for the first invocation (after cold start) doesn't have the tag durable_function:true as expected.

This is because the metric is emitted in on_invoke_event(), which, from what we observed, always happens before the extension receives platform.initStart event, which carries the information of whether the function is a durable function. As a result, the durable_function:true tag is only set on the metric for subsequent invocations but not for the first one.

Fix

Delay the flushing of aws.lambda.enhanced.invocations metric 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:

enum FirstInvocationMetricStatus {
    Pending,
    InitStartSeen,
    HeldBack(i64),
    Resolved,
}

This covers both cases about the order of on_invoke_event() event and platform.initStart event:

  1. Common case: on_invoke_event() comes first, then platform.initStart:
    • To start with: Pending
    • on_invoke_event() comes: hold metric and move to HeldBack
    • platform.initStart comes: flush metric and move to Resolved
  2. Rare case: platform.initStart comes before on_invoke_event()
    • To start with: Pending
    • platform.initStart comes: InitStartSeen
    • on_invoke_event() comes: flush metric immediately and move to HeldBack

Also, flush the metric at shutdown, in case platform.initStart is never received for whatever reason.

Testing

Manual testing

For a durable function, all data points for the aws.lambda.enhanced.invocations metric have the tag durable_function:true set.
image

Automated testing

Passed the added integration test.

…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.
@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Jul 11, 2026

Copy link
Copy Markdown

Tests

🔄 Datadog auto-retried 3 jobs - 2 passed on retry View in Datadog

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: cc19dee | Docs | Datadog PR Page | Give us feedback!

lym953 and others added 10 commits July 13, 2026 15:31
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>
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>
@lym953 lym953 changed the title fix(metrics): hold back cold-start invocation metric until durable tag is known [SVLS-8800] fix: hold back cold-start invocation metric until durable tag is known Jul 13, 2026
@lym953 lym953 requested a review from Copilot July 13, 2026 20:44
@lym953 lym953 marked this pull request as ready for review July 13, 2026 20:44
@lym953 lym953 requested a review from a team as a code owner July 13, 2026 20:44
@lym953 lym953 requested a review from jchrostek-dd July 13, 2026 20:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread bottlecap/src/lifecycle/invocation/processor.rs Outdated
Comment thread integration-tests/tests/durable-cold-start.test.ts
Comment thread bottlecap/src/lifecycle/invocation/processor.rs Outdated
Comment thread bottlecap/src/lifecycle/invocation/processor.rs Outdated
Comment thread bottlecap/src/lifecycle/invocation/processor.rs
Comment thread bottlecap/src/lifecycle/invocation/processor.rs
@jchrostek-dd

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

lym953 and others added 6 commits July 14, 2026 13:16
…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>
@lym953

lym953 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Oh no, I see duplicate logs in Datadog. Let me fix it.

@lym953 lym953 marked this pull request as draft July 14, 2026 18:50
@lym953

lym953 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

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.

@lym953 lym953 marked this pull request as ready for review July 15, 2026 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants