Skip to content

Commit 4272990

Browse files
lym953claude
andcommitted
fix: track first On-Demand invocation with a dedicated flag, not buffer 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>
1 parent 53bd001 commit 4272990

1 file changed

Lines changed: 18 additions & 16 deletions

File tree

bottlecap/src/lifecycle/invocation/processor.rs

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ pub const DATADOG_INVOCATION_ERROR_KEY: &str = "x-datadog-invocation-error";
6363
const DATADOG_SPAN_ID_KEY: &str = "x-datadog-span-id";
6464
const TAG_SAMPLING_PRIORITY: &str = "_sampling_priority_v1";
6565

66+
#[allow(clippy::struct_excessive_bools)]
6667
pub struct Processor {
6768
/// Buffer containing context of the previous 5 invocations
6869
context_buffer: ContextBuffer,
@@ -117,6 +118,13 @@ pub struct Processor {
117118
/// This assumes `platform.initStart` always arrives after the Invoke event, which is what
118119
/// we've observed in production.
119120
pending_first_invocation_metric_timestamp: Option<i64>,
121+
/// Whether the first On-Demand invocation has already been seen.
122+
///
123+
/// Used instead of `context_buffer.is_empty()` to detect the first invocation, since the
124+
/// buffer is also emptied between warm invocations once `platform.report` is processed
125+
/// (an unordered, separate path from `on_invoke_event`), which would otherwise make later
126+
/// invocations look like cold starts too and hold back their metric indefinitely.
127+
first_on_demand_invocation_seen: bool,
120128
}
121129

122130
impl Processor {
@@ -161,15 +169,19 @@ impl Processor {
161169
restore_time: None,
162170
init_duration_metric_emitted: false,
163171
pending_first_invocation_metric_timestamp: None,
172+
first_on_demand_invocation_seen: false,
164173
}
165174
}
166175

167176
/// Given a `request_id`, creates the context and adds the enhanced metric offsets to the context buffer.
168177
///
169178
pub fn on_invoke_event(&mut self, request_id: String) {
170-
// See `pending_first_invocation_metric_timestamp`.
179+
// See `pending_first_invocation_metric_timestamp` and `first_on_demand_invocation_seen`.
171180
let is_on_demand_cold_start =
172-
!self.aws_config.is_managed_instance_mode() && self.context_buffer.is_empty();
181+
!self.aws_config.is_managed_instance_mode() && !self.first_on_demand_invocation_seen;
182+
if is_on_demand_cold_start {
183+
self.first_on_demand_invocation_seen = true;
184+
}
173185

174186
// In Managed Instance mode, if awaiting the first invocation after init, find and update the empty context created on init start
175187
if self.aws_config.is_managed_instance_mode() && self.awaiting_first_invocation {
@@ -2130,19 +2142,9 @@ mod tests {
21302142
// Simulate the On-Demand race: the Invoke event reaches the processor before
21312143
// `platform.initStart`. The invocation metric must not be emitted yet.
21322144
processor.on_invoke_event("req-1".to_string());
2133-
assert!(
2134-
processor
2135-
.pending_first_invocation_metric_timestamp
2136-
.is_some(),
2137-
"Expected the cold start invocation metric to be held back"
2138-
);
2139-
2140-
let now: i64 = std::time::UNIX_EPOCH
2141-
.elapsed()
2142-
.expect("unable to poll clock, unrecoverable")
2143-
.as_secs()
2144-
.try_into()
2145-
.unwrap_or_default();
2145+
let held_back_timestamp = processor
2146+
.pending_first_invocation_metric_timestamp
2147+
.expect("Expected the cold start invocation metric to be held back");
21462148

21472149
// `platform.initStart` arrives afterwards and reports a durable runtime.
21482150
processor.on_platform_init_start(
@@ -2161,7 +2163,7 @@ mod tests {
21612163
.runtime
21622164
.clone()
21632165
.expect("runtime should have been resolved by on_invoke_event");
2164-
let ts = (now / 10) * 10;
2166+
let ts = (held_back_timestamp / 10) * 10;
21652167
let expected_tags = dogstatsd::metric::SortedTags::parse(&format!(
21662168
"cold_start:true,durable_function:true,runtime:{runtime}"
21672169
))

0 commit comments

Comments
 (0)