[coordinator] Remove includeRollupsOnDefaultRuleFiltering flag (#3073)

Confirmed this is working as expected, and is not useful to configure otherwise. Co-authored-by: Rob Skillington <[email protected]>
m3db · Jan 9, 2021 · 4bad816 · 4bad816
1 parent cfa652b
commit 4bad816
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 40 deletions.
diff --git a/src/cmd/services/m3coordinator/downsample/downsampler.go b/src/cmd/services/m3coordinator/downsample/downsampler.go
@@ -127,16 +127,15 @@ func defaultMetricsAppenderOptions(opts DownsamplerOptions, agg agg) metricsAppe
 	}
 
 	return metricsAppenderOptions{
-		agg:                                  agg.aggregator,
-		clientRemote:                         agg.clientRemote,
-		clockOpts:                            agg.clockOpts,
-		tagEncoderPool:                       agg.pools.tagEncoderPool,
-		matcher:                              agg.matcher,
-		metricTagsIteratorPool:               agg.pools.metricTagsIteratorPool,
-		debugLogging:                         debugLogging,
-		logger:                               logger,
-		augmentM3Tags:                        agg.augmentM3Tags,
-		includeRollupsOnDefaultRuleFiltering: agg.includeRollupsOnDefaultRuleFiltering,
+		agg:                    agg.aggregator,
+		clientRemote:           agg.clientRemote,
+		clockOpts:              agg.clockOpts,
+		tagEncoderPool:         agg.pools.tagEncoderPool,
+		matcher:                agg.matcher,
+		metricTagsIteratorPool: agg.pools.metricTagsIteratorPool,
+		debugLogging:           debugLogging,
+		logger:                 logger,
+		augmentM3Tags:          agg.augmentM3Tags,
 	}
 }
 

diff --git a/src/cmd/services/m3coordinator/downsample/metrics_appender.go b/src/cmd/services/m3coordinator/downsample/metrics_appender.go
@@ -98,12 +98,11 @@ type metricsAppenderOptions struct {
 	agg          aggregator.Aggregator
 	clientRemote client.Client
 
-	defaultStagedMetadatasProtos         []metricpb.StagedMetadatas
-	matcher                              matcher.Matcher
-	tagEncoderPool                       serialize.TagEncoderPool
-	metricTagsIteratorPool               serialize.MetricTagsIteratorPool
-	augmentM3Tags                        bool
-	includeRollupsOnDefaultRuleFiltering bool
+	defaultStagedMetadatasProtos []metricpb.StagedMetadatas
+	matcher                      matcher.Matcher
+	tagEncoderPool               serialize.TagEncoderPool
+	metricTagsIteratorPool       serialize.MetricTagsIteratorPool
+	augmentM3Tags                bool
 
 	clockOpts    clock.Options
 	debugLogging bool
@@ -246,7 +245,7 @@ func (a *metricsAppender) SamplesAppender(opts SampleAppenderOptions) (SamplesAp
 					// as we still want to apply default mapping rules to
 					// metrics that are rolled up to ensure the underlying metric
 					// gets written to aggregated namespaces.
-					if a.includeRollupsOnDefaultRuleFiltering || pipe.IsMappingRule() {
+					if pipe.IsMappingRule() {
 						for _, sp := range pipe.StoragePolicies {
 							a.mappingRuleStoragePolicies =
 								append(a.mappingRuleStoragePolicies, sp)

diff --git a/src/cmd/services/m3coordinator/downsample/options.go b/src/cmd/services/m3coordinator/downsample/options.go
@@ -225,11 +225,10 @@ type agg struct {
 	aggregator   aggregator.Aggregator
 	clientRemote client.Client
 
-	clockOpts                            clock.Options
-	matcher                              matcher.Matcher
-	pools                                aggPools
-	augmentM3Tags                        bool
-	includeRollupsOnDefaultRuleFiltering bool
+	clockOpts     clock.Options
+	matcher       matcher.Matcher
+	pools         aggPools
+	augmentM3Tags bool
 }
 
 // Configuration configurates a downsampler.
@@ -271,14 +270,6 @@ type Configuration struct {
 	// Furthermore, the option is automatically enabled if static rules are
 	// used and any filter contain an __m3_type__ tag.
 	AugmentM3Tags bool `yaml:"augmentM3Tags"`
-
-	// IncludeRollupsOnDefaultRuleFiltering will include rollup rules
-	// when deciding if the downsampler should ignore the default auto mapping rules
-	// based on the storage policies applied on a given rule.
-	// This is usually not what you want to do, as it means the raw metric
-	// that is being rolled up by your rule will not be forward into aggregated namespaces,
-	// and will only be written to the unaggregated namespace.
-	IncludeRollupsOnDefaultRuleFiltering bool `yaml:"includeRollupsOnDefaultRuleFiltering"`
 }
 
 // MatcherConfiguration is the configuration for the rule matcher.
@@ -797,11 +788,10 @@ func (cfg Configuration) newAggregator(o DownsamplerOptions) (agg, error) {
 		}
 
 		return agg{
-			clientRemote:                         client,
-			matcher:                              matcher,
-			pools:                                pools,
-			augmentM3Tags:                        augmentM3Tags,
-			includeRollupsOnDefaultRuleFiltering: cfg.IncludeRollupsOnDefaultRuleFiltering,
+			clientRemote:  client,
+			matcher:       matcher,
+			pools:         pools,
+			augmentM3Tags: augmentM3Tags,
 		}, nil
 	}
 
@@ -963,11 +953,10 @@ func (cfg Configuration) newAggregator(o DownsamplerOptions) (agg, error) {
 	}
 
 	return agg{
-		aggregator:                           aggregatorInstance,
-		matcher:                              matcher,
-		pools:                                pools,
-		augmentM3Tags:                        augmentM3Tags,
-		includeRollupsOnDefaultRuleFiltering: cfg.IncludeRollupsOnDefaultRuleFiltering,
+		aggregator:    aggregatorInstance,
+		matcher:       matcher,
+		pools:         pools,
+		augmentM3Tags: augmentM3Tags,
 	}, nil
 }