Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions linkerd/http/prom/src/record_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,27 @@ where
struct ResponseState<L: StreamLabel> {
labeler: L,
duration: DurationFamily<L::DurationLabels>,
start: oneshot::Receiver<time::Instant>,
start: StartTime,
}

/// Start time for a duration measurement.
///
/// Request duration knows the start time immediately whereas response duration
/// needs to wait until the request body is fully flushed.
pub(crate) enum StartTime {
/// Start time is already known.
Known(Option<time::Instant>),
/// Start time will be sent when the request body finishes streaming
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Start time will be sent when the request body finishes streaming
/// Start time will be sent when the request body finishes streaming.

Pending(oneshot::Receiver<time::Instant>),
}

impl StartTime {
fn recv(&mut self) -> Option<time::Instant> {
match self {
Self::Known(t) => t.take(),
Self::Pending(rx) => rx.try_recv().ok(),
}
}
}

type DurationFamily<L> = Family<L, Histogram, MkDurationHistogram>;
Expand Down Expand Up @@ -269,7 +289,7 @@ fn end_stream<L>(

labeler.end_response(res);

let elapsed = if let Ok(start) = start.try_recv() {
let elapsed = if let Some(start) = start.recv() {
time::Instant::now().saturating_duration_since(start)
} else {
time::Duration::ZERO
Expand Down
5 changes: 2 additions & 3 deletions linkerd/http/prom/src/record_response/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{
sync::Arc,
task::{Context, Poll},
};
use tokio::{sync::oneshot, time};
use tokio::time;

/// Metrics type that tracks completed requests.
#[derive(Debug)]
Expand Down Expand Up @@ -81,8 +81,7 @@ where

fn call(&mut self, req: http::Request<ReqB>) -> Self::Future {
let state = self.labeler.mk_stream_labeler(&req).map(|labeler| {
let (tx, start) = oneshot::channel();
tx.send(time::Instant::now()).unwrap();
let start = super::StartTime::Known(Some(time::Instant::now()));
Copy link
Member

Choose a reason for hiding this comment

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

🏃💨

this is an astute observation. thank you for opening this, @unleashed!

let RequestMetrics { duration } = self.metric.clone();
super::ResponseState {
labeler,
Expand Down
2 changes: 1 addition & 1 deletion linkerd/http/prom/src/record_response/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ where
let ResponseMetrics { duration } = self.metric.clone();
Some(super::ResponseState {
labeler,
start,
start: super::StartTime::Pending(start),
duration,
})
} else {
Expand Down
Loading