Log sleep durations more definitively #699
Merged
+36
−14
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While looking into #695 the other day, I was reminded that the handling
of
time.Duration
in things like logging is potentially not very good.time.Duration
has a goodString()
override that tends to get usedwith text handlers, but for various legacy reasons it doesn't have a
MarshalJSON
implementation, so when dumped to JSON it gets put innanoseconds, which isn't very readable for human or computer.
This is relevant to use because slog's
JSONHandler
andTextHandler
will probably be the most common logging instruments for River. e.g.
Here, change the various places that we log sleep to use a more
definitive value for purposes of the log. In this case a duration string
representation like
10s
.I debated making this a float instead that'd be represented in seconds
because the float would be more parseable for ingestion engines like
Splunk. I went with the former option in the end though because it's
probably better for humans and most other cases, and we could always add
an alternative
sleep_seconds
field that's a float later if someoneasks for it.