From ad7d968079c912cc55bdf7ee0fa71320cb4f7207 Mon Sep 17 00:00:00 2001 From: Ante Kresic Date: Fri, 25 Sep 2020 15:19:51 +0200 Subject: [PATCH] Add log format flag and default to logfmt formatting for logs This commit adds a new log format flag which can be used for setting the format of the logs to either logfmt (default) or json. It also cleans up logger flags and initalization logic a bit. --- cmd/promscale/main.go | 4 +-- pkg/api/common_test.go | 4 ++- pkg/api/health_test.go | 4 ++- pkg/api/labels_test.go | 4 ++- pkg/api/query_range_test.go | 4 ++- pkg/api/query_test.go | 4 ++- pkg/api/series_test.go | 4 ++- pkg/api/write_test.go | 4 ++- pkg/log/log.go | 34 +++++++++++++++++++---- pkg/pgmodel/end_to_end_tests/main_test.go | 4 ++- pkg/pgmodel/upgrade_tests/upgrade_test.go | 4 ++- pkg/runner/runner.go | 2 ++ pkg/runner/runner_test.go | 4 ++- pkg/util/election_test.go | 4 ++- pkg/util/util_test.go | 4 ++- 15 files changed, 68 insertions(+), 20 deletions(-) diff --git a/cmd/promscale/main.go b/cmd/promscale/main.go index 30ab01841b..67cbd290ec 100644 --- a/cmd/promscale/main.go +++ b/cmd/promscale/main.go @@ -4,7 +4,6 @@ package main import ( - "flag" "fmt" "os" @@ -14,7 +13,6 @@ import ( ) func main() { - logLevel := flag.String("log-level", "debug", "The log level to use [ \"error\", \"warn\", \"info\", \"debug\" ].") cfg := &runner.Config{} cfg, err := runner.ParseFlags(cfg) if err != nil { @@ -22,7 +20,7 @@ func main() { fmt.Println("Fatal error: cannot parse flags ", err) os.Exit(1) } - err = log.Init(*logLevel) + err = log.Init(cfg.LogCfg) if err != nil { fmt.Println("Version: ", version.Version, "Commit Hash: ", version.CommitHash) fmt.Println("Fatal error: cannot start logger", err) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 3149064a1a..8481f6d939 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -12,7 +12,9 @@ import ( ) func TestCORSWrapper(t *testing.T) { - _ = log.Init("debug") + _ = log.Init(log.Config{ + Level: "debug", + }) acceptSpecific, _ := regexp.Compile("^(?:" + "http://some-site.com" + ")$") acceptAny, _ := regexp.Compile("^(?:" + ".*" + ")$") diff --git a/pkg/api/health_test.go b/pkg/api/health_test.go index 60b4a7bf3e..e0bd7108df 100644 --- a/pkg/api/health_test.go +++ b/pkg/api/health_test.go @@ -27,7 +27,9 @@ func (m *mockHealthChecker) HealthCheck() error { } func TestHealth(t *testing.T) { - _ = log.Init("debug") + _ = log.Init(log.Config{ + Level: "debug", + }) testCases := []struct { name string diff --git a/pkg/api/labels_test.go b/pkg/api/labels_test.go index 17389d0b35..140af25205 100644 --- a/pkg/api/labels_test.go +++ b/pkg/api/labels_test.go @@ -15,7 +15,9 @@ import ( ) func TestLabels(t *testing.T) { - _ = log.Init("debug") + _ = log.Init(log.Config{ + Level: "debug", + }) testCases := []struct { name string querier *mockQuerier diff --git a/pkg/api/query_range_test.go b/pkg/api/query_range_test.go index e3aa375fba..d0d1a54f02 100644 --- a/pkg/api/query_range_test.go +++ b/pkg/api/query_range_test.go @@ -18,7 +18,9 @@ import ( ) func TestRangedQuery(t *testing.T) { - _ = log.Init("debug") + _ = log.Init(log.Config{ + Level: "debug", + }) testCases := []struct { name string timeout string diff --git a/pkg/api/query_test.go b/pkg/api/query_test.go index 2419d0f700..6620d9973d 100644 --- a/pkg/api/query_test.go +++ b/pkg/api/query_test.go @@ -121,7 +121,9 @@ func TestParseDuration(t *testing.T) { } func TestQuery(t *testing.T) { - _ = log.Init("debug") + _ = log.Init(log.Config{ + Level: "debug", + }) testCases := []struct { name string timeout string diff --git a/pkg/api/series_test.go b/pkg/api/series_test.go index 6ba8824599..8e19acd641 100644 --- a/pkg/api/series_test.go +++ b/pkg/api/series_test.go @@ -15,7 +15,9 @@ import ( ) func TestSeries(t *testing.T) { - _ = log.Init("debug") + _ = log.Init(log.Config{ + Level: "debug", + }) testCases := []struct { name string diff --git a/pkg/api/write_test.go b/pkg/api/write_test.go index d296843c9c..c28205c5a9 100644 --- a/pkg/api/write_test.go +++ b/pkg/api/write_test.go @@ -24,7 +24,9 @@ import ( ) func TestWrite(t *testing.T) { - testutil.Ok(t, log.Init("debug")) + testutil.Ok(t, log.Init(log.Config{ + Level: "debug", + })) testCases := []struct { name string responseCode int diff --git a/pkg/log/log.go b/pkg/log/log.go index 653012d483..ea78983f82 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -6,6 +6,7 @@ package log import ( + "flag" "fmt" "os" "time" @@ -25,16 +26,39 @@ var ( ) ) -// Init starts logging given the minimum log level -func Init(logLevel string) error { - l := log.NewJSONLogger(log.NewSyncWriter(os.Stderr)) - logLevelOption, err := parseLogLevel(logLevel) +// Config represents a logger configuration used upon initialization. +type Config struct { + Level string + Format string +} + +// ParseFlags parses the configuration flags for logging. +func ParseFlags(cfg *Config) *Config { + flag.StringVar(&cfg.Level, "log-level", "debug", "The log level to use [ \"error\", \"warn\", \"info\", \"debug\" ].") + flag.StringVar(&cfg.Format, "log-format", "logfmt", "The log format to use [ \"logfmt\", \"json\" ].") + return cfg +} + +// Init starts logging given the configuration. By default, it uses logfmt format +// and minimum logging level. +func Init(cfg Config) error { + var l log.Logger + switch cfg.Format { + case "logfmt", "": + l = log.NewLogfmtLogger(log.NewSyncWriter(os.Stderr)) + case "json": + l = log.NewJSONLogger(log.NewSyncWriter(os.Stderr)) + default: + return fmt.Errorf("unrecognized log format %s", cfg.Format) + } + + logLevelOption, err := parseLogLevel(cfg.Level) if err != nil { return err } l = level.NewFilter(l, logLevelOption) - logger = log.With(l, "ts", timestampFormat, "caller", log.Caller(4)) + logger = log.With(l, "ts", timestampFormat, "caller", log.DefaultCaller) return nil } diff --git a/pkg/pgmodel/end_to_end_tests/main_test.go b/pkg/pgmodel/end_to_end_tests/main_test.go index 80d912c473..55d6efa883 100644 --- a/pkg/pgmodel/end_to_end_tests/main_test.go +++ b/pkg/pgmodel/end_to_end_tests/main_test.go @@ -41,7 +41,9 @@ func TestMain(m *testing.M) { func() { flag.Parse() ctx := context.Background() - _ = log.Init("debug") + _ = log.Init(log.Config{ + Level: "debug", + }) if !testing.Short() { var err error diff --git a/pkg/pgmodel/upgrade_tests/upgrade_test.go b/pkg/pgmodel/upgrade_tests/upgrade_test.go index a36357de23..2a52dc6d05 100644 --- a/pkg/pgmodel/upgrade_tests/upgrade_test.go +++ b/pkg/pgmodel/upgrade_tests/upgrade_test.go @@ -51,7 +51,9 @@ func TestMain(m *testing.M) { prevDBImage = "timescaledev/timescale_prometheus_extra:0.1.1-pg12" } flag.Parse() - _ = log.Init("debug") + _ = log.Init(log.Config{ + Level: "debug", + }) code = m.Run() os.Exit(code) } diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 23cda16601..5306e95fac 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -33,6 +33,7 @@ type Config struct { ListenAddr string TelemetryPath string PgmodelCfg pgclient.Config + LogCfg log.Config HaGroupLockID int64 RestElection bool PrometheusTimeout time.Duration @@ -57,6 +58,7 @@ var ( func ParseFlags(cfg *Config) (*Config, error) { pgclient.ParseFlags(&cfg.PgmodelCfg) + log.ParseFlags(&cfg.LogCfg) flag.StringVar(&cfg.ListenAddr, "web-listen-address", ":9201", "Address to listen on for web endpoints.") flag.StringVar(&cfg.TelemetryPath, "web-telemetry-path", "/metrics", "Address to listen on for web endpoints.") diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 82e7a6ebc6..54fef362bf 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -14,7 +14,9 @@ import ( func TestMain(m *testing.M) { flag.Parse() - err := log.Init("debug") + err := log.Init(log.Config{ + Level: "debug", + }) if err != nil { fmt.Println("Error initializing logger", err) diff --git a/pkg/util/election_test.go b/pkg/util/election_test.go index 1ca8712dc1..43c7b01697 100644 --- a/pkg/util/election_test.go +++ b/pkg/util/election_test.go @@ -205,7 +205,9 @@ func TestPrometheusLivenessCheck(t *testing.T) { } func TestMain(m *testing.M) { flag.Parse() - err := log.Init("debug") + err := log.Init(log.Config{ + Level: "debug", + }) if err != nil { panic(err) } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index a60a5efbb4..5b925c3a80 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -17,7 +17,9 @@ const ( ) func init() { - err := log.Init("debug") + err := log.Init(log.Config{ + Level: "debug", + }) if err != nil { panic(err) }