fix(internal): Fix time parsing for abbreviated timezones (#13363)

CHANGELOG.md

@@ -3,9 +3,19 @@
 
 ## next
 
-### BREAKING API CHANGES
+### **BREAKING CHANGES**
 
-- Removal of old-style parser creation
+- 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]
 

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
 	}
+
+	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
 }

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

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)
 	}

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)
 	}

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)
 	}

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 {

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)

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)
 

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),
 				),
 			},
 		},