From bd5dcc1b2b3d2dccb4479e2354a4fcad292b2fdd Mon Sep 17 00:00:00 2001 From: Bernard Tolosa Date: Thu, 17 Sep 2020 09:57:49 +0800 Subject: [PATCH 1/3] Remove deprecated es.max-num-spans Signed-off-by: Bernard Tolosa --- plugin/storage/es/options.go | 13 ------------- plugin/storage/es/options_test.go | 4 ---- 2 files changed, 17 deletions(-) diff --git a/plugin/storage/es/options.go b/plugin/storage/es/options.go index a18f3d0e4d4..dd6ae6ba8e0 100644 --- a/plugin/storage/es/options.go +++ b/plugin/storage/es/options.go @@ -18,7 +18,6 @@ package es import ( "flag" "fmt" - "math" "strings" "time" @@ -37,7 +36,6 @@ const ( suffixTokenPath = ".token-file" suffixServerURLs = ".server-urls" suffixMaxSpanAge = ".max-span-age" - suffixMaxNumSpans = ".max-num-spans" // deprecated suffixNumShards = ".num-shards" suffixNumReplicas = ".num-replicas" suffixBulkSize = ".bulk.size" @@ -180,12 +178,6 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) { nsConfig.namespace+suffixMaxSpanAge, nsConfig.MaxSpanAge, "The maximum lookback for spans in Elasticsearch") - flagSet.Int( - nsConfig.namespace+suffixMaxNumSpans, - nsConfig.MaxDocCount, - "(deprecated, will be removed in release v1.21.0. Please use "+nsConfig.namespace+".max-doc-count). "+ - "The maximum number of spans to fetch at a time per query in Elasticsearch. "+ - "The lesser of "+nsConfig.namespace+".max-num-spans and "+nsConfig.namespace+".max-doc-count will be used if both are set.") flagSet.Int64( nsConfig.namespace+suffixNumShards, nsConfig.NumShards, @@ -301,11 +293,6 @@ func initFromViper(cfg *namespaceConfig, v *viper.Viper) { cfg.MaxDocCount = v.GetInt(cfg.namespace + suffixMaxDocCount) - if v.IsSet(cfg.namespace + suffixMaxNumSpans) { - maxNumSpans := v.GetInt(cfg.namespace + suffixMaxNumSpans) - cfg.MaxDocCount = int(math.Min(float64(maxNumSpans), float64(cfg.MaxDocCount))) - } - // TODO: Need to figure out a better way for do this. cfg.AllowTokenFromContext = v.GetBool(spanstore.StoragePropagationKey) cfg.TLS = cfg.getTLSFlagsConfig().InitFromViper(v) diff --git a/plugin/storage/es/options_test.go b/plugin/storage/es/options_test.go index 73bb1476e93..b2903417f1b 100644 --- a/plugin/storage/es/options_test.go +++ b/plugin/storage/es/options_test.go @@ -134,11 +134,7 @@ func TestMaxDocCount(t *testing.T) { wantMaxDocCount int }{ {"neither defined", []string{}, 10_000}, - {"max-num-spans only", []string{"--es.max-num-spans=1000"}, 1000}, {"max-doc-count only", []string{"--es.max-doc-count=1000"}, 1000}, - {"max-num-spans == max-doc-count", []string{"--es.max-num-spans=1000", "--es.max-doc-count=1000"}, 1000}, - {"max-num-spans < max-doc-count", []string{"--es.max-num-spans=999", "--es.max-doc-count=1000"}, 999}, - {"max-num-spans > max-doc-count", []string{"--es.max-num-spans=1000", "--es.max-doc-count=999"}, 999}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { From d0b060b8b549590b94ffd2c2ad103346fc68f028 Mon Sep 17 00:00:00 2001 From: Bernardo Tolosa Date: Sun, 31 Jan 2021 19:01:22 +0800 Subject: [PATCH 2/3] Removed maxNumSpansUsage test Signed-off-by: Bernardo Tolosa --- plugin/storage/es/options_test.go | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/plugin/storage/es/options_test.go b/plugin/storage/es/options_test.go index b2903417f1b..3c59d7ccdd8 100644 --- a/plugin/storage/es/options_test.go +++ b/plugin/storage/es/options_test.go @@ -100,33 +100,6 @@ func TestOptionsWithFlags(t *testing.T) { assert.Equal(t, "2006.01.02", aux.IndexDateLayout) } -func TestMaxNumSpansUsage(t *testing.T) { - testCases := []struct { - namespace string - wantUsage string - }{ - { - namespace: "es", - wantUsage: "(deprecated, will be removed in release v1.21.0. Please use es.max-doc-count). " + - "The maximum number of spans to fetch at a time per query in Elasticsearch. " + - "The lesser of es.max-num-spans and es.max-doc-count will be used if both are set.", - }, - { - namespace: "es-archive", - wantUsage: "(deprecated, will be removed in release v1.21.0. Please use es-archive.max-doc-count). " + - "The maximum number of spans to fetch at a time per query in Elasticsearch. " + - "The lesser of es-archive.max-num-spans and es-archive.max-doc-count will be used if both are set.", - }, - } - for _, tc := range testCases { - t.Run(tc.namespace, func(t *testing.T) { - opts := NewOptions(tc.namespace) - _, command := config.Viperize(opts.AddFlags) - assert.Equal(t, tc.wantUsage, command.Flag(tc.namespace+".max-num-spans").Usage) - }) - } -} - func TestMaxDocCount(t *testing.T) { testCases := []struct { name string From 59e3e6a60d3aa9c5d8f04190734b6dd8d9aff2dd Mon Sep 17 00:00:00 2001 From: Bernardo Tolosa Date: Mon, 1 Feb 2021 09:27:47 +0800 Subject: [PATCH 3/3] Add breaking change entry to CHANGELOG Signed-off-by: Bernardo Tolosa --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b13d3b01ca..5523b30e73e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Changes by Version ### Backend Changes #### Breaking changes +* Remove deprecated flag `--es.max-num-spans`, please use `--es.max-doc-count` ([#2482](https://github.com/jaegertracing/jaeger/pull/2482),[@BernardTolosajr](https://github.com/BernardTolosajr)) #### New Features