remove deprecated log.request.decision flag (#6686)

* remove deprecated log.request.decision flag Signed-off-by: Coleen Iona Quadros <[email protected]> * add changelog Signed-off-by: Coleen Iona Quadros <[email protected]> --------- Signed-off-by: Coleen Iona Quadros <[email protected]>
thanos-io · Aug 31, 2023 · f75e44a · f75e44a
1 parent 7b20b1d
commit f75e44a
Show file tree

Hide file tree

Showing 11 changed files with 17 additions and 97 deletions.
diff --git 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

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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=<content>

diff --git 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
@@ -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=<content>
                                  Alternative to 'objstore.config-file'
                                  flag (mutually exclusive). Content of

diff --git a/pkg/logging/options.go b/pkg/logging/options.go
@@ -5,15 +5,13 @@ package logging
 
 import (
 	"fmt"
-	"math/rand"
 	"time"
 
 	extflag "github.com/efficientgo/tools/extkingpin"
 	"github.com/go-kit/log"
 	"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
+
 }