Skip to content

Commit

Permalink
Overrides module refactor (#2688)
Browse files Browse the repository at this point in the history
* Extract configuration out of overrides.Limits into dedicated a Config struct

* Move default limits under default_limits key

* Fix compile error

* Fix test

* Respect default settings when unmarshalling config

* Change overrides to ident format

* Add tempo-cli command to migrate configs

* Some more tweaks

* Fixes

* Rename limit to override

* Add missing test

* Address review comments

* Add changelog entry

* Update docs and example configs

* typo

---------

Co-authored-by: Koenraad Verheyden <[email protected]>
  • Loading branch information
mapno and Koenraad Verheyden authored Aug 24, 2023
1 parent ef29986 commit 3d88035
Show file tree
Hide file tree
Showing 44 changed files with 1,622 additions and 752 deletions.
28 changes: 27 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* [FEATURE] Add the `/api/status/buildinfo` endpoint [#2702](https://github.com/grafana/tempo/pull/2702) (@fabrizio-grafana)
* [FEATURE] New encoding vParquet3 with support for dedicated attribute columns (@mapno, @stoewer) [#2649](https://github.com/grafana/tempo/pull/2649)
* [FEATURE] Add filtering support to Generic Forwarding [#2742](https://github.com/grafana/tempo/pull/2742) (@Blinkuu)
* [FEATURE] Add cli command to print out summary of large traces [#2775](https://github.com/grafana/tempo/pull/2775) (@ie-pham)
* [CHANGE] Update Go to 1.21 [#2486](https://github.com/grafana/tempo/pull/2829) (@zalegrala)
* [ENHANCEMENT] Assert ingestion rate limits as early as possible [#2640](https://github.com/grafana/tempo/pull/2703) (@mghildiy)
* [ENHANCEMENT] Add several metrics-generator fields to user-configurable overrides [#2711](https://github.com/grafana/tempo/pull/2711) (@kvrhdn)
Expand All @@ -11,7 +12,32 @@
* [BUGFIX] Fix panic in metrics summary api [#2738](https://github.com/grafana/tempo/pull/2738) (@mdisibio)
* [BUGFIX] Fix node role auth IDMSv1 [#2760](https://github.com/grafana/tempo/pull/2760) (@coufalja)
* [BUGFIX] Only search ingester blocks that fall within the request time range. [#2783](https://github.com/grafana/tempo/pull/2783) (@joe-elliott)
* [FEATURE] Add cli command to print out summary of large traces [#2775](https://github.com/grafana/tempo/pull/2775) (@ie-pham)
* [CHANGE] Overrides module refactor [#2688](https://github.com/grafana/tempo/pull/2688) (@mapno)
Added new `defaults` block to the overrides' module. Overrides change to indented syntax.
Old config:
```
overrides:
ingestion_rate_strategy: local
ingestion_rate_limit_bytes: 12345
ingestion_burst_size_bytes: 67890
max_search_duration: 17s
forwarders: ['foo']
metrics_generator_processors: [service-graphs, span-metrics]
```
New config:
```
overrides:
defaults:
ingestion:
rate_strategy: local
rate_limit_bytes: 12345
burst_size_bytes: 67890
read:
max_search_duration: 17s
forwarders: ['foo']
metrics_generator:
processors: [service-graphs, span-metrics]
```

## v2.2.1 / 2023-08-??

Expand Down
102 changes: 102 additions & 0 deletions cmd/tempo-cli/cmd-migrate-overrides-config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package main

import (
"bytes"
"context"
"flag"
"fmt"
"net/http"
"net/url"
"os"

"github.com/grafana/dskit/services"

"github.com/grafana/tempo/modules/overrides"

"github.com/grafana/tempo/cmd/tempo/app"
"gopkg.in/yaml.v2"
)

type migrateOverridesConfigCmd struct {
ConfigFile string `arg:"" help:"Path to tempo config file"`

ConfigDest string `type:"path" short:"d" help:"Path to tempo config file. If not specified, output to stdout"`
OverridesDest string `type:"path" short:"o" help:"Path to tempo overrides file. If not specified, output to stdout"`
}

func (cmd *migrateOverridesConfigCmd) Run(*globalOptions) error {
// Defaults
cfg := app.Config{}
cfg.RegisterFlagsAndApplyDefaults("", &flag.FlagSet{})

// Existing config
buff, err := os.ReadFile(cmd.ConfigFile)
if err != nil {
return fmt.Errorf("failed to read configFile %s: %w", cmd.ConfigFile, err)
}

if err := yaml.UnmarshalStrict(buff, &cfg); err != nil {
return fmt.Errorf("failed to parse configFile %s: %w", cmd.ConfigFile, err)
}

o, err := overrides.NewOverrides(cfg.Overrides)
if err != nil {
return fmt.Errorf("failed to load overrides module: %w", err)
}

if err := services.StartAndAwaitRunning(context.Background(), o); err != nil {
return fmt.Errorf("failed to start overrides module: %w", err)
}

buffer := bytes.NewBuffer(make([]byte, 0))
if err := o.WriteStatusRuntimeConfig(buffer, &http.Request{URL: &url.URL{}}); err != nil {
return fmt.Errorf("failed to output runtime config: %w", err)
}

var runtimeConfig struct {
Defaults overrides.Overrides `yaml:"defaults"`
PerTenantOverrides map[string]overrides.Config `yaml:"overrides"`
}
if err := yaml.UnmarshalStrict(buffer.Bytes(), &runtimeConfig); err != nil {
return fmt.Errorf("failed parsing overrides config: %w", err)
}

cfg.Overrides.Defaults = runtimeConfig.Defaults
configBytes, err := yaml.Marshal(cfg)
if err != nil {
return fmt.Errorf("failed to marshal config: %w", err)
}

if cmd.ConfigDest != "" {
if err := os.WriteFile(cmd.ConfigDest, configBytes, 0o644); err != nil {
return fmt.Errorf("failed to write config file: %w", err)
}
} else {
fmt.Println(cmd.ConfigFile)
// Only print the overrides block
partialCfg := struct {
Overrides overrides.Config `yaml:"overrides"`
}{Overrides: cfg.Overrides}
overridesBytes, err := yaml.Marshal(partialCfg)
if err != nil {
return fmt.Errorf("failed to marshal overrides: %w", err)
}
fmt.Println(string(overridesBytes))
}

overridesBytes, err := yaml.Marshal(runtimeConfig.PerTenantOverrides)
if err != nil {
return fmt.Errorf("failed to marshal overrides: %w", err)
}

if cmd.OverridesDest != "" {
if err := os.WriteFile(cmd.OverridesDest, overridesBytes, 0o644); err != nil {
return fmt.Errorf("failed to write overrides file: %w", err)
}
} else {
fmt.Println(cfg.Overrides.PerTenantOverrideConfig)
fmt.Println(string(overridesBytes))
}

return nil
}
3 changes: 2 additions & 1 deletion cmd/tempo-cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ var cli struct {
} `cmd:""`

Migrate struct {
Tenant migrateTenantCmd `cmd:"" help:"migrate tenant between two backends"`
Tenant migrateTenantCmd `cmd:"" help:"migrate tenant between two backends"`
OverridesConfig migrateOverridesConfigCmd `cmd:"" help:"migrate overrides config"`
} `cmd:""`
}

Expand Down
14 changes: 11 additions & 3 deletions cmd/tempo/app/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Config struct {
Ingester ingester.Config `yaml:"ingester,omitempty"`
Generator generator.Config `yaml:"metrics_generator,omitempty"`
StorageConfig storage.Config `yaml:"storage,omitempty"`
LimitsConfig overrides.Limits `yaml:"overrides,omitempty"`
Overrides overrides.Config `yaml:"overrides,omitempty"`
MemberlistKV memberlist.KVConfig `yaml:"memberlist,omitempty"`
UsageReport usagestats.Config `yaml:"usage_report,omitempty"`
}
Expand Down Expand Up @@ -114,7 +114,7 @@ func (c *Config) RegisterFlagsAndApplyDefaults(prefix string, f *flag.FlagSet) {
c.IngesterClient.GRPCClientConfig.GRPCCompression = "snappy"
flagext.DefaultValues(&c.GeneratorClient)
c.GeneratorClient.GRPCClientConfig.GRPCCompression = "snappy"
c.LimitsConfig.RegisterFlagsAndApplyDefaults(f)
c.Overrides.RegisterFlagsAndApplyDefaults(f)

c.Distributor.RegisterFlagsAndApplyDefaults(util.PrefixConfig(prefix, "distributor"), f)
c.Ingester.RegisterFlagsAndApplyDefaults(util.PrefixConfig(prefix, "ingester"), f)
Expand Down Expand Up @@ -197,6 +197,10 @@ func (c *Config) CheckConfig() []ConfigWarning {
warnings = append(warnings, warnTracesAndUserConfigurableOverridesStorageConflict)
}

if c.Overrides.ConfigType == overrides.ConfigTypeLegacy {
warnings = append(warnings, warnLegacyOverridesConfig)
}

return warnings
}

Expand Down Expand Up @@ -251,6 +255,10 @@ var (
warnStorageTraceBackendLocal = ConfigWarning{
Message: "Local backend will not correctly retrieve traces with a distributed deployment unless all components have access to the same disk. You should probably be using object storage as a backend.",
}
warnLegacyOverridesConfig = ConfigWarning{
Message: "Inline, unscoped overrides are deprecated. Please use the new overrides config format.",
}

warnTracesAndUserConfigurableOverridesStorageConflict = ConfigWarning{
Message: "Trace storage conflicts with user-configurable overrides storage",
}
Expand All @@ -265,7 +273,7 @@ func newV2Warning(setting string) ConfigWarning {

func (c *Config) tracesAndOverridesStorageConflict() bool {
traceStorage := c.StorageConfig.Trace
overridesStorage := c.LimitsConfig.UserConfigurableOverrides.Client
overridesStorage := c.Overrides.UserConfigurableOverridesConfig.Client

if traceStorage.Backend != overridesStorage.Backend {
return false
Expand Down
14 changes: 7 additions & 7 deletions cmd/tempo/app/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ func TestConfig_CheckConfig(t *testing.T) {
cfg := newDefaultConfig()
cfg.StorageConfig.Trace.Backend = backend.Local
cfg.StorageConfig.Trace.Local.Path = "/var/tempo"
cfg.LimitsConfig.UserConfigurableOverrides.Client.Backend = backend.Local
cfg.LimitsConfig.UserConfigurableOverrides.Client.Local.Path = "/var/tempo"
cfg.Overrides.UserConfigurableOverridesConfig.Client.Backend = backend.Local
cfg.Overrides.UserConfigurableOverridesConfig.Client.Local.Path = "/var/tempo"
return cfg
}(),
expect: []ConfigWarning{warnTracesAndUserConfigurableOverridesStorageConflict},
Expand All @@ -121,9 +121,9 @@ func TestConfig_CheckConfig(t *testing.T) {
cfg.StorageConfig.Trace.Backend = backend.GCS
cfg.StorageConfig.Trace.GCS.BucketName = "bucketname"
cfg.StorageConfig.Trace.GCS.Prefix = "tempo"
cfg.LimitsConfig.UserConfigurableOverrides.Client.Backend = backend.GCS
cfg.LimitsConfig.UserConfigurableOverrides.Client.GCS.BucketName = "bucketname"
cfg.LimitsConfig.UserConfigurableOverrides.Client.GCS.Prefix = "tempo"
cfg.Overrides.UserConfigurableOverridesConfig.Client.Backend = backend.GCS
cfg.Overrides.UserConfigurableOverridesConfig.Client.GCS.BucketName = "bucketname"
cfg.Overrides.UserConfigurableOverridesConfig.Client.GCS.Prefix = "tempo"
return cfg
}(),
expect: []ConfigWarning{warnTracesAndUserConfigurableOverridesStorageConflict},
Expand All @@ -134,8 +134,8 @@ func TestConfig_CheckConfig(t *testing.T) {
cfg := newDefaultConfig()
cfg.StorageConfig.Trace.Backend = backend.GCS
cfg.StorageConfig.Trace.GCS.BucketName = "my-bucket"
cfg.LimitsConfig.UserConfigurableOverrides.Client.Backend = backend.S3
cfg.LimitsConfig.UserConfigurableOverrides.Client.S3.Bucket = "my-bucket"
cfg.Overrides.UserConfigurableOverridesConfig.Client.Backend = backend.S3
cfg.Overrides.UserConfigurableOverridesConfig.Client.S3.Bucket = "my-bucket"
return cfg
}(),
expect: nil,
Expand Down
8 changes: 4 additions & 4 deletions cmd/tempo/app/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,23 +181,23 @@ func (t *App) initReadRing(cfg ring.Config, name, key string) (*ring.Ring, error
}

func (t *App) initOverrides() (services.Service, error) {
o, err := overrides.NewOverrides(t.cfg.LimitsConfig)
o, err := overrides.NewOverrides(t.cfg.Overrides)
if err != nil {
return nil, fmt.Errorf("failed to create overrides %w", err)
}
t.Overrides = o

prometheus.MustRegister(&t.cfg.LimitsConfig)
prometheus.MustRegister(&t.cfg.Overrides)

if t.cfg.LimitsConfig.PerTenantOverrideConfig != "" {
if t.cfg.Overrides.PerTenantOverrideConfig != "" {
prometheus.MustRegister(t.Overrides)
}

return t.Overrides, nil
}

func (t *App) initOverridesAPI() (services.Service, error) {
cfg := t.cfg.LimitsConfig.UserConfigurableOverrides
cfg := t.cfg.Overrides.UserConfigurableOverridesConfig

if !cfg.Enabled {
return services.NewIdleService(nil, nil), nil
Expand Down
6 changes: 4 additions & 2 deletions docs/sources/tempo/api_docs/metrics-summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ This API returns RED metrics (span count, erroring span count, and latency infor

To enable the experimental metrics summary API you must turn on the local blocks processor in the metrics generator. Be aware that the generator will use considerably more resources including disk space if this is enabled:

```
```yaml
overrides:
metrics_generator_processors: [..., 'local-blocks']
defaults:
metrics_generator:
processors: [..., 'local-blocks']
```
## Request
Expand Down
Loading

0 comments on commit 3d88035

Please sign in to comment.