Skip to content

add latency to log messages#737

Merged
davepacheco merged 2 commits into
mainfrom
dap/request-latency
Aug 4, 2023
Merged

add latency to log messages#737
davepacheco merged 2 commits into
mainfrom
dap/request-latency

Conversation

@davepacheco

Copy link
Copy Markdown
Collaborator

Fixes #692. Okay, I figured it'd be easy, but this was even easier than I expected. Sorry it took so long.

@davepacheco davepacheco requested a review from luqmana August 3, 2023 00:10

@luqmana luqmana left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

one nit aside, lgtm!

Comment thread dropshot/src/server.rs Outdated
// TODO-debug: add request and response headers here
info!(request_log, "request completed";
"response_code" => response.status().as_str().to_string()
"response_code" => response.status().as_str().to_string(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was pre-existing, but is there any reason the extra to_string() allocation is needed here (and above)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think it's probably old code from before I really grokked when slog needs an owned value vs. when it doesn't. I can include this improvement into this PR too.

@davepacheco davepacheco enabled auto-merge (squash) August 4, 2023 19:20
@davepacheco davepacheco merged commit b3c4d0b into main Aug 4, 2023
@davepacheco davepacheco deleted the dap/request-latency branch August 4, 2023 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"request completed" log message should include response latency

3 participants