From d39dc09ca655f87bf0c0a421be7dec8c6b848205 Mon Sep 17 00:00:00 2001 From: Ed Welch Date: Wed, 3 Apr 2024 14:35:22 -0400 Subject: [PATCH] refactor!: refactor how we do defaults for runtime overrides (#12448) Signed-off-by: Edward Welch Co-authored-by: Dylan Guedes Co-authored-by: J Stickler --- CHANGELOG.md | 1 + cmd/loki/main.go | 2 + docs/sources/configure/_index.md | 37 ++++++++++++++++- docs/sources/setup/upgrade/_index.md | 6 +++ pkg/loki/loki.go | 8 ++-- pkg/loki/modules.go | 1 + pkg/loki/runtime_config.go | 5 +-- pkg/loki/runtime_config_test.go | 23 ++++++----- pkg/runtime/config.go | 44 +++++++++++++++++++-- production/ksonnet/loki/overrides.libsonnet | 9 ----- tools/doc-generator/parse/root_blocks.go | 8 +++- 11 files changed, 109 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 289045cf2f..294f7ee010 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,7 @@ * [11121](https://github.com/grafana/loki/pull/11121) **periklis** Ensure all lifecycler cfgs ref a valid IPv6 addr and port combination * [10650](https://github.com/grafana/loki/pull/10650) **matthewpi** Ensure the frontend uses a valid IPv6 addr and port combination * [11665](https://github.com/grafana/loki/pull/11665) **salvacorts** Deprecate and flip `-legacy-read-mode` flag to `false` by default. +* [12448](https://github.com/grafana/loki/pull/12448/files) **slim-bean** BREAKING CHANGE: refactor how we do defaults for runtime overrides #### Promtail diff --git a/cmd/loki/main.go b/cmd/loki/main.go index 250568203b..d9f4613977 100644 --- a/cmd/loki/main.go +++ b/cmd/loki/main.go @@ -17,6 +17,7 @@ import ( "github.com/prometheus/common/version" "github.com/grafana/loki/v3/pkg/loki" + loki_runtime "github.com/grafana/loki/v3/pkg/runtime" "github.com/grafana/loki/v3/pkg/util" _ "github.com/grafana/loki/v3/pkg/util/build" "github.com/grafana/loki/v3/pkg/util/cfg" @@ -49,6 +50,7 @@ func main() { // call it atleast once, the defaults are set to an empty struct. // We call it with the flag values so that the config file unmarshalling only overrides the values set in the config. validation.SetDefaultLimitsForYAMLUnmarshalling(config.LimitsConfig) + loki_runtime.SetDefaultLimitsForYAMLUnmarshalling(config.OperationalConfig) // Init the logger which will honor the log level set in config.Server if reflect.DeepEqual(&config.Server.LogLevel, &log.Level{}) { diff --git a/docs/sources/configure/_index.md b/docs/sources/configure/_index.md index cce92e24eb..bd561e1116 100644 --- a/docs/sources/configure/_index.md +++ b/docs/sources/configure/_index.md @@ -186,7 +186,9 @@ Pass the `-config.expand-env` flag at the command line to enable this way of set # shards for performance. [compactor: ] -# The limits_config block configures global and per-tenant limits in Loki. +# The limits_config block configures global and per-tenant limits in Loki. The +# values here can be overridden in the `overrides` section of the runtime_config +# file [limits_config: ] # The frontend_worker configures the worker - running within the Loki querier - @@ -208,6 +210,11 @@ Pass the `-config.expand-env` flag at the command line to enable this way of set # configuration file. [runtime_config: ] +# These are values which allow you to control aspects of Loki's operation, most +# commonly used for controlling types of higher verbosity logging, the values +# here can be overridden in the `configs` section of the `runtime_config` file. +[operational_config: ] + # Configuration for tracing. [tracing: ] @@ -2747,7 +2754,7 @@ retention: ### limits_config -The `limits_config` block configures global and per-tenant limits in Loki. +The `limits_config` block configures global and per-tenant limits in Loki. The values here can be overridden in the `overrides` section of the runtime_config file ```yaml # Whether the ingestion rate limit should be applied individually to each @@ -3675,6 +3682,32 @@ Configuration for 'runtime config' module, responsible for reloading runtime con [file: | default = ""] ``` +### operational_config + +These are values which allow you to control aspects of Loki's operation, most commonly used for controlling types of higher verbosity logging, the values here can be overridden in the `configs` section of the `runtime_config` file. + +```yaml +# Log every new stream created by a push request (very verbose, recommend to +# enable via runtime config only). +# CLI flag: -operation-config.log-stream-creation +[log_stream_creation: | default = false] + +# Log every push request (very verbose, recommend to enable via runtime config +# only). +# CLI flag: -operation-config.log-push-request +[log_push_request: | default = false] + +# Log every stream in a push request (very verbose, recommend to enable via +# runtime config only). +# CLI flag: -operation-config.log-push-request-streams +[log_push_request_streams: | default = false] + +# Log push errors with a rate limited logger, will show client push errors +# without overly spamming logs. +# CLI flag: -operation-config.limited-log-push-errors +[limited_log_push_errors: | default = true] +``` + ### tracing Configuration for `tracing`. diff --git a/docs/sources/setup/upgrade/_index.md b/docs/sources/setup/upgrade/_index.md index 84fac5835b..74d8b97131 100644 --- a/docs/sources/setup/upgrade/_index.md +++ b/docs/sources/setup/upgrade/_index.md @@ -38,6 +38,12 @@ The output is incredibly verbose as it shows the entire internal config struct u ### Loki +#### Removed the `default` section of the runtime overrides config file. + +This was introduced in 2.9 and likely not widely used. This only affects you if you run Loki with a runtime config file AND you had populated the new `default` block added in 2.9. + +The `default` block was removed and instead a top level config now exists in the standard Loki config called `operational_config`, you can set default values here for runtime configs. + #### Removed `shared_store` and `shared_store_key_prefix` from shipper configuration The following CLI flags and the corresponding YAML settings to configure shared store for TSDB and BoltDB shippers are now removed: diff --git a/pkg/loki/loki.go b/pkg/loki/loki.go index c77ef07892..536ec5728b 100644 --- a/pkg/loki/loki.go +++ b/pkg/loki/loki.go @@ -99,9 +99,10 @@ type Config struct { TableManager index.TableManagerConfig `yaml:"table_manager,omitempty"` MemberlistKV memberlist.KVConfig `yaml:"memberlist"` - RuntimeConfig runtimeconfig.Config `yaml:"runtime_config,omitempty"` - Tracing tracing.Config `yaml:"tracing"` - Analytics analytics.Config `yaml:"analytics"` + RuntimeConfig runtimeconfig.Config `yaml:"runtime_config,omitempty"` + OperationalConfig runtime.Config `yaml:"operational_config,omitempty"` + Tracing tracing.Config `yaml:"tracing"` + Analytics analytics.Config `yaml:"analytics"` LegacyReadTarget bool `yaml:"legacy_read_target,omitempty" doc:"hidden|deprecated"` @@ -171,6 +172,7 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { c.BloomCompactor.RegisterFlags(f) c.QueryScheduler.RegisterFlags(f) c.Analytics.RegisterFlags(f) + c.OperationalConfig.RegisterFlags(f) } func (c *Config) registerServerFlagsWithChangedDefaultValues(fs *flag.FlagSet) { diff --git a/pkg/loki/modules.go b/pkg/loki/modules.go index 79c8683633..599218a5bf 100644 --- a/pkg/loki/modules.go +++ b/pkg/loki/modules.go @@ -260,6 +260,7 @@ func (t *Loki) initRuntimeConfig() (services.Service, error) { // make sure to set default limits before we start loading configuration into memory validation.SetDefaultLimitsForYAMLUnmarshalling(t.Cfg.LimitsConfig) + runtime.SetDefaultLimitsForYAMLUnmarshalling(t.Cfg.OperationalConfig) var err error t.runtimeConfig, err = runtimeconfig.New(t.Cfg.RuntimeConfig, "loki", prometheus.WrapRegistererWithPrefix("loki_", prometheus.DefaultRegisterer), util_log.Logger) diff --git a/pkg/loki/runtime_config.go b/pkg/loki/runtime_config.go index e8e3c7e315..c2e72fbd4b 100644 --- a/pkg/loki/runtime_config.go +++ b/pkg/loki/runtime_config.go @@ -21,9 +21,6 @@ type runtimeConfigValues struct { TenantLimits map[string]*validation.Limits `yaml:"overrides"` TenantConfig map[string]*runtime.Config `yaml:"configs"` - // DefaultConfig defines the default runtime configuration. Used if a tenant runtime configuration wasn't given. - DefaultConfig *runtime.Config `yaml:"default"` - Multi kv.MultiRuntimeConfig `yaml:"multi_kv_config"` } @@ -107,7 +104,7 @@ func (t *tenantConfigProvider) TenantConfig(userID string) *runtime.Config { if tenantCfg, ok := cfg.TenantConfig[userID]; ok { return tenantCfg } - return cfg.DefaultConfig + return nil } func multiClientRuntimeConfigChannel(manager *runtimeconfig.Manager) func() <-chan kv.MultiRuntimeConfig { diff --git a/pkg/loki/runtime_config_test.go b/pkg/loki/runtime_config_test.go index cf60445592..36126841dc 100644 --- a/pkg/loki/runtime_config_test.go +++ b/pkg/loki/runtime_config_test.go @@ -87,9 +87,6 @@ overrides: func Test_DefaultConfig(t *testing.T) { runtimeGetter := newTestRuntimeconfig(t, ` -default: - log_push_request: true - limited_log_push_errors: false configs: "1": log_push_request: false @@ -98,16 +95,15 @@ configs: log_push_request: true `) - user1 := runtimeGetter.TenantConfig("1") - user2 := runtimeGetter.TenantConfig("2") - user3 := runtimeGetter.TenantConfig("3") + tenantConfigs, err := runtime.NewTenantConfigs(runtimeGetter) + require.NoError(t, err) - require.Equal(t, false, user1.LogPushRequest) - require.Equal(t, false, user1.LimitedLogPushErrors) - require.Equal(t, false, user2.LimitedLogPushErrors) - require.Equal(t, true, user2.LogPushRequest) - require.Equal(t, false, user3.LimitedLogPushErrors) - require.Equal(t, true, user3.LogPushRequest) + require.Equal(t, false, tenantConfigs.LogPushRequest("1")) + require.Equal(t, false, tenantConfigs.LimitedLogPushErrors("1")) + require.Equal(t, false, tenantConfigs.LimitedLogPushErrors("2")) + require.Equal(t, true, tenantConfigs.LogPushRequest("2")) + require.Equal(t, true, tenantConfigs.LimitedLogPushErrors("3")) + require.Equal(t, false, tenantConfigs.LogPushRequest("3")) } func newTestRuntimeconfig(t *testing.T, yaml string) runtime.TenantConfigProvider { @@ -126,7 +122,10 @@ func newTestRuntimeconfig(t *testing.T, yaml string) runtime.TenantConfigProvide } flagset := flag.NewFlagSet("", flag.PanicOnError) var defaults validation.Limits + var operations runtime.Config defaults.RegisterFlags(flagset) + operations.RegisterFlags(flagset) + runtime.SetDefaultLimitsForYAMLUnmarshalling(operations) require.NoError(t, flagset.Parse(nil)) reg := prometheus.NewPedanticRegistry() diff --git a/pkg/runtime/config.go b/pkg/runtime/config.go index 234b5c0c5e..85f8dc3d81 100644 --- a/pkg/runtime/config.go +++ b/pkg/runtime/config.go @@ -1,5 +1,9 @@ package runtime +import ( + "flag" +) + type Config struct { LogStreamCreation bool `yaml:"log_stream_creation"` LogPushRequest bool `yaml:"log_push_request"` @@ -9,7 +13,26 @@ type Config struct { LimitedLogPushErrors bool `yaml:"limited_log_push_errors"` } -var EmptyConfig = &Config{} +// RegisterFlags adds the flags required to config this to the given FlagSet +func (cfg *Config) RegisterFlags(f *flag.FlagSet) { + f.BoolVar(&cfg.LogStreamCreation, "operation-config.log-stream-creation", false, "Log every new stream created by a push request (very verbose, recommend to enable via runtime config only).") + f.BoolVar(&cfg.LogPushRequest, "operation-config.log-push-request", false, "Log every push request (very verbose, recommend to enable via runtime config only).") + f.BoolVar(&cfg.LogPushRequestStreams, "operation-config.log-push-request-streams", false, "Log every stream in a push request (very verbose, recommend to enable via runtime config only).") + f.BoolVar(&cfg.LimitedLogPushErrors, "operation-config.limited-log-push-errors", true, "Log push errors with a rate limited logger, will show client push errors without overly spamming logs.") +} + +// When we load YAML from disk, we want the various per-customer limits +// to default to any values specified on the command line, not default +// command line values. This global contains those values. I (Tom) cannot +// find a nicer way I'm afraid. +var defaultConfig *Config + +// SetDefaultLimitsForYAMLUnmarshalling sets global default limits, used when loading +// Limits from YAML files. This is used to ensure per-tenant limits are defaulted to +// those values. +func SetDefaultLimitsForYAMLUnmarshalling(defaults Config) { + defaultConfig = &defaults +} // TenantConfigProvider serves a tenant or default config. type TenantConfigProvider interface { @@ -23,13 +46,26 @@ type TenantConfigs struct { } // DefaultTenantConfigs creates and returns a new TenantConfigs with the defaults populated. +// Only useful for testing, the provider will ignore any tenants passed in. func DefaultTenantConfigs() *TenantConfigs { return &TenantConfigs{ - TenantConfigProvider: nil, + TenantConfigProvider: &defaultsOnlyConfigProvider{}, + } +} + +type defaultsOnlyConfigProvider struct { +} + +// TenantConfig implementation for defaultsOnlyConfigProvider, ignores the tenant input and only returns a default config +func (t *defaultsOnlyConfigProvider) TenantConfig(_ string) *Config { + if defaultConfig == nil { + defaultConfig = &Config{} + defaultConfig.RegisterFlags(flag.NewFlagSet("", flag.PanicOnError)) } + return defaultConfig } -// NewTenantConfig makes a new TenantConfigs +// NewTenantConfigs makes a new TenantConfigs func NewTenantConfigs(configProvider TenantConfigProvider) (*TenantConfigs, error) { return &TenantConfigs{ TenantConfigProvider: configProvider, @@ -43,7 +79,7 @@ func (o *TenantConfigs) getOverridesForUser(userID string) *Config { return l } } - return EmptyConfig + return defaultConfig } func (o *TenantConfigs) LogStreamCreation(userID string) bool { diff --git a/production/ksonnet/loki/overrides.libsonnet b/production/ksonnet/loki/overrides.libsonnet index 4b83e1df58..53313628c7 100644 --- a/production/ksonnet/loki/overrides.libsonnet +++ b/production/ksonnet/loki/overrides.libsonnet @@ -25,14 +25,6 @@ local k = import 'ksonnet-util/kausal.libsonnet'; // log_push_request_streams: true, // }, }, - - defaultRuntimeConfigs: { - // override the default runtime configs. Useful to change a behavior globally across multiple tenants. See pkg/loki/runtime_config.go - // - // log_stream_creation: true, - // log_push_request: true, - // log_push_request_streams: true, - }, }, local configMap = k.core.v1.configMap, @@ -43,7 +35,6 @@ local k = import 'ksonnet-util/kausal.libsonnet'; { overrides: $._config.overrides, configs: $._config.runtimeConfigs, - default: $._config.defaultRuntimeConfigs, } + (if std.length($._config.multi_kv_config) > 0 then { multi_kv_config: $._config.multi_kv_config } else {}), ), diff --git a/tools/doc-generator/parse/root_blocks.go b/tools/doc-generator/parse/root_blocks.go index 37debb7c41..731a629016 100644 --- a/tools/doc-generator/parse/root_blocks.go +++ b/tools/doc-generator/parse/root_blocks.go @@ -27,6 +27,7 @@ import ( "github.com/grafana/loki/v3/pkg/querier/queryrange" querier_worker "github.com/grafana/loki/v3/pkg/querier/worker" "github.com/grafana/loki/v3/pkg/ruler" + "github.com/grafana/loki/v3/pkg/runtime" "github.com/grafana/loki/v3/pkg/scheduler" "github.com/grafana/loki/v3/pkg/storage" "github.com/grafana/loki/v3/pkg/storage/chunk/cache" @@ -133,7 +134,7 @@ var ( { Name: "limits_config", StructType: []reflect.Type{reflect.TypeOf(validation.Limits{})}, - Desc: "The limits_config block configures global and per-tenant limits in Loki.", + Desc: "The limits_config block configures global and per-tenant limits in Loki. The values here can be overridden in the `overrides` section of the runtime_config file", }, { Name: "frontend_worker", @@ -151,6 +152,11 @@ var ( StructType: []reflect.Type{reflect.TypeOf(runtimeconfig.Config{})}, Desc: "Configuration for 'runtime config' module, responsible for reloading runtime configuration file.", }, + { + Name: "operational_config", + StructType: []reflect.Type{reflect.TypeOf(runtime.Config{})}, + Desc: "These are values which allow you to control aspects of Loki's operation, most commonly used for controlling types of higher verbosity logging, the values here can be overridden in the `configs` section of the `runtime_config` file.", + }, { Name: "tracing", StructType: []reflect.Type{reflect.TypeOf(tracing.Config{})},