Skip to content

Delay logging config warnings until the logger has been initialized #2645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yvrhdn merged 2 commits into from
Jul 13, 2023
Merged

Delay logging config warnings until the logger has been initialized #2645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae171e0
Delay logging config warnings until the logger has been initialized
Jul 12, 2023
df0dac9
Update CHANGELOG.md
Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ To make use of filtering, configure `autocomplete_filtering_enabled`.
* [BUGFIX] Fixes issue where matches and other spanset level attributes were not persisted to the TraceQL results. [#2490](https://github.com/grafana/tempo/pull/2490)
* [BUGFIX] Fixes issue where ingester search could occasionally fail with file does not exist error [#2534](https://github.com/grafana/tempo/issues/2534) (@mdisibio)
* [BUGFIX] Tempo failed to find meta.json path after adding prefix in S3/GCS/Azure configuration. [#2585](https://github.com/grafana/tempo/issues/2585) (@WildCatFish)
* [BUGFIX] Delay logging config warnings until the logger has been initialized [#2645](https://github.com/grafana/tempo/pull/2645) (@kvrhdn)
* [CHANGE] **Breaking Change** Rename s3.insecure_skip_verify [#2407](https://github.com/grafana/tempo/pull/2407) (@zalegrala)
```yaml
storage:
Expand Down Expand Up @@ -795,4 +796,4 @@ Jsonnet users will now need to specify a storage request and limit for the gener
* [BUGFIX] S3 multi-part upload errors [#306](https://github.com/grafana/tempo/pull/325)
* [BUGFIX] Increase Prometheus `notfound` metric on tempo-vulture. [#301](https://github.com/grafana/tempo/pull/301)
* [BUGFIX] Return 404 if searching for a tenant id that does not exist in the backend. [#321](https://github.com/grafana/tempo/pull/321)
* [BUGFIX] Prune in-memory blocks from missing tenants. [#314](https://github.com/grafana/tempo/pull/314)
* [BUGFIX] Prune in-memory blocks from missing tenants. [#314](https://github.com/grafana/tempo/pull/314)
36 changes: 21 additions & 15 deletions cmd/tempo/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ import (
"github.com/drone/envsubst"
"github.com/go-kit/log/level"
"github.com/grafana/dskit/flagext"
"github.com/grafana/tempo/cmd/tempo/app"
"github.com/grafana/tempo/cmd/tempo/build"
"github.com/grafana/tempo/pkg/util/log"
ot "github.com/opentracing/opentracing-go"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
Expand All @@ -32,6 +29,10 @@ import (
tracesdk "go.opentelemetry.io/otel/sdk/trace"
semconv "go.opentelemetry.io/otel/semconv/v1.18.0"
"gopkg.in/yaml.v2"

"github.com/grafana/tempo/cmd/tempo/app"
"github.com/grafana/tempo/cmd/tempo/build"
"github.com/grafana/tempo/pkg/util/log"
)

const appName = "tempo"
Expand All @@ -55,7 +56,7 @@ func main() {
ballastMBs := flag.Int("mem-ballast-size-mbs", 0, "Size of memory ballast to allocate in MBs.")
mutexProfileFraction := flag.Int("mutex-profile-fraction", 0, "Enable mutex profiling.")

config, err := loadConfig()
config, warningLogs, err := loadConfig()
if err != nil {
fmt.Fprintf(os.Stderr, "failed parsing config: %v\n", err)
os.Exit(1)
Expand All @@ -72,6 +73,11 @@ func main() {
}
log.InitLogger(&config.Server)

// Log warnings from loading the config now the logger is initialized
for _, warning := range warningLogs {
level.Warn(log.Logger).Log(warning...)
}

// Init tracer
var shutdownTracer func()
if config.UseOTelTracer {
Expand Down Expand Up @@ -108,23 +114,23 @@ func main() {
runtime.KeepAlive(ballast)
}

func configIsValid(config *app.Config) bool {
func configIsValid(config *app.Config) (warningLogs [][]any, isValid bool) {
// Warn the user for suspect configurations
if warnings := config.CheckConfig(); len(warnings) != 0 {
level.Warn(log.Logger).Log("-- CONFIGURATION WARNINGS --")
warningLogs = append(warningLogs, []any{"msg", "-- CONFIGURATION WARNINGS --"})
for _, w := range warnings {
output := []any{"msg", w.Message}
if w.Explain != "" {
output = append(output, "explain", w.Explain)
}
level.Warn(log.Logger).Log(output...)
warningLogs = append(warningLogs, output)
}
return false
return warningLogs, false
}
return true
return warningLogs, true
}

func loadConfig() (*app.Config, error) {
func loadConfig() (*app.Config, [][]any, error) {
const (
configFileOption = "config.file"
configExpandEnvOption = "config.expand-env"
Expand Down Expand Up @@ -163,20 +169,20 @@ func loadConfig() (*app.Config, error) {
if configFile != "" {
buff, err := os.ReadFile(configFile)
if err != nil {
return nil, fmt.Errorf("failed to read configFile %s: %w", configFile, err)
return nil, nil, fmt.Errorf("failed to read configFile %s: %w", configFile, err)
}

if configExpandEnv {
s, err := envsubst.EvalEnv(string(buff))
if err != nil {
return nil, fmt.Errorf("failed to expand env vars from configFile %s: %w", configFile, err)
return nil, nil, fmt.Errorf("failed to expand env vars from configFile %s: %w", configFile, err)
}
buff = []byte(s)
}

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

}
Expand All @@ -200,15 +206,15 @@ func loadConfig() (*app.Config, error) {
}

// after finalizing the configuration, verify its validity and exit if config.verify flag is true.
isValid := configIsValid(config)
warningLogs, isValid := configIsValid(config)
if configVerify {
if !isValid {
os.Exit(1)
}
os.Exit(0)
}

return config, nil
return config, warningLogs, nil
}

func installOpenTracingTracer(config *app.Config) (func(), error) {
Expand Down