diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a6cf54a4a97b..9a012eff7150d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,9 +3,19 @@ ## next -### BREAKING API CHANGES - -- Removal of old-style parser creation +### **BREAKING CHANGES** + +- Fix parsing of timezone abbreviations such as `MST`. Up to now, when parsing + times with abbreviated timezones (i.e. the format ) the timezone information + is ignored completely and the _timestamp_ is located in UTC. This is a golang + issue (see [#9617](https://github.com/golang/go/issues/9617) or + [#56528](https://github.com/golang/go/issues/56528)). If you worked around + that issue, please remove the workaround before using v1.27+. In case you + experience issues with abbreviated timezones please file an issue! +- Removal of old-style parser creation. This should not directly affect users as + it is an API change. All parsers in Telegraf are already ported to the new + framework. If you experience any issues with not being able to create parsers + let us know! ## v1.26.3 [2023-05-22] diff --git a/internal/internal.go b/internal/internal.go index 11e13d1c25f06..a424d07641457 100644 --- a/internal/internal.go +++ b/internal/internal.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "log" "math/big" "math/rand" "os" @@ -24,6 +25,8 @@ import ( const alphanum string = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" +var once sync.Once + var ( ErrTimeout = errors.New("command timed out") ErrNotImplemented = errors.New("not implemented yet") @@ -391,5 +394,40 @@ func parseTime(format string, timestamp string, location *time.Location) (time.T case "stampnano": format = time.StampNano } - return time.ParseInLocation(format, timestamp, loc) + + if !strings.Contains(format, "MST") { + return time.ParseInLocation(format, timestamp, loc) + } + + // Golang does not parse times with ambiguous timezone abbreviations, + // but only parses the time-fields and the timezone NAME with a zero + // offset (see https://groups.google.com/g/golang-nuts/c/hDMdnm_jUFQ/m/yeL9IHOsAQAJ). + // To handle those timezones correctly we can use the timezone-name and + // force parsing the time in that timezone. This way we get the correct + // time for the "most probably" of the ambiguous timezone-abbreviations. + ts, err := time.Parse(format, timestamp) + if err != nil { + return time.Time{}, err + } + zone, offset := ts.Zone() + if zone == "UTC" || offset != 0 { + return ts.In(loc), nil + } + once.Do(func() { + const msg = `Your config is using abbreviated timezones and parsing was changed in v1.27.0! + Please see the change log, remove any workarounds in place, and carefully + check your data timestamps! If case you experience any problems, please + file an issue on https://github.com/influxdata/telegraf/issues!` + log.Print("W! " + msg) + }) + + abbrevLoc, err := time.LoadLocation(zone) + if err != nil { + return time.Time{}, fmt.Errorf("cannot resolve timezone abbreviation %q: %w", zone, err) + } + ts, err = time.ParseInLocation(format, timestamp, abbrevLoc) + if err != nil { + return time.Time{}, err + } + return ts.In(loc), nil } diff --git a/internal/internal_test.go b/internal/internal_test.go index 4fe52e97a3867..f3b73f852e3a8 100644 --- a/internal/internal_test.go +++ b/internal/internal_test.go @@ -8,6 +8,7 @@ import ( "log" "os/exec" "regexp" + "sync" "testing" "time" @@ -349,42 +350,18 @@ func TestParseTimestamp(t *testing.T) { return tm } - unixdate := func(value string) time.Time { - tm, err := time.Parse(time.UnixDate, value) - require.NoError(t, err) - return tm - } - rubydate := func(value string) time.Time { tm, err := time.Parse(time.RubyDate, value) require.NoError(t, err) return tm } - rfc822 := func(value string) time.Time { - tm, err := time.Parse(time.RFC822, value) - require.NoError(t, err) - return tm - } - rfc822z := func(value string) time.Time { tm, err := time.Parse(time.RFC822Z, value) require.NoError(t, err) return tm } - rfc850 := func(value string) time.Time { - tm, err := time.Parse(time.RFC850, value) - require.NoError(t, err) - return tm - } - - rfc1123 := func(value string) time.Time { - tm, err := time.Parse(time.RFC1123, value) - require.NoError(t, err) - return tm - } - rfc1123z := func(value string) time.Time { tm, err := time.Parse(time.RFC1123Z, value) require.NoError(t, err) @@ -580,7 +557,7 @@ func TestParseTimestamp(t *testing.T) { name: "UnixDate", format: "UnixDate", timestamp: "Mon Jan 2 15:04:05 MST 2006", - expected: unixdate("Mon Jan 2 15:04:05 MST 2006"), + expected: time.Unix(1136239445, 0), location: "Local", }, @@ -596,7 +573,7 @@ func TestParseTimestamp(t *testing.T) { name: "RFC822", format: "RFC822", timestamp: "02 Jan 06 15:04 MST", - expected: rfc822("02 Jan 06 15:04 MST"), + expected: time.Unix(1136239440, 0), location: "Local", }, @@ -612,7 +589,7 @@ func TestParseTimestamp(t *testing.T) { name: "RFC850", format: "RFC850", timestamp: "Monday, 02-Jan-06 15:04:05 MST", - expected: rfc850("Monday, 02-Jan-06 15:04:05 MST"), + expected: time.Unix(1136239445, 0), location: "Local", }, @@ -620,7 +597,7 @@ func TestParseTimestamp(t *testing.T) { name: "RFC1123", format: "RFC1123", timestamp: "Mon, 02 Jan 2006 15:04:05 MST", - expected: rfc1123("Mon, 02 Jan 2006 15:04:05 MST"), + expected: time.Unix(1136239445, 0), location: "Local", }, @@ -667,6 +644,13 @@ func TestParseTimestamp(t *testing.T) { timestamp: "Jan 2 15:04:05.000000000", expected: stampnano("Jan 2 15:04:05.000000000"), }, + + { + name: "RFC850", + format: "RFC850", + timestamp: "Monday, 02-Jan-06 15:04:05 MST", + expected: time.Unix(1136239445, 0), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -678,7 +662,7 @@ func TestParseTimestamp(t *testing.T) { } tm, err := ParseTimestamp(tt.format, tt.timestamp, loc, tt.separator...) require.NoError(t, err) - require.Equal(t, tt.expected, tm) + require.Equal(t, tt.expected.Unix(), tm.Unix()) }) } } @@ -732,6 +716,12 @@ func TestParseTimestampInvalid(t *testing.T) { timestamp: "1,568,338,208.500", expected: "invalid number", }, + { + name: "invalid timezone abbreviation", + format: "RFC850", + timestamp: "Monday, 02-Jan-06 15:04:05 CDT", + expected: "cannot resolve timezone abbreviation", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -741,6 +731,20 @@ func TestParseTimestampInvalid(t *testing.T) { } } +func TestTimestampAbbrevWarning(t *testing.T) { + var buf bytes.Buffer + backup := log.Writer() + log.SetOutput(&buf) + defer log.SetOutput(backup) + + once = sync.Once{} + ts, err := ParseTimestamp("RFC1123", "Mon, 02 Jan 2006 15:04:05 MST", nil) + require.NoError(t, err) + require.EqualValues(t, 1136239445, ts.Unix()) + + require.Contains(t, buf.String(), "Your config is using abbreviated timezones and parsing was changed in v1.27.0") +} + func TestProductToken(t *testing.T) { token := ProductToken() // Telegraf version depends on the call to SetVersion, it cannot be set diff --git a/plugins/inputs/consul_agent/consul_agent.go b/plugins/inputs/consul_agent/consul_agent.go index 0ab05c81c67fb..b0c20f45a12f7 100644 --- a/plugins/inputs/consul_agent/consul_agent.go +++ b/plugins/inputs/consul_agent/consul_agent.go @@ -13,6 +13,7 @@ import ( "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/config" + "github.com/influxdata/telegraf/internal" "github.com/influxdata/telegraf/plugins/common/tls" "github.com/influxdata/telegraf/plugins/inputs" ) @@ -119,7 +120,7 @@ func (n *ConsulAgent) loadJSON(url string) (*AgentInfo, error) { // buildConsulAgent, it builds all the metrics and adds them to the accumulator) func buildConsulAgent(acc telegraf.Accumulator, agentInfo *AgentInfo) error { - t, err := time.Parse(timeLayout, agentInfo.Timestamp) + t, err := internal.ParseTimestamp(timeLayout, agentInfo.Timestamp, nil) if err != nil { return fmt.Errorf("error parsing time: %w", err) } diff --git a/plugins/inputs/nomad/nomad.go b/plugins/inputs/nomad/nomad.go index 66be8f3823c86..8d37f59f11bda 100644 --- a/plugins/inputs/nomad/nomad.go +++ b/plugins/inputs/nomad/nomad.go @@ -10,6 +10,7 @@ import ( "github.com/influxdata/telegraf" "github.com/influxdata/telegraf/config" + "github.com/influxdata/telegraf/internal" "github.com/influxdata/telegraf/plugins/common/tls" "github.com/influxdata/telegraf/plugins/inputs" ) @@ -103,7 +104,7 @@ func (n *Nomad) loadJSON(url string, v interface{}) error { // buildNomadMetrics, it builds all the metrics and adds them to the accumulator) func buildNomadMetrics(acc telegraf.Accumulator, summaryMetrics *MetricsSummary) error { - t, err := time.Parse(timeLayout, summaryMetrics.Timestamp) + t, err := internal.ParseTimestamp(timeLayout, summaryMetrics.Timestamp, nil) if err != nil { return fmt.Errorf("error parsing time: %w", err) } diff --git a/plugins/inputs/vault/vault.go b/plugins/inputs/vault/vault.go index 079a1894851e2..c73dece4092d1 100644 --- a/plugins/inputs/vault/vault.go +++ b/plugins/inputs/vault/vault.go @@ -123,7 +123,7 @@ func (n *Vault) loadJSON(url string) (*SysMetrics, error) { // buildVaultMetrics, it builds all the metrics and adds them to the accumulator func buildVaultMetrics(acc telegraf.Accumulator, sysMetrics *SysMetrics) error { - t, err := time.Parse(timeLayout, sysMetrics.Timestamp) + t, err := internal.ParseTimestamp(timeLayout, sysMetrics.Timestamp, nil) if err != nil { return fmt.Errorf("error parsing time: %w", err) } diff --git a/plugins/parsers/binary/entry.go b/plugins/parsers/binary/entry.go index ebdac2124f9ca..890cbaa3ec6c5 100644 --- a/plugins/parsers/binary/entry.go +++ b/plugins/parsers/binary/entry.go @@ -8,6 +8,8 @@ import ( "math" "strings" "time" + + "github.com/influxdata/telegraf/internal" ) type Entry struct { @@ -244,7 +246,7 @@ func (e *Entry) convertTimeType(in []byte, order binary.ByteOrder) (time.Time, e } // We have a format specification (hopefully) v := convertStringType(in) - return time.ParseInLocation(e.Type, v, e.location) + return internal.ParseTimestamp(e.Type, v, e.location) } func convertStringType(in []byte) string { diff --git a/plugins/parsers/grok/parser.go b/plugins/parsers/grok/parser.go index 3602bc2f79bf3..6087155519f01 100644 --- a/plugins/parsers/grok/parser.go +++ b/plugins/parsers/grok/parser.go @@ -13,6 +13,7 @@ import ( "github.com/vjeantet/grok" "github.com/influxdata/telegraf" + "github.com/influxdata/telegraf/internal" "github.com/influxdata/telegraf/metric" "github.com/influxdata/telegraf/plugins/parsers" ) @@ -317,7 +318,7 @@ func (p *Parser) ParseLine(line string) (telegraf.Metric, error) { timestamp = time.Unix(0, iv) } case SyslogTimestamp: - ts, err := time.ParseInLocation(time.Stamp, v, p.loc) + ts, err := internal.ParseTimestamp(time.Stamp, v, p.loc) if err == nil { if ts.Year() == 0 { ts = ts.AddDate(timestamp.Year(), 0, 0) @@ -330,7 +331,7 @@ func (p *Parser) ParseLine(line string) (telegraf.Metric, error) { var foundTs bool // first try timestamp layouts that we've already found for _, layout := range p.foundTsLayouts { - ts, err := time.ParseInLocation(layout, v, p.loc) + ts, err := internal.ParseTimestamp(layout, v, p.loc) if err == nil { timestamp = ts foundTs = true @@ -341,7 +342,7 @@ func (p *Parser) ParseLine(line string) (telegraf.Metric, error) { // layouts. if !foundTs { for _, layout := range timeLayouts { - ts, err := time.ParseInLocation(layout, v, p.loc) + ts, err := internal.ParseTimestamp(layout, v, p.loc) if err == nil { timestamp = ts foundTs = true @@ -360,7 +361,7 @@ func (p *Parser) ParseLine(line string) (telegraf.Metric, error) { // goodbye! default: v = strings.ReplaceAll(v, ",", ".") - ts, err := time.ParseInLocation(t, v, p.loc) + ts, err := internal.ParseTimestamp(t, v, p.loc) if err == nil { if ts.Year() == 0 { ts = ts.AddDate(timestamp.Year(), 0, 0) diff --git a/plugins/parsers/grok/parser_test.go b/plugins/parsers/grok/parser_test.go index 607751fc0350b..06bdf2bcc18cd 100644 --- a/plugins/parsers/grok/parser_test.go +++ b/plugins/parsers/grok/parser_test.go @@ -792,10 +792,11 @@ func TestShortPatternRegression(t *testing.T) { CustomPatterns: ` TS_UNIX %{DAY} %{MONTH} %{MONTHDAY} %{HOUR}:%{MINUTE}:%{SECOND} %{TZ} %{YEAR} `, + Log: testutil.Logger{}, } require.NoError(t, p.Compile()) - m, err := p.ParseLine(`Wed Apr 12 13:10:34 PST 2017 42`) + m, err := p.ParseLine(`Wed Apr 12 13:10:34 MST 2017 42`) require.NoError(t, err) require.NotNil(t, m) diff --git a/plugins/processors/converter/converter_test.go b/plugins/processors/converter/converter_test.go index d56fa112869ca..09628a2c899cd 100644 --- a/plugins/processors/converter/converter_test.go +++ b/plugins/processors/converter/converter_test.go @@ -604,7 +604,7 @@ func TestConverter(t *testing.T) { map[string]interface{}{ "a": 42.0, }, - time.Unix(1456799999, 0), + time.Unix(1456825199, 0), ), }, },