From f1a79cd834d100f677ad152188078b723828ace4 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Fri, 24 Jul 2020 10:37:58 -0400 Subject: [PATCH] add RegisterFlagsWithPrefix to config structs All config structs now have a RegisterFlagsWithPrefix function that allows specifying a prefixed string to append to all flag names. This may be used by clients to namespace support for Promtail in a larger application (e.g., grafana/agent). When implementing this PR, I noticed a few config structs were incorrectly registering to the global flagset rather than the flagset passed to RegisterFlags. This PR also includes a fix for that and ensures every option is registered to the flagset in the parameter. Related to #2405. --- pkg/promtail/client/config.go | 28 +++++++++++++++---------- pkg/promtail/config/config.go | 14 +++++++++---- pkg/promtail/positions/positions.go | 12 ++++++++--- pkg/promtail/server/server.go | 12 +++++++++-- pkg/promtail/targets/file/filetarget.go | 10 +++++++-- 5 files changed, 54 insertions(+), 22 deletions(-) diff --git a/pkg/promtail/client/config.go b/pkg/promtail/client/config.go index 3d5058c7d366..444490acd1cb 100644 --- a/pkg/promtail/client/config.go +++ b/pkg/promtail/client/config.go @@ -29,19 +29,25 @@ type Config struct { TenantID string `yaml:"tenant_id"` } -// RegisterFlags registers flags. -func (c *Config) RegisterFlags(flags *flag.FlagSet) { - flags.Var(&c.URL, "client.url", "URL of log server") - flags.DurationVar(&c.BatchWait, "client.batch-wait", 1*time.Second, "Maximum wait period before sending batch.") - flags.IntVar(&c.BatchSize, "client.batch-size-bytes", 100*1024, "Maximum batch size to accrue before sending. ") +// RegisterFlags with prefix registers flags where every name is prefixed by +// prefix. If prefix is a non-empty string, prefix should end with a period. +func (c *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { + f.Var(&c.URL, prefix+"client.url", "URL of log server") + f.DurationVar(&c.BatchWait, prefix+"client.batch-wait", 1*time.Second, "Maximum wait period before sending batch.") + f.IntVar(&c.BatchSize, prefix+"client.batch-size-bytes", 100*1024, "Maximum batch size to accrue before sending. ") // Default backoff schedule: 0.5s, 1s, 2s, 4s, 8s, 16s, 32s, 64s, 128s, 256s(4.267m) For a total time of 511.5s(8.5m) before logs are lost - flag.IntVar(&c.BackoffConfig.MaxRetries, "client.max-retries", 10, "Maximum number of retires when sending batches.") - flag.DurationVar(&c.BackoffConfig.MinBackoff, "client.min-backoff", 500*time.Millisecond, "Initial backoff time between retries.") - flag.DurationVar(&c.BackoffConfig.MaxBackoff, "client.max-backoff", 5*time.Minute, "Maximum backoff time between retries.") - flag.DurationVar(&c.Timeout, "client.timeout", 10*time.Second, "Maximum time to wait for server to respond to a request") - flags.Var(&c.ExternalLabels, "client.external-labels", "list of external labels to add to each log (e.g: --client.external-labels=lb1=v1,lb2=v2)") + f.IntVar(&c.BackoffConfig.MaxRetries, prefix+"client.max-retries", 10, "Maximum number of retires when sending batches.") + f.DurationVar(&c.BackoffConfig.MinBackoff, prefix+"client.min-backoff", 500*time.Millisecond, "Initial backoff time between retries.") + f.DurationVar(&c.BackoffConfig.MaxBackoff, prefix+"client.max-backoff", 5*time.Minute, "Maximum backoff time between retries.") + f.DurationVar(&c.Timeout, prefix+"client.timeout", 10*time.Second, "Maximum time to wait for server to respond to a request") + f.Var(&c.ExternalLabels, prefix+"client.external-labels", "list of external labels to add to each log (e.g: --client.external-labels=lb1=v1,lb2=v2)") + + f.StringVar(&c.TenantID, prefix+"client.tenant-id", "", "Tenant ID to use when pushing logs to Loki.") +} - flags.StringVar(&c.TenantID, "client.tenant-id", "", "Tenant ID to use when pushing logs to Loki.") +// RegisterFlags registers flags. +func (c *Config) RegisterFlags(flags *flag.FlagSet) { + c.RegisterFlagsWithPrefix("", flags) } // UnmarshalYAML implement Yaml Unmarshaler diff --git a/pkg/promtail/config/config.go b/pkg/promtail/config/config.go index 47ebb4094a97..3cd9e264510c 100644 --- a/pkg/promtail/config/config.go +++ b/pkg/promtail/config/config.go @@ -21,10 +21,16 @@ type Config struct { TargetConfig file.Config `yaml:"target_config,omitempty"` } +// RegisterFlags with prefix registers flags where every name is prefixed by +// prefix. If prefix is a non-empty string, prefix should end with a period. +func (c *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { + c.ServerConfig.RegisterFlagsWithPrefix(prefix, f) + c.ClientConfig.RegisterFlagsWithPrefix(prefix, f) + c.PositionsConfig.RegisterFlagsWithPrefix(prefix, f) + c.TargetConfig.RegisterFlagsWithPrefix(prefix, f) +} + // RegisterFlags registers flags. func (c *Config) RegisterFlags(f *flag.FlagSet) { - c.ServerConfig.RegisterFlags(f) - c.ClientConfig.RegisterFlags(f) - c.PositionsConfig.RegisterFlags(f) - c.TargetConfig.RegisterFlags(f) + c.RegisterFlagsWithPrefix("", f) } diff --git a/pkg/promtail/positions/positions.go b/pkg/promtail/positions/positions.go index bffef3fd6fdd..1d4d56066cc9 100644 --- a/pkg/promtail/positions/positions.go +++ b/pkg/promtail/positions/positions.go @@ -26,11 +26,17 @@ type Config struct { ReadOnly bool `yaml:"-"` } +// RegisterFlags with prefix registers flags where every name is prefixed by +// prefix. If prefix is a non-empty string, prefix should end with a period. +func (cfg *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { + f.DurationVar(&cfg.SyncPeriod, prefix+"positions.sync-period", 10*time.Second, "Period with this to sync the position file.") + f.StringVar(&cfg.PositionsFile, prefix+"positions.file", "/var/log/positions.yaml", "Location to read/write positions from.") + f.BoolVar(&cfg.IgnoreInvalidYaml, prefix+"positions.ignore-invalid-yaml", false, "whether to ignore & later overwrite positions files that are corrupted") +} + // RegisterFlags register flags. func (cfg *Config) RegisterFlags(flags *flag.FlagSet) { - flags.DurationVar(&cfg.SyncPeriod, "positions.sync-period", 10*time.Second, "Period with this to sync the position file.") - flag.StringVar(&cfg.PositionsFile, "positions.file", "/var/log/positions.yaml", "Location to read/write positions from.") - flag.BoolVar(&cfg.IgnoreInvalidYaml, "positions.ignore-invalid-yaml", false, "whether to ignore & later overwrite positions files that are corrupted") + cfg.RegisterFlagsWithPrefix("", flags) } // Positions tracks how far through each file we've read. diff --git a/pkg/promtail/server/server.go b/pkg/promtail/server/server.go index 21fa07a2c442..11d18ef7c894 100644 --- a/pkg/promtail/server/server.go +++ b/pkg/promtail/server/server.go @@ -50,10 +50,18 @@ type Config struct { Disable bool `yaml:"disable"` } +// RegisterFlags with prefix registers flags where every name is prefixed by +// prefix. If prefix is a non-empty string, prefix should end with a period. +func (cfg *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { + // NOTE: weaveworks server's config can't be registered with a prefix. + cfg.Config.RegisterFlags(f) + + f.BoolVar(&cfg.Disable, prefix+"server.disable", false, "Disable the http and grpc server.") +} + // RegisterFlags adds the flags required to config this to the given FlagSet func (cfg *Config) RegisterFlags(f *flag.FlagSet) { - cfg.Config.RegisterFlags(f) - f.BoolVar(&cfg.Disable, "server.disable", false, "Disable the http and grpc server.") + cfg.RegisterFlagsWithPrefix("", f) } // New makes a new Server diff --git a/pkg/promtail/targets/file/filetarget.go b/pkg/promtail/targets/file/filetarget.go index efd1c8ad8797..38bde073f58b 100644 --- a/pkg/promtail/targets/file/filetarget.go +++ b/pkg/promtail/targets/file/filetarget.go @@ -60,10 +60,16 @@ type Config struct { Stdin bool `yaml:"stdin"` } +// RegisterFlags with prefix registers flags where every name is prefixed by +// prefix. If prefix is a non-empty string, prefix should end with a period. +func (cfg *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { + f.DurationVar(&cfg.SyncPeriod, prefix+"target.sync-period", 10*time.Second, "Period to resync directories being watched and files being tailed.") + f.BoolVar(&cfg.Stdin, prefix+"stdin", false, "Set to true to pipe logs to promtail.") +} + // RegisterFlags register flags. func (cfg *Config) RegisterFlags(flags *flag.FlagSet) { - flags.DurationVar(&cfg.SyncPeriod, "target.sync-period", 10*time.Second, "Period to resync directories being watched and files being tailed.") - flags.BoolVar(&cfg.Stdin, "stdin", false, "Set to true to pipe logs to promtail.") + cfg.RegisterFlagsWithPrefix("", flags) } // FileTarget describes a particular set of logs.