Expand the usage of friendly durations#4887
Conversation
AhmedSoliman
force-pushed
the
pr4887
branch
2 times, most recently
from
ae9323e to
e31f103
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e31f103479
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .round( | ||
| SpanRound::new() | ||
| .largest(jiff::Unit::Second) | ||
| .largest(jiff::Unit::Microsecond) |
There was a problem hiding this comment.
This second call to largest() overwrites the earlier largest(Unit::Second) setting rather than setting the minimum precision, so to_seconds_span() rebalances durations into microseconds instead of seconds. The repo uses latency.friendly().to_seconds_span() for HTTP tracing in crates/admin/src/service.rs and crates/ingress-http/src/server.rs, so latencies over one second will be emitted as large microsecond values instead of the intended seconds-based span; use smallest(Unit::Microsecond) here if the goal is only to suppress nanoseconds.
Useful? React with 👍 / 👎.
| .round( | ||
| SpanRound::new() | ||
| .largest(jiff::Unit::Day) | ||
| .smallest(jiff::Unit::Millisecond) |
There was a problem hiding this comment.
Rounding the default days span to milliseconds also affects the human-readable serde paths (SerializeAs and Serialize both collect source.friendly().to_days_span() in this file), not just log messages. Any configured nonzero duration below 500µs now serializes as a zero millisecond span, so a human-readable round-trip can lose the value or deserialize to zero for a NonZeroFriendlyDuration; keep the lossy millisecond rounding out of the default/serde display path or make it opt-in for logs only.
Useful? React with 👍 / 👎.
Also limits the minimum units depending on the timespan used. This reduces verbosity as in pretty much all cases we don't care about nanos or micros in log messages.
1 participant
Also limits the minimum units depending on the timespan used. This reduces verbosity as in pretty much all cases we don't care about nanos or micros in log messages.