Skip to content

Commit

Permalink
add RegisterFlagsWithPrefix to config structs
Browse files Browse the repository at this point in the history
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 grafana#2405.
  • Loading branch information
rfratto committed Jul 24, 2020
1 parent 2e1fd42 commit f1a79cd
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 22 deletions.
28 changes: 17 additions & 11 deletions pkg/promtail/client/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions pkg/promtail/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
12 changes: 9 additions & 3 deletions pkg/promtail/positions/positions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 10 additions & 2 deletions pkg/promtail/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions pkg/promtail/targets/file/filetarget.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit f1a79cd

Please sign in to comment.