[otelcol] Tell all loggers to encode timestamps in ISO8601. #10543
Conversation
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10543 +/- ##
=======================================
Coverage 91.69% 91.69%
=======================================
Files 411 411
Lines 19294 19296 +2
=======================================
+ Hits 17692 17694 +2
Misses 1241 1241
Partials 361 361 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
LGTM. Please add a changelog entry. IMO this is best documented as a breaking change since it may affect users who are consuming the collector's logs.
| ec := zap.NewProductionEncoderConfig() | ||
| ec.EncodeTime = zapcore.ISO8601TimeEncoder | ||
| zapCfg := &zap.Config{ | ||
| Level: zap.NewAtomicLevelAt(cfg.Level), | ||
| Development: cfg.Development, | ||
| Encoding: cfg.Encoding, | ||
| EncoderConfig: zap.NewProductionEncoderConfig(), | ||
| EncoderConfig: ec, |
There was a problem hiding this comment.
I believe only the change in collector_windows.go is necessary. Are you sure this needs changed also?
There was a problem hiding this comment.
Wouldn't we have the same issue with the collector's stderr on Linux without this change?
There was a problem hiding this comment.
I'll check again next week, but I believe it is getting set below. I've only ever seen the collector write out ISO timestamps, so I don't believe Linux is affected.
There was a problem hiding this comment.
@TylerHelmuth Any progress on checking this? Thanks.
There was a problem hiding this comment.
@igorpeshansky a couple lines below there is:
if zapCfg.Encoding == "console" {
// Human-readable timestamps for console format of logs.
zapCfg.EncoderConfig.EncodeTime = zapcore.ISO8601TimeEncoder
}My guess is that for linux this condition is getting met which is why the timestamps when the collector is run in linux are ISO8601. I think if we set the value here, we can remove that conditional.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Sorry, still planning to work on this. |
Description
OTel collector log messages in Windows are currently displayed in scientific notation, e.g.:
This fix will cause the log messages to use ISO8601 timestamps instead.
Testing
Manual testing.
Documentation
The previous behavior was undocumented, so no documentation changes needed.