From 2ac3b97f78f31a575b8a6e991a7e4af9b3ce0664 Mon Sep 17 00:00:00 2001 From: shreyamalpani Date: Fri, 1 Nov 2024 15:29:05 -0400 Subject: [PATCH 1/4] create context on invoke event --- bottlecap/src/lifecycle/invocation/context.rs | 38 +++++++------------ .../src/lifecycle/invocation/processor.rs | 2 + 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/bottlecap/src/lifecycle/invocation/context.rs b/bottlecap/src/lifecycle/invocation/context.rs index 9c00dea30..be442aa22 100644 --- a/bottlecap/src/lifecycle/invocation/context.rs +++ b/bottlecap/src/lifecycle/invocation/context.rs @@ -93,8 +93,13 @@ impl ContextBuffer { .find(|context| context.request_id == *request_id) } - /// Adds the init duration to a `Context` in the buffer. If the `Context` is not found, a new - /// `Context` is created and added to the buffer. + /// Creates a new `Context` and adds it to the buffer. + /// + pub fn create_context(&mut self, request_id: &String) { + self.insert(Context::new(request_id.clone(), 0.0, 0.0, 0, None)); + } + + /// Adds the init duration to a `Context` in the buffer. /// pub fn add_init_duration(&mut self, request_id: &String, init_duration_ms: f64) { if let Some(context) = self @@ -104,18 +109,11 @@ impl ContextBuffer { { context.init_duration_ms = init_duration_ms; } else { - self.insert(Context::new( - request_id.clone(), - 0.0, - init_duration_ms, - 0, - None, - )); + debug!("Could not add init duration - context not found") } } - /// Adds the start time to a `Context` in the buffer. If the `Context` is not found, a new - /// `Context` is created and added to the buffer. + /// Adds the start time to a `Context` in the buffer. /// pub fn add_start_time(&mut self, request_id: &String, start_time: i64) { if let Some(context) = self @@ -125,12 +123,11 @@ impl ContextBuffer { { context.start_time = start_time; } else { - self.insert(Context::new(request_id.clone(), 0.0, 0.0, start_time, None)); + debug!("Could not add start time - context not found") } } - /// Adds the runtime duration to a `Context` in the buffer. If the `Context` is not found, a new - /// `Context` is created and added to the buffer. + /// Adds the runtime duration to a `Context` in the buffer. /// pub fn add_runtime_duration(&mut self, request_id: &String, runtime_duration_ms: f64) { if let Some(context) = self @@ -140,18 +137,11 @@ impl ContextBuffer { { context.runtime_duration_ms = runtime_duration_ms; } else { - self.insert(Context::new( - request_id.clone(), - runtime_duration_ms, - 0.0, - 0, - None, - )); + debug!("Could not add runtime duration - context not found") } } - /// Adds the network offset to a `Context` in the buffer. If the `Context` is not found, a new - /// `Context` is created and added to the buffer. + /// Adds the network offset to a `Context` in the buffer. /// pub fn add_network_offset(&mut self, request_id: &String, network_data: Option) { if let Some(context) = self @@ -161,7 +151,7 @@ impl ContextBuffer { { context.network_offset = network_data; } else { - self.insert(Context::new(request_id.clone(), 0.0, 0.0, 0, network_data)); + debug!("Could not add network offset - context not found") } } diff --git a/bottlecap/src/lifecycle/invocation/processor.rs b/bottlecap/src/lifecycle/invocation/processor.rs index 112791e9a..108731fcd 100644 --- a/bottlecap/src/lifecycle/invocation/processor.rs +++ b/bottlecap/src/lifecycle/invocation/processor.rs @@ -79,6 +79,8 @@ impl Processor { /// Given a `request_id`, add the enhanced metric offsets to the context buffer. /// pub fn on_invoke_event(&mut self, request_id: String) { + self.context_buffer.create_context(&request_id); + let network_offset: Option = proc::get_network_data().ok(); self.context_buffer .add_network_offset(&request_id, network_offset); From 88eb9a278f9eecbf131e124adc398f98598afafb Mon Sep 17 00:00:00 2001 From: shreyamalpani Date: Fri, 1 Nov 2024 15:31:06 -0400 Subject: [PATCH 2/4] update tests --- bottlecap/src/lifecycle/invocation/context.rs | 36 ------------------- 1 file changed, 36 deletions(-) diff --git a/bottlecap/src/lifecycle/invocation/context.rs b/bottlecap/src/lifecycle/invocation/context.rs index be442aa22..248a361bc 100644 --- a/bottlecap/src/lifecycle/invocation/context.rs +++ b/bottlecap/src/lifecycle/invocation/context.rs @@ -255,15 +255,6 @@ mod tests { buffer.add_init_duration(&request_id, 100.0); assert_eq!(buffer.get(&request_id).unwrap().init_duration_ms, 100.0); - - // Add init duration to a context that doesn't exist - let unexistent_request_id = String::from("unexistent"); - buffer.add_init_duration(&unexistent_request_id, 200.0); - assert_eq!(buffer.size(), 2); - assert_eq!( - buffer.get(&unexistent_request_id).unwrap().init_duration_ms, - 200.0 - ); } #[test] @@ -278,12 +269,6 @@ mod tests { buffer.add_start_time(&request_id, 100); assert_eq!(buffer.get(&request_id).unwrap().start_time, 100); - - // Add start time to a context that doesn't exist - let unexistent_request_id = String::from("unexistent"); - buffer.add_start_time(&unexistent_request_id, 200); - assert_eq!(buffer.size(), 2); - assert_eq!(buffer.get(&unexistent_request_id).unwrap().start_time, 200); } #[test] @@ -298,18 +283,6 @@ mod tests { buffer.add_runtime_duration(&request_id, 100.0); assert_eq!(buffer.get(&request_id).unwrap().runtime_duration_ms, 100.0); - - // Add runtime duration to a context that doesn't exist - let unexistent_request_id = String::from("unexistent"); - buffer.add_runtime_duration(&unexistent_request_id, 200.0); - assert_eq!(buffer.size(), 2); - assert_eq!( - buffer - .get(&unexistent_request_id) - .unwrap() - .runtime_duration_ms, - 200.0 - ); } #[test] @@ -332,14 +305,5 @@ mod tests { buffer.get(&request_id).unwrap().network_offset, network_offset, ); - - // Add network offset to a context that doesn't exist - let unexistent_request_id = String::from("unexistent"); - buffer.add_network_offset(&unexistent_request_id, network_offset); - assert_eq!(buffer.size(), 2); - assert_eq!( - buffer.get(&unexistent_request_id).unwrap().network_offset, - network_offset - ); } } From e6b6717689de276fff61195dc76da021b9f89ada Mon Sep 17 00:00:00 2001 From: shreyamalpani Date: Mon, 4 Nov 2024 10:18:27 -0500 Subject: [PATCH 3/4] clippy fixes --- bottlecap/src/lifecycle/invocation/context.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bottlecap/src/lifecycle/invocation/context.rs b/bottlecap/src/lifecycle/invocation/context.rs index 248a361bc..1c8fdb256 100644 --- a/bottlecap/src/lifecycle/invocation/context.rs +++ b/bottlecap/src/lifecycle/invocation/context.rs @@ -95,6 +95,7 @@ impl ContextBuffer { /// Creates a new `Context` and adds it to the buffer. /// + #[allow(clippy::ptr_arg)] pub fn create_context(&mut self, request_id: &String) { self.insert(Context::new(request_id.clone(), 0.0, 0.0, 0, None)); } @@ -109,7 +110,7 @@ impl ContextBuffer { { context.init_duration_ms = init_duration_ms; } else { - debug!("Could not add init duration - context not found") + debug!("Could not add init duration - context not found"); } } @@ -123,7 +124,7 @@ impl ContextBuffer { { context.start_time = start_time; } else { - debug!("Could not add start time - context not found") + debug!("Could not add start time - context not found"); } } @@ -137,7 +138,7 @@ impl ContextBuffer { { context.runtime_duration_ms = runtime_duration_ms; } else { - debug!("Could not add runtime duration - context not found") + debug!("Could not add runtime duration - context not found"); } } @@ -151,7 +152,7 @@ impl ContextBuffer { { context.network_offset = network_data; } else { - debug!("Could not add network offset - context not found") + debug!("Could not add network offset - context not found"); } } From 9d0af310c29a0c142910bd2db9f16ad55f028f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?jordan=20gonz=C3=A1lez?= <30836115+duncanista@users.noreply.github.com> Date: Tue, 5 Nov 2024 13:13:55 -0500 Subject: [PATCH 4/4] remove `allow(clippy::ptr_arg)` --- bottlecap/src/lifecycle/invocation/context.rs | 5 ++--- bottlecap/src/lifecycle/invocation/processor.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/bottlecap/src/lifecycle/invocation/context.rs b/bottlecap/src/lifecycle/invocation/context.rs index 1c8fdb256..6fef5a673 100644 --- a/bottlecap/src/lifecycle/invocation/context.rs +++ b/bottlecap/src/lifecycle/invocation/context.rs @@ -95,9 +95,8 @@ impl ContextBuffer { /// Creates a new `Context` and adds it to the buffer. /// - #[allow(clippy::ptr_arg)] - pub fn create_context(&mut self, request_id: &String) { - self.insert(Context::new(request_id.clone(), 0.0, 0.0, 0, None)); + pub fn create_context(&mut self, request_id: String) { + self.insert(Context::new(request_id, 0.0, 0.0, 0, None)); } /// Adds the init duration to a `Context` in the buffer. diff --git a/bottlecap/src/lifecycle/invocation/processor.rs b/bottlecap/src/lifecycle/invocation/processor.rs index 108731fcd..4e5c34e3a 100644 --- a/bottlecap/src/lifecycle/invocation/processor.rs +++ b/bottlecap/src/lifecycle/invocation/processor.rs @@ -79,7 +79,7 @@ impl Processor { /// Given a `request_id`, add the enhanced metric offsets to the context buffer. /// pub fn on_invoke_event(&mut self, request_id: String) { - self.context_buffer.create_context(&request_id); + self.context_buffer.create_context(request_id.clone()); let network_offset: Option = proc::get_network_data().ok(); self.context_buffer