Skip to content

Commit

Permalink
Slight metrics improvements (#1387)
Browse files Browse the repository at this point in the history
- Avoid `format!`-ing metric keys
- Use macros instead of manual invocations
- Add a metrics for the JS `file_in_bundle` cache
  • Loading branch information
Swatinem authored Feb 27, 2024
1 parent c5d7ecc commit 0225e44
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 20 deletions.
3 changes: 3 additions & 0 deletions crates/symbolicator-js/src/bundle_lookup.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::fmt;
use std::time::Duration;

use symbolicator_service::metric;
use symbolicator_sources::RemoteFileUri;

use crate::interface::ResolvedWith;
Expand Down Expand Up @@ -62,10 +63,12 @@ impl FileInBundleCache {
// XXX: this is a really horrible workaround for not being able to look up things via `(&A, &B)` instead of `&(A, B)`.
let lookup_key = (bundle_uri, key);
if let Some((file_entry, resolved_with)) = self.cache.get(&lookup_key) {
metric!(counter("js.file_in_bundle.hit") += 1);
return Some((lookup_key.0, file_entry, resolved_with));
}
key = lookup_key.1;
}
metric!(counter("js.file_in_bundle.miss") += 1);
None
}

Expand Down
8 changes: 4 additions & 4 deletions crates/symbolicator-service/src/download/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -615,9 +615,9 @@ impl Drop for MeasureSourceDownloadGuard<'_> {
};

let duration = self.creation_time.elapsed();
let metric_name = format!("{}.duration", self.task_name);
metric!(
timer(&metric_name) = duration,
timer("download_duration") = duration,
"task_name" => self.task_name,
"status" => status,
"source" => self.source_name,
);
Expand All @@ -631,9 +631,9 @@ impl Drop for MeasureSourceDownloadGuard<'_> {
.checked_div(duration.as_millis())
.and_then(|t| t.try_into().ok())
.unwrap_or(bytes_transferred);
let throughput_name = format!("{}.throughput", self.task_name);
metric!(
histogram(&throughput_name) = throughput,
histogram("download_throughput") = throughput,
"task_name" => self.task_name,
"status" => status,
"source" => self.source_name,
);
Expand Down
24 changes: 9 additions & 15 deletions crates/symbolicator-service/src/utils/futures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ use std::time::Instant;

use tokio::task::JoinHandle;

use crate::metrics::{self, prelude::*};

/// Execute a callback on dropping of the container type.
///
/// The callback must not panic under any circumstance. Since it is called while dropping an item,
Expand Down Expand Up @@ -100,12 +98,10 @@ impl<'a> MeasureGuard<'a> {
/// By default, the future is waiting to be polled. `start` emits the `futures.wait_time`
/// metric.
pub fn start(&mut self) {
metrics::with_client(|client| {
client
.time_with_tags("futures.wait_time", self.creation_time.elapsed())
.with_tag("task_name", self.task_name)
.send();
})
metric!(
timer("futures.wait_time") = self.creation_time.elapsed(),
"task_name" => self.task_name,
);
}

/// Marks the future as terminated and emits the `futures.done` metric.
Expand All @@ -120,13 +116,11 @@ impl Drop for MeasureGuard<'_> {
MeasureState::Pending => "canceled",
MeasureState::Done(status) => status,
};
metrics::with_client(|client| {
client
.time_with_tags("futures.done", self.creation_time.elapsed())
.with_tag("task_name", self.task_name)
.with_tag("status", status)
.send();
})
metric!(
timer("futures.done") = self.creation_time.elapsed(),
"task_name" => self.task_name,
"status" => status,
);
}
}

Expand Down
5 changes: 4 additions & 1 deletion crates/symbolicator/src/endpoints/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ where
.as_ref()
.map(|r| r.status())
.unwrap_or(StatusCode::INTERNAL_SERVER_ERROR);
metric!(counter(&format!("responses.status_code.{status}")) += 1);
metric!(
counter("responses.status_code") += 1,
"status" => status.as_str(),
);
}
poll
}
Expand Down

0 comments on commit 0225e44

Please sign in to comment.