From 80ba606b33f722bf8e049d109e92ba6d8809afe6 Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Thu, 31 Aug 2023 14:52:18 +0200 Subject: [PATCH 1/2] remove deprecated log.request.decision flag Signed-off-by: Coleen Iona Quadros --- cmd/thanos/query.go | 6 +-- cmd/thanos/query_frontend.go | 3 +- cmd/thanos/receive.go | 2 +- cmd/thanos/rule.go | 6 +-- cmd/thanos/sidecar.go | 2 +- cmd/thanos/store.go | 4 +- docs/components/query-frontend.md | 9 ----- docs/components/query.md | 9 ----- docs/components/rule.md | 9 ----- pkg/logging/options.go | 62 +++---------------------------- 10 files changed, 15 insertions(+), 97 deletions(-) diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go index 068dd3ae6f..969e2baa1d 100644 --- a/cmd/thanos/query.go +++ b/cmd/thanos/query.go @@ -97,8 +97,6 @@ func registerQuery(app *extkingpin.App) { webPrefixHeaderName := cmd.Flag("web.prefix-header", "Name of HTTP request header used for dynamic prefixing of UI links and redirects. This option is ignored if web.external-prefix argument is set. Security risk: enable this option only if a reverse proxy in front of thanos is resetting the header. The --web.prefix-header=X-Forwarded-Prefix option can be useful, for example, if Thanos UI is served via Traefik reverse proxy with PathPrefixStrip option enabled, which sends the stripped prefix value in X-Forwarded-Prefix header. This allows thanos UI to be served on a sub-path.").Default("").String() webDisableCORS := cmd.Flag("web.disable-cors", "Whether to disable CORS headers to be set by Thanos. By default Thanos sets CORS headers to be allowed by all.").Default("false").Bool() - reqLogDecision := cmd.Flag("log.request.decision", "Deprecation Warning - This flag would be soon deprecated, and replaced with `request.logging-config`. Request Logging for logging the start and end of requests. By default this flag is disabled. LogFinishCall: Logs the finish call of the requests. LogStartAndFinishCall: Logs the start and finish call of the requests. NoLogCall: Disable request logging.").Default("").Enum("NoLogCall", "LogFinishCall", "LogStartAndFinishCall", "") - queryTimeout := extkingpin.ModelDuration(cmd.Flag("query.timeout", "Maximum time to process query by query node."). Default("2m")) @@ -245,12 +243,12 @@ func registerQuery(app *extkingpin.App) { } } - httpLogOpts, err := logging.ParseHTTPOptions(*reqLogDecision, reqLogConfig) + httpLogOpts, err := logging.ParseHTTPOptions(reqLogConfig) if err != nil { return errors.Wrap(err, "error while parsing config for request logging") } - tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions(*reqLogDecision, reqLogConfig) + tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions(reqLogConfig) if err != nil { return errors.Wrap(err, "error while parsing config for request logging") } diff --git a/cmd/thanos/query_frontend.go b/cmd/thanos/query_frontend.go index f3d2b1a9f7..82c06c4b56 100644 --- a/cmd/thanos/query_frontend.go +++ b/cmd/thanos/query_frontend.go @@ -148,11 +148,10 @@ func registerQueryFrontend(app *extkingpin.App) { cmd.Flag("query-frontend.vertical-shards", "Number of shards to use when distributing shardable PromQL queries. For more details, you can refer to the Vertical query sharding proposal: https://thanos.io/tip/proposals-accepted/202205-vertical-query-sharding.md").IntVar(&cfg.NumShards) - cmd.Flag("log.request.decision", "Deprecation Warning - This flag would be soon deprecated, and replaced with `request.logging-config`. Request Logging for logging the start and end of requests. By default this flag is disabled. LogFinishCall : Logs the finish call of the requests. LogStartAndFinishCall : Logs the start and finish call of the requests. NoLogCall : Disable request logging.").Default("").EnumVar(&cfg.RequestLoggingDecision, "NoLogCall", "LogFinishCall", "LogStartAndFinishCall", "") reqLogConfig := extkingpin.RegisterRequestLoggingFlags(cmd) cmd.Setup(func(g *run.Group, logger log.Logger, reg *prometheus.Registry, tracer opentracing.Tracer, _ <-chan struct{}, _ bool) error { - httpLogOpts, err := logging.ParseHTTPOptions(cfg.RequestLoggingDecision, reqLogConfig) + httpLogOpts, err := logging.ParseHTTPOptions(reqLogConfig) if err != nil { return errors.Wrap(err, "error while parsing config for request logging") } diff --git a/cmd/thanos/receive.go b/cmd/thanos/receive.go index 544fb618a6..8d37f42c23 100644 --- a/cmd/thanos/receive.go +++ b/cmd/thanos/receive.go @@ -74,7 +74,7 @@ func registerReceive(app *extkingpin.App) { return errors.New("no external labels configured for receive, uniquely identifying external labels must be configured (ideally with `receive_` prefix); see https://thanos.io/tip/thanos/storage.md#external-labels for details.") } - tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions("", conf.reqLogConfig) + tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions(conf.reqLogConfig) if err != nil { return errors.Wrap(err, "error while parsing config for request logging") } diff --git a/cmd/thanos/rule.go b/cmd/thanos/rule.go index 7d3840a9f9..63f32a6bea 100644 --- a/cmd/thanos/rule.go +++ b/cmd/thanos/rule.go @@ -144,8 +144,6 @@ func registerRule(app *extkingpin.App) { conf.rwConfig = extflag.RegisterPathOrContent(cmd, "remote-write.config", "YAML config for the remote-write configurations, that specify servers where samples should be sent to (see https://prometheus.io/docs/prometheus/latest/configuration/configuration/#remote_write). This automatically enables stateless mode for ruler and no series will be stored in the ruler's TSDB. If an empty config (or file) is provided, the flag is ignored and ruler is run with its own TSDB.", extflag.WithEnvSubstitution()) - reqLogDecision := cmd.Flag("log.request.decision", "Deprecation Warning - This flag would be soon deprecated, and replaced with `request.logging-config`. Request Logging for logging the start and end of requests. By default this flag is disabled. LogFinishCall: Logs the finish call of the requests. LogStartAndFinishCall: Logs the start and finish call of the requests. NoLogCall: Disable request logging.").Default("").Enum("NoLogCall", "LogFinishCall", "LogStartAndFinishCall", "") - conf.objStoreConfig = extkingpin.RegisterCommonObjStoreFlags(cmd, "", false) reqLogConfig := extkingpin.RegisterRequestLoggingFlags(cmd) @@ -210,12 +208,12 @@ func registerRule(app *extkingpin.App) { return err } - httpLogOpts, err := logging.ParseHTTPOptions(*reqLogDecision, reqLogConfig) + httpLogOpts, err := logging.ParseHTTPOptions(reqLogConfig) if err != nil { return errors.Wrap(err, "error while parsing config for request logging") } - tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions(*reqLogDecision, reqLogConfig) + tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions(reqLogConfig) if err != nil { return errors.Wrap(err, "error while parsing config for request logging") } diff --git a/cmd/thanos/sidecar.go b/cmd/thanos/sidecar.go index 21147e7532..968a09ee9c 100644 --- a/cmd/thanos/sidecar.go +++ b/cmd/thanos/sidecar.go @@ -58,7 +58,7 @@ func registerSidecar(app *extkingpin.App) { conf := &sidecarConfig{} conf.registerFlag(cmd) cmd.Setup(func(g *run.Group, logger log.Logger, reg *prometheus.Registry, tracer opentracing.Tracer, _ <-chan struct{}, _ bool) error { - tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions("", conf.reqLogConfig) + tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions(conf.reqLogConfig) if err != nil { return errors.Wrap(err, "error while parsing config for request logging") } diff --git a/cmd/thanos/store.go b/cmd/thanos/store.go index 62fb14751f..9ddfbd89b7 100644 --- a/cmd/thanos/store.go +++ b/cmd/thanos/store.go @@ -211,12 +211,12 @@ func registerStore(app *extkingpin.App) { conf.filterConf.MinTime, conf.filterConf.MaxTime) } - httpLogOpts, err := logging.ParseHTTPOptions("", conf.reqLogConfig) + httpLogOpts, err := logging.ParseHTTPOptions(conf.reqLogConfig) if err != nil { return errors.Wrap(err, "error while parsing config for request logging") } - tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions("", conf.reqLogConfig) + tagOpts, grpcLogOpts, err := logging.ParsegRPCOptions(conf.reqLogConfig) if err != nil { return errors.Wrap(err, "error while parsing config for request logging") } diff --git a/docs/components/query-frontend.md b/docs/components/query-frontend.md index 20e197d7db..a0a37f816d 100644 --- a/docs/components/query-frontend.md +++ b/docs/components/query-frontend.md @@ -232,15 +232,6 @@ Flags: --log.format=logfmt Log format to use. Possible options: logfmt or json. --log.level=info Log filtering level. - --log.request.decision= Deprecation Warning - This flag would - be soon deprecated, and replaced with - `request.logging-config`. Request Logging - for logging the start and end of requests. - By default this flag is disabled. LogFinishCall - : Logs the finish call of the requests. - LogStartAndFinishCall : Logs the start and - finish call of the requests. NoLogCall : - Disable request logging. --query-frontend.compress-responses Compress HTTP responses. --query-frontend.downstream-tripper-config= diff --git a/docs/components/query.md b/docs/components/query.md index 63ef683451..87733ff02d 100644 --- a/docs/components/query.md +++ b/docs/components/query.md @@ -351,15 +351,6 @@ Flags: --log.format=logfmt Log format to use. Possible options: logfmt or json. --log.level=info Log filtering level. - --log.request.decision= Deprecation Warning - This flag would - be soon deprecated, and replaced with - `request.logging-config`. Request Logging - for logging the start and end of requests. By - default this flag is disabled. LogFinishCall: - Logs the finish call of the requests. - LogStartAndFinishCall: Logs the start and - finish call of the requests. NoLogCall: Disable - request logging. --query.active-query-path="" Directory to log currently active queries in the queries.active file. diff --git a/docs/components/rule.md b/docs/components/rule.md index 93e5e91cfe..25c98463af 100644 --- a/docs/components/rule.md +++ b/docs/components/rule.md @@ -358,15 +358,6 @@ Flags: --log.format=logfmt Log format to use. Possible options: logfmt or json. --log.level=info Log filtering level. - --log.request.decision= Deprecation Warning - This flag would - be soon deprecated, and replaced with - `request.logging-config`. Request Logging - for logging the start and end of requests. By - default this flag is disabled. LogFinishCall: - Logs the finish call of the requests. - LogStartAndFinishCall: Logs the start and - finish call of the requests. NoLogCall: Disable - request logging. --objstore.config= Alternative to 'objstore.config-file' flag (mutually exclusive). Content of diff --git a/pkg/logging/options.go b/pkg/logging/options.go index 76b0f8be7d..85eed878c0 100644 --- a/pkg/logging/options.go +++ b/pkg/logging/options.go @@ -5,7 +5,6 @@ package logging import ( "fmt" - "math/rand" "time" extflag "github.com/efficientgo/tools/extkingpin" @@ -13,7 +12,6 @@ import ( "github.com/go-kit/log/level" grpc_logging "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging" "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/tags" - "github.com/oklog/ulid" "google.golang.org/grpc/codes" ) @@ -161,7 +159,7 @@ var MapAllowedLevels = map[string][]string{ } // TODO: @yashrsharma44 - To be deprecated in the next release. -func ParseHTTPOptions(flagDecision string, reqLogConfig *extflag.PathOrContent) ([]Option, error) { +func ParseHTTPOptions(reqLogConfig *extflag.PathOrContent) ([]Option, error) { // Default Option: No Logging. logOpts := []Option{WithDecider(func(_ string, _ error) Decision { return NoLogCall @@ -173,22 +171,11 @@ func ParseHTTPOptions(flagDecision string, reqLogConfig *extflag.PathOrContent) return logOpts, fmt.Errorf("getting request logging config failed. %v", err) } - // Check if the user enables request logging through flags and YAML. - if len(configYAML) != 0 && flagDecision != "" { - return logOpts, fmt.Errorf("both log.request.decision and request.logging have been enabled, please use only one of the flags") - } - // If old flag is enabled. - if len(flagDecision) > 0 { - logOpts := []Option{WithDecider(func(_ string, _ error) Decision { - return LogDecision[flagDecision] - })} - return logOpts, nil - } return NewHTTPOption(configYAML) } // TODO: @yashrsharma44 - To be deprecated in the next release. -func ParsegRPCOptions(flagDecision string, reqLogConfig *extflag.PathOrContent) ([]tags.Option, []grpc_logging.Option, error) { +func ParsegRPCOptions(reqLogConfig *extflag.PathOrContent) ([]tags.Option, []grpc_logging.Option, error) { // Default Option: No Logging. logOpts := []grpc_logging.Option{grpc_logging.WithDecider(func(_ string, _ error) grpc_logging.Decision { return grpc_logging.NoLogCall @@ -199,47 +186,10 @@ func ParsegRPCOptions(flagDecision string, reqLogConfig *extflag.PathOrContent) return []tags.Option{}, logOpts, fmt.Errorf("getting request logging config failed. %v", err) } - // Check if the user enables request logging through flags and YAML. - if len(configYAML) != 0 && flagDecision != "" { - return []tags.Option{}, logOpts, fmt.Errorf("both log.request.decision and request.logging-config have been enabled, please use only one of the flags") - } - - // If the old flag is empty, use the new YAML config. - if flagDecision == "" { - tagOpts, logOpts, err := NewGRPCOption(configYAML) - if err != nil { - return []tags.Option{}, logOpts, err - } - return tagOpts, logOpts, nil - } - - tagOpts := []tags.Option{ - tags.WithFieldExtractor(func(_ string, req interface{}) map[string]string { - tagMap := tags.TagBasedRequestFieldExtractor("request-id")("", req) - // If a request-id exists for a given request. - if tagMap != nil { - if _, ok := tagMap["request-id"]; ok { - return tagMap - } - } - entropy := ulid.Monotonic(rand.New(rand.NewSource(time.Now().UnixNano())), 0) - reqID := ulid.MustNew(ulid.Timestamp(time.Now()), entropy) - tagMap = make(map[string]string) - tagMap["request-id"] = reqID.String() - return tagMap - }), + tagOpts, logOpts, err := NewGRPCOption(configYAML) + if err != nil { + return []tags.Option{}, logOpts, err } - logOpts = []grpc_logging.Option{grpc_logging.WithDecider(func(_ string, _ error) grpc_logging.Decision { - switch flagDecision { - case "NoLogCall": - return grpc_logging.NoLogCall - case "LogFinishCall": - return grpc_logging.LogFinishCall - case "LogStartAndFinishCall": - return grpc_logging.LogStartAndFinishCall - default: - return grpc_logging.NoLogCall - } - })} return tagOpts, logOpts, nil + } From 1c73ab427fdb9912964efef25dcf6abf1b1f5d8c Mon Sep 17 00:00:00 2001 From: Coleen Iona Quadros Date: Thu, 31 Aug 2023 16:12:53 +0200 Subject: [PATCH 2/2] add changelog Signed-off-by: Coleen Iona Quadros --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27bf8d5ff8..8681d96cd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re ### Removed +- [#6686](https://github.com/thanos-io/thanos/pull/6686) Remove deprecated `--log.request.decision` flag. We now use `--request.logging-config` to set logging decisions. + ## [v0.32.1](https://github.com/thanos-io/thanos/tree/release-0.32) - 28.08.2023 ### Fixed