Skip to content

Commit f46dd48

Browse files
astuyveastuyve
authored
feat: honor enhanced metrics bool (#307)
* feat: honor enhanced metrics bool * feat: add test * feat: refactor to log instead of return result * fix: clippy
1 parent d5f51ec commit f46dd48

File tree

3 files changed

+123
-44
lines changed

3 files changed

+123
-44
lines changed

‎bottlecap/src/bin/bottlecap/main.rs‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,8 @@ async fn extension_loop_active(
313313
error!("Error starting trace agent: {e:?}");
314314
}
315315
});
316-
let lambda_enhanced_metrics = enhanced_metrics::new(Arc::clone(&metrics_aggr));
316+
let lambda_enhanced_metrics =
317+
enhanced_metrics::new(Arc::clone(&metrics_aggr), Arc::clone(config));
317318
let dogstatsd_cancel_token = start_dogstatsd(event_bus.get_sender_copy(), &metrics_aggr).await;
318319

319320
let telemetry_listener_cancel_token =
@@ -335,9 +336,7 @@ async fn extension_loop_active(
335336
"[extension_next] Invoke event {}; deadline: {}, invoked_function_arn: {}",
336337
request_id, deadline_ms, invoked_function_arn
337338
);
338-
if let Err(e) = lambda_enhanced_metrics.increment_invocation_metric() {
339-
error!("Failed to increment invocation metric: {e:?}");
340-
}
339+
lambda_enhanced_metrics.increment_invocation_metric();
341340
}
342341
Ok(NextEventResponse::Shutdown {
343342
shutdown_reason,
@@ -375,11 +374,8 @@ async fn extension_loop_active(
375374
metrics,
376375
} => {
377376
debug!("Platform init report for initialization_type: {:?} with phase: {:?} and metrics: {:?}", initialization_type, phase, metrics);
378-
if let Err(e) = lambda_enhanced_metrics
379-
.set_init_duration_metric(metrics.duration_ms)
380-
{
381-
error!("Failed to set init duration metric: {e:?}");
382-
}
377+
lambda_enhanced_metrics
378+
.set_init_duration_metric(metrics.duration_ms);
383379
}
384380
TelemetryRecord::PlatformRuntimeDone {
385381
request_id,
@@ -395,17 +391,9 @@ async fn extension_loop_active(
395391
}
396392

397393
if status != Status::Success {
398-
if let Err(e) =
399-
lambda_enhanced_metrics.increment_errors_metric()
400-
{
401-
error!("Failed to increment error metric: {e:?}");
402-
}
394+
lambda_enhanced_metrics.increment_errors_metric();
403395
if status == Status::Timeout {
404-
if let Err(e) =
405-
lambda_enhanced_metrics.increment_timeout_metric()
406-
{
407-
error!("Failed to increment timeout metric: {e:?}");
408-
}
396+
lambda_enhanced_metrics.increment_timeout_metric();
409397
}
410398
}
411399
debug!(

‎bottlecap/src/config/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ pub struct Config {
4343
pub merge_xray_traces: bool,
4444
pub serverless_appsec_enabled: bool,
4545
pub extension_version: Option<String>,
46+
pub enhanced_metrics: bool,
4647
}
4748

4849
impl Default for Config {
@@ -75,6 +76,7 @@ impl Default for Config {
7576
merge_xray_traces: false,
7677
serverless_appsec_enabled: false,
7778
extension_version: None,
79+
enhanced_metrics: true,
7880
}
7981
}
8082
}

‎bottlecap/src/metrics/enhanced/lambda.rs‎

Lines changed: 114 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,60 +1,81 @@
11
use super::constants::{self, BASE_LAMBDA_INVOCATION_PRICE};
22
use crate::metrics::aggregator::Aggregator;
3-
use crate::metrics::{errors, metric};
3+
use crate::metrics::metric;
44
use crate::telemetry::events::ReportMetrics;
55
use std::env::consts::ARCH;
66
use std::sync::{Arc, Mutex};
77
use tracing::error;
88

99
pub struct Lambda {
1010
pub aggregator: Arc<Mutex<Aggregator<1024>>>,
11+
pub config: Arc<crate::config::Config>,
1112
}
1213

1314
impl Lambda {
1415
#[must_use]
15-
pub fn new(aggregator: Arc<Mutex<Aggregator<1024>>>) -> Lambda {
16-
Lambda { aggregator }
16+
pub fn new(
17+
aggregator: Arc<Mutex<Aggregator<1024>>>,
18+
config: Arc<crate::config::Config>,
19+
) -> Lambda {
20+
Lambda { aggregator, config }
1721
}
1822

19-
pub fn increment_invocation_metric(&self) -> Result<(), errors::Insert> {
20-
self.increment_metric(constants::INVOCATIONS_METRIC)
23+
pub fn increment_invocation_metric(&self) {
24+
self.increment_metric(constants::INVOCATIONS_METRIC);
2125
}
2226

23-
pub fn increment_errors_metric(&self) -> Result<(), errors::Insert> {
24-
self.increment_metric(constants::ERRORS_METRIC)
27+
pub fn increment_errors_metric(&self) {
28+
self.increment_metric(constants::ERRORS_METRIC);
2529
}
2630

27-
pub fn increment_timeout_metric(&self) -> Result<(), errors::Insert> {
28-
self.increment_metric(constants::TIMEOUTS_METRIC)
31+
pub fn increment_timeout_metric(&self) {
32+
self.increment_metric(constants::TIMEOUTS_METRIC);
2933
}
3034

31-
pub fn set_init_duration_metric(&self, init_duration_ms: f64) -> Result<(), errors::Insert> {
35+
pub fn set_init_duration_metric(&self, init_duration_ms: f64) {
36+
if !self.config.enhanced_metrics {
37+
return;
38+
}
3239
let metric = metric::Metric::new(
3340
constants::INIT_DURATION_METRIC.into(),
3441
metric::Type::Distribution,
3542
(init_duration_ms * constants::MS_TO_SEC).to_string().into(),
3643
None,
3744
);
38-
self.aggregator
45+
if let Err(e) = self
46+
.aggregator
3947
.lock()
4048
.expect("lock poisoned")
4149
.insert(&metric)
50+
{
51+
error!("failed to insert metric: {}", e);
52+
}
4253
}
4354

44-
fn increment_metric(&self, metric_name: &str) -> Result<(), errors::Insert> {
55+
fn increment_metric(&self, metric_name: &str) {
56+
if !self.config.enhanced_metrics {
57+
return;
58+
}
4559
let metric = metric::Metric::new(
4660
metric_name.into(),
4761
metric::Type::Distribution,
4862
"1".into(),
4963
None,
5064
);
51-
self.aggregator
65+
if let Err(e) = self
66+
.aggregator
5267
.lock()
5368
.expect("lock poisoned")
5469
.insert(&metric)
70+
{
71+
error!("failed to insert metric: {}", e);
72+
}
5573
}
5674

5775
pub fn set_runtime_duration_metric(&self, duration_ms: f64) {
76+
if !self.config.enhanced_metrics {
77+
return;
78+
}
5879
let metric = metric::Metric::new(
5980
constants::RUNTIME_DURATION_METRIC.into(),
6081
metric::Type::Distribution,
@@ -73,6 +94,9 @@ impl Lambda {
7394
}
7495

7596
pub fn set_post_runtime_duration_metric(&self, duration_ms: f64) {
97+
if !self.config.enhanced_metrics {
98+
return;
99+
}
76100
let metric = metric::Metric::new(
77101
constants::POST_RUNTIME_DURATION_METRIC.into(),
78102
metric::Type::Distribution,
@@ -107,6 +131,9 @@ impl Lambda {
107131
}
108132

109133
pub fn set_report_log_metrics(&self, metrics: &ReportMetrics) {
134+
if !self.config.enhanced_metrics {
135+
return;
136+
}
110137
let mut aggr: std::sync::MutexGuard<Aggregator<1024>> =
111138
self.aggregator.lock().expect("lock poisoned");
112139
let metric = metric::Metric::new(
@@ -176,7 +203,7 @@ mod tests {
176203
use std::collections::hash_map::HashMap;
177204
use std::sync::MutexGuard;
178205

179-
fn setup() -> Arc<Mutex<Aggregator<1024>>> {
206+
fn setup() -> (Arc<Mutex<Aggregator<1024>>>, Arc<config::Config>) {
180207
let config = Arc::new(config::Config {
181208
service: Some("test-service".to_string()),
182209
tags: Some("test:tags".to_string()),
@@ -187,17 +214,21 @@ mod tests {
187214
LAMBDA_RUNTIME_SLUG.to_string(),
188215
&HashMap::new(),
189216
));
190-
Arc::new(Mutex::new(
191-
Aggregator::<1024>::new(tags_provider.clone()).expect("failed to create aggregator"),
192-
))
217+
(
218+
Arc::new(Mutex::new(
219+
Aggregator::<1024>::new(tags_provider.clone())
220+
.expect("failed to create aggregator"),
221+
)),
222+
config,
223+
)
193224
}
194225

195226
#[test]
196227
#[allow(clippy::float_cmp)]
197228
fn test_increment_invocation_metric() {
198-
let metrics_aggr = setup();
199-
let lambda = Lambda::new(metrics_aggr.clone());
200-
lambda.increment_invocation_metric().unwrap();
229+
let (metrics_aggr, my_config) = setup();
230+
let lambda = Lambda::new(metrics_aggr.clone(), my_config);
231+
lambda.increment_invocation_metric();
201232
match metrics_aggr
202233
.lock()
203234
.expect("lock poisoned")
@@ -211,9 +242,9 @@ mod tests {
211242
#[test]
212243
#[allow(clippy::float_cmp)]
213244
fn test_increment_errors_metric() {
214-
let metrics_aggr = setup();
215-
let lambda = Lambda::new(metrics_aggr.clone());
216-
lambda.increment_errors_metric().unwrap();
245+
let (metrics_aggr, my_config) = setup();
246+
let lambda = Lambda::new(metrics_aggr.clone(), my_config);
247+
lambda.increment_errors_metric();
217248
match metrics_aggr
218249
.lock()
219250
.expect("lock poisoned")
@@ -224,10 +255,68 @@ mod tests {
224255
};
225256
}
226257

258+
#[test]
259+
fn test_disabled() {
260+
let (metrics_aggr, no_config) = setup();
261+
let my_config = Arc::new(config::Config {
262+
enhanced_metrics: false,
263+
..no_config.as_ref().clone()
264+
});
265+
let lambda = Lambda::new(metrics_aggr.clone(), my_config);
266+
lambda.increment_invocation_metric();
267+
lambda.increment_errors_metric();
268+
lambda.increment_timeout_metric();
269+
lambda.set_init_duration_metric(100.0);
270+
lambda.set_runtime_duration_metric(100.0);
271+
lambda.set_post_runtime_duration_metric(100.0);
272+
lambda.set_report_log_metrics(&ReportMetrics {
273+
duration_ms: 100.0,
274+
billed_duration_ms: 100,
275+
max_memory_used_mb: 128,
276+
memory_size_mb: 256,
277+
init_duration_ms: Some(50.0),
278+
restore_duration_ms: None,
279+
});
280+
let mut aggr = metrics_aggr.lock().expect("lock poisoned");
281+
assert!(aggr
282+
.get_value_by_id(constants::INVOCATIONS_METRIC.into(), None)
283+
.is_none());
284+
assert!(aggr
285+
.get_value_by_id(constants::ERRORS_METRIC.into(), None)
286+
.is_none());
287+
assert!(aggr
288+
.get_value_by_id(constants::TIMEOUTS_METRIC.into(), None)
289+
.is_none());
290+
assert!(aggr
291+
.get_value_by_id(constants::INIT_DURATION_METRIC.into(), None)
292+
.is_none());
293+
assert!(aggr
294+
.get_value_by_id(constants::RUNTIME_DURATION_METRIC.into(), None)
295+
.is_none());
296+
assert!(aggr
297+
.get_value_by_id(constants::POST_RUNTIME_DURATION_METRIC.into(), None)
298+
.is_none());
299+
assert!(aggr
300+
.get_value_by_id(constants::DURATION_METRIC.into(), None)
301+
.is_none());
302+
assert!(aggr
303+
.get_value_by_id(constants::BILLED_DURATION_METRIC.into(), None)
304+
.is_none());
305+
assert!(aggr
306+
.get_value_by_id(constants::MAX_MEMORY_USED_METRIC.into(), None)
307+
.is_none());
308+
assert!(aggr
309+
.get_value_by_id(constants::MEMORY_SIZE_METRIC.into(), None)
310+
.is_none());
311+
assert!(aggr
312+
.get_value_by_id(constants::ESTIMATED_COST_METRIC.into(), None)
313+
.is_none());
314+
}
315+
227316
#[test]
228317
fn test_set_report_log_metrics() {
229-
let metrics_aggr = setup();
230-
let lambda = Lambda::new(metrics_aggr.clone());
318+
let (metrics_aggr, my_config) = setup();
319+
let lambda = Lambda::new(metrics_aggr.clone(), my_config);
231320
let report_metrics = ReportMetrics {
232321
duration_ms: 100.0,
233322
billed_duration_ms: 100,

0 commit comments

Comments
 (0)