From c87870ad19c25b0dfba30497f275aa8be67fd316 Mon Sep 17 00:00:00 2001 From: Brandon Johnson Date: Tue, 12 Oct 2021 13:33:48 -0400 Subject: [PATCH 1/3] Wrap error from time.LoadLocation to include reason --- operator/builtin/parser/syslog/syslog.go | 2 +- operator/builtin/parser/syslog/syslog_test.go | 11 ++++++++++ operator/helper/time.go | 4 ++-- operator/helper/time_test.go | 21 +++++++++++++++++++ 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/operator/builtin/parser/syslog/syslog.go b/operator/builtin/parser/syslog/syslog.go index f0d234d9..464517eb 100644 --- a/operator/builtin/parser/syslog/syslog.go +++ b/operator/builtin/parser/syslog/syslog.go @@ -75,7 +75,7 @@ func (c SyslogParserConfig) Build(context operator.BuildContext) ([]operator.Ope location, err := time.LoadLocation(c.Location) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to load location %s (is the time zone database installed?): %w", c.Location, err) } syslogParser := &SyslogParser{ diff --git a/operator/builtin/parser/syslog/syslog_test.go b/operator/builtin/parser/syslog/syslog_test.go index 94265986..0e5e96f2 100644 --- a/operator/builtin/parser/syslog/syslog_test.go +++ b/operator/builtin/parser/syslog/syslog_test.go @@ -20,6 +20,7 @@ import ( "time" "github.com/stretchr/testify/require" + "go.uber.org/zap" "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-log-collection/entry" @@ -101,3 +102,13 @@ parse_to: $.to` require.Equal(t, expect, &actual) }) } + +func TestSyslogParserInvalidLocation(t *testing.T) { + config := NewSyslogParserConfig("test") + config.Location = "not_a_location" + config.Protocol = RFC3164 + + _, err := config.Build(operator.NewBuildContext(zap.NewNop().Sugar())) + require.Error(t, err) + require.Contains(t, err.Error(), "failed to load location "+config.Location) +} diff --git a/operator/helper/time.go b/operator/helper/time.go index 6d333cad..c3d9ba7f 100644 --- a/operator/helper/time.go +++ b/operator/helper/time.go @@ -115,7 +115,7 @@ func (t *TimeParser) setLocation() error { // If "location" is specified, it must be in the local timezone database loc, err := time.LoadLocation(t.Location) if err != nil { - return err + return fmt.Errorf("failed to load location %s (is the time zone database installed?): %w", t.Location, err) } t.location = loc return nil @@ -198,7 +198,7 @@ func (t *TimeParser) parseGotime(value interface{}) (time.Time, error) { loc, locErr := time.LoadLocation(zone) if locErr != nil { // can't correct offset, just return what we have - return result, err + return result, fmt.Errorf("failed to load location %s (is the time zone database installed?): %w", zone, locErr) } // Reparse the timestamp, with the location diff --git a/operator/helper/time_test.go b/operator/helper/time_test.go index 451dad11..72bd2df0 100644 --- a/operator/helper/time_test.go +++ b/operator/helper/time_test.go @@ -736,3 +736,24 @@ func defaultTimeCfg() *TimeParser { newCfg := NewTimeParser() return &newCfg } + +func TestSetInvalidLocation(t *testing.T) { + tp := NewTimeParser() + tp.Location = "not_a_location" + err := tp.setLocation() + require.Error(t, err) + require.Contains(t, err.Error(), "failed to load location "+"not_a_location") +} + +func TestParseGoTimeBadLocation(t *testing.T) { + tp := NewTimeParser() + tp.Location = "America/New_York" + + err := tp.setLocation() + require.NoError(t, err) + + tp.Layout = time.RFC822 + _, err = tp.parseGotime("02 Jan 06 15:04 BST") + require.Error(t, err) + require.Contains(t, err.Error(), "failed to load location BST") +} From 25af759b3e1a8d2bf5633257e09ccc32313155c0 Mon Sep 17 00:00:00 2001 From: Brandon Johnson Date: Tue, 12 Oct 2021 13:41:51 -0400 Subject: [PATCH 2/3] time zone -> timezone --- operator/builtin/parser/syslog/syslog.go | 2 +- operator/helper/time.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/operator/builtin/parser/syslog/syslog.go b/operator/builtin/parser/syslog/syslog.go index 464517eb..bd6c7b2e 100644 --- a/operator/builtin/parser/syslog/syslog.go +++ b/operator/builtin/parser/syslog/syslog.go @@ -75,7 +75,7 @@ func (c SyslogParserConfig) Build(context operator.BuildContext) ([]operator.Ope location, err := time.LoadLocation(c.Location) if err != nil { - return nil, fmt.Errorf("failed to load location %s (is the time zone database installed?): %w", c.Location, err) + return nil, fmt.Errorf("failed to load location %s (is the timezone database installed?): %w", c.Location, err) } syslogParser := &SyslogParser{ diff --git a/operator/helper/time.go b/operator/helper/time.go index c3d9ba7f..746cbee8 100644 --- a/operator/helper/time.go +++ b/operator/helper/time.go @@ -115,7 +115,7 @@ func (t *TimeParser) setLocation() error { // If "location" is specified, it must be in the local timezone database loc, err := time.LoadLocation(t.Location) if err != nil { - return fmt.Errorf("failed to load location %s (is the time zone database installed?): %w", t.Location, err) + return fmt.Errorf("failed to load location %s (is the timezone database installed?): %w", t.Location, err) } t.location = loc return nil @@ -198,7 +198,7 @@ func (t *TimeParser) parseGotime(value interface{}) (time.Time, error) { loc, locErr := time.LoadLocation(zone) if locErr != nil { // can't correct offset, just return what we have - return result, fmt.Errorf("failed to load location %s (is the time zone database installed?): %w", zone, locErr) + return result, fmt.Errorf("failed to load location %s (is the timezone database installed?): %w", zone, locErr) } // Reparse the timestamp, with the location From 80c143721bf5ad5cc59b48964105b6f21350c6fa Mon Sep 17 00:00:00 2001 From: Brandon Johnson Date: Tue, 12 Oct 2021 14:19:36 -0400 Subject: [PATCH 3/3] Remove timezone database suggestion --- operator/builtin/parser/syslog/syslog.go | 2 +- operator/helper/time.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/operator/builtin/parser/syslog/syslog.go b/operator/builtin/parser/syslog/syslog.go index bd6c7b2e..add04f31 100644 --- a/operator/builtin/parser/syslog/syslog.go +++ b/operator/builtin/parser/syslog/syslog.go @@ -75,7 +75,7 @@ func (c SyslogParserConfig) Build(context operator.BuildContext) ([]operator.Ope location, err := time.LoadLocation(c.Location) if err != nil { - return nil, fmt.Errorf("failed to load location %s (is the timezone database installed?): %w", c.Location, err) + return nil, fmt.Errorf("failed to load location %s: %w", c.Location, err) } syslogParser := &SyslogParser{ diff --git a/operator/helper/time.go b/operator/helper/time.go index 746cbee8..71adff24 100644 --- a/operator/helper/time.go +++ b/operator/helper/time.go @@ -115,7 +115,7 @@ func (t *TimeParser) setLocation() error { // If "location" is specified, it must be in the local timezone database loc, err := time.LoadLocation(t.Location) if err != nil { - return fmt.Errorf("failed to load location %s (is the timezone database installed?): %w", t.Location, err) + return fmt.Errorf("failed to load location %s: %w", t.Location, err) } t.location = loc return nil @@ -198,7 +198,7 @@ func (t *TimeParser) parseGotime(value interface{}) (time.Time, error) { loc, locErr := time.LoadLocation(zone) if locErr != nil { // can't correct offset, just return what we have - return result, fmt.Errorf("failed to load location %s (is the timezone database installed?): %w", zone, locErr) + return result, fmt.Errorf("failed to load location %s: %w", zone, locErr) } // Reparse the timestamp, with the location