Skip to content

Commit d5e1c62

Browse files
authored
fix(promslog): always print time.Duration values as go duration strings (#798)
* fix(promslog): always print time.Duration values as go duration strings Addresses: prometheus/prometheus#16766 As brought up in the linked issue, it seems that upstream slog handles time.Duration values in a sub-optimal way in the JSON handler. To improve handling of duration values in a consistent way across the ecosystem, we should ensure that duration values are handled the same way across all supported log formats (json and logfmt). This change ensures that any key containing a value that is a time.Duration is reformatted as a boring string value, explicitly formatted as a Go duration string (ie, "1d2h3m") by calling the `String()` method on the duration. Example test output pre/post patch: Before: ``` ~/go/src/github.com/prometheus/common (main [ U ]) -> go test -v -race ./promslog -run TestDurationValues === RUN TestDurationValues time=2025-06-30T22:11:01.682-04:00 level=INFO source=slog_test.go:108 msg="duration testing" duration_raw=1m30s duration_string=1m30s {"time":"2025-06-30T22:11:01.683153372-04:00","level":"INFO","source":"slog_test.go:123","msg":"duration testing","duration_raw":90000000000,"duration_string":"1m30s"} slog_test.go:128: Error Trace: /home/tjhop/go/src/github.com/prometheus/common/promslog/slog_test.go:128 Error: "{\"time\":\"2025-06-30T22:11:01.683153372-04:00\",\"level\":\"INFO\",\"source\":\"slog_test.go:123\",\"msg\":\"duration testing\",\"duration_raw\":90000000000,\"duration_string\":\"1m30s\"}\n" should not contain "\"duration_raw\":90000000000" Test: TestDurationValues Messages: Expected duration to be output as Go duration string "1m30s", got "90000000000" --- FAIL: TestDurationValues (0.00s) FAIL FAIL github.com/prometheus/common/promslog 0.018s FAIL ``` After: ``` ~/go/src/github.com/prometheus/common (main [ U ]) -> go test -v -race ./promslog -run TestDurationValues === RUN TestDurationValues time=2025-06-30T22:13:03.714-04:00 level=INFO source=slog_test.go:108 msg="duration testing" duration_raw=1m30s duration_string=1m30s {"time":"2025-06-30T22:13:03.714880745-04:00","level":"INFO","source":"slog_test.go:123","msg":"duration testing","duration_raw":"1m30s","duration_string":"1m30s"} --- PASS: TestDurationValues (0.00s) PASS ok github.com/prometheus/common/promslog 1.014s ``` Signed-off-by: TJ Hoplock <[email protected]> * test(promslog): improve duration tests, convert to table driven suite Addresses PR feedback Signed-off-by: TJ Hoplock <[email protected]> * test(promslog): refactor duration tests Signed-off-by: TJ Hoplock <[email protected]> * test(promslog): more specific string check for duration testing Signed-off-by: TJ Hoplock <[email protected]> --------- Signed-off-by: TJ Hoplock <[email protected]>
1 parent e231fec commit d5e1c62

File tree

2 files changed

+42
-0
lines changed

2 files changed

+42
-0
lines changed

promslog/slog.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,13 @@ func newGoKitStyleReplaceAttrFunc(lvl *Level) func(groups []string, a slog.Attr)
197197
}
198198
default:
199199
}
200+
201+
// Ensure time.Duration values are _always_ formatted as a Go
202+
// duration string (ie, "1d2h3m").
203+
if v, ok := a.Value.Any().(time.Duration); ok {
204+
a.Value = slog.StringValue(v.String())
205+
}
206+
200207
return a
201208
}
202209
}
@@ -238,6 +245,13 @@ func defaultReplaceAttr(_ []string, a slog.Attr) slog.Attr {
238245
}
239246
default:
240247
}
248+
249+
// Ensure time.Duration values are _always_ formatted as a Go duration
250+
// string (ie, "1d2h3m").
251+
if v, ok := a.Value.Any().(time.Duration); ok {
252+
a.Value = slog.StringValue(v.String())
253+
}
254+
241255
return a
242256
}
243257

promslog/slog_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"regexp"
2222
"strings"
2323
"testing"
24+
"time"
2425

2526
"github.com/stretchr/testify/require"
2627
"gopkg.in/yaml.v2"
@@ -94,6 +95,33 @@ func getLogEntryLevelCounts(s string, re *regexp.Regexp) map[string]int {
9495
return counters
9596
}
9697

98+
func TestDurationValues(t *testing.T) {
99+
dur, err := time.ParseDuration("1m30s")
100+
require.NoError(t, err)
101+
102+
tests := map[string]struct {
103+
logFormat string
104+
want string
105+
}{
106+
"logfmt_duration_testing": {want: "duration_raw=1m30s duration_string=1m30s", logFormat: "logfmt"},
107+
"json_duration_testing": {want: "\"duration_raw\":\"1m30s\",\"duration_string\":\"1m30s\"", logFormat: "json"},
108+
}
109+
110+
for name, tc := range tests {
111+
t.Run(name, func(t *testing.T) {
112+
var buf bytes.Buffer
113+
config := &Config{
114+
Writer: &buf,
115+
Format: NewFormat(),
116+
}
117+
require.NoError(t, config.Format.Set(tc.logFormat))
118+
logger := New(config)
119+
logger.Info("duration testing", "duration_raw", dur, "duration_string", dur.String())
120+
require.Contains(t, buf.String(), tc.want)
121+
})
122+
}
123+
}
124+
97125
func TestDynamicLevels(t *testing.T) {
98126
var buf bytes.Buffer
99127
wantedLevelCounts := map[string]int{"info": 1, "debug": 1}

0 commit comments

Comments
 (0)