From b6908e734d6f4a93dcffd170e372b82d2329cf96 Mon Sep 17 00:00:00 2001 From: Wesley Kim Date: Fri, 8 Jan 2021 20:41:01 -0500 Subject: [PATCH] [coordinator] Remove includeRollupsOnDefaultRuleFiltering flag Confirmed this is working as expected, and is not useful to configure otherwise. --- .../m3coordinator/downsample/downsampler.go | 19 +++++----- .../downsample/metrics_appender.go | 13 ++++--- .../m3coordinator/downsample/options.go | 35 +++++++------------ 3 files changed, 27 insertions(+), 40 deletions(-) diff --git a/src/cmd/services/m3coordinator/downsample/downsampler.go b/src/cmd/services/m3coordinator/downsample/downsampler.go index 0e5fc69689..9a635758cc 100644 --- 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 index 46e26c0a04..72d295f200 100644 --- 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 index 65e09160f5..07ea13d37f 100644 --- 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 }