From aad53a4d80e95348f29b94c02086cc8e65504467 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Mon, 8 Mar 2021 17:57:15 -0500 Subject: [PATCH 1/3] [coordinator] Apply auto-mapping rules if-and-only-if no drop policies are in effect --- .../downsample/metrics_appender.go | 166 +++++++++--------- src/metrics/metadata/metadata.go | 29 ++- 2 files changed, 115 insertions(+), 80 deletions(-) diff --git a/src/cmd/services/m3coordinator/downsample/metrics_appender.go b/src/cmd/services/m3coordinator/downsample/metrics_appender.go index 8c38f5d86d..d37a23f5b2 100644 --- a/src/cmd/services/m3coordinator/downsample/metrics_appender.go +++ b/src/cmd/services/m3coordinator/downsample/metrics_appender.go @@ -209,14 +209,11 @@ func (a *metricsAppender) SamplesAppender(opts SampleAppenderOptions) (SamplesAp tags.filterPrefix(filter) } - var ( - dropApplyResult metadata.ApplyOrRemoveDropPoliciesResult - dropTimestamp bool - ) - if opts.Override { - // Reuse a slice to keep the current staged metadatas we will apply. - a.curr.Pipelines = a.curr.Pipelines[:0] + // Reuse a slice to keep the current staged metadatas we will apply. + a.curr.Pipelines = a.curr.Pipelines[:0] + if opts.Override { + // Process an override explicitly provided as part of request. for _, rule := range opts.OverrideRules.MappingRules { stagedMetadatas, err := rule.StagedMetadatas() if err != nil { @@ -234,56 +231,68 @@ func (a *metricsAppender) SamplesAppender(opts SampleAppenderOptions) (SamplesAp if err := a.addSamplesAppenders(tags, a.curr); err != nil { return SamplesAppenderResult{}, err } - } else { - // Reuse a slice to keep the current staged metadatas we will apply. - a.curr.Pipelines = a.curr.Pipelines[:0] - - // NB(r): First apply mapping rules to see which storage policies - // have been applied, any that have been applied as part of - // mapping rules that exact match a default storage policy will be - // skipped when applying default rules, so as to avoid storing - // the same metrics in the same namespace with the same metric - // name and tags (i.e. overwriting each other). - a.mappingRuleStoragePolicies = a.mappingRuleStoragePolicies[:0] - - ruleStagedMetadatas := matchResult.ForExistingIDAt(nowNanos) - if !ruleStagedMetadatas.IsDefault() && len(ruleStagedMetadatas) != 0 { - a.debugLogMatch("downsampler applying matched rule", - debugLogMatchOptions{Meta: ruleStagedMetadatas}) - - // Collect storage policies for all the current active mapping rules. - // TODO: we should convert this to iterate over pointers - // nolint:gocritic - for _, stagedMetadata := range ruleStagedMetadatas { - for _, pipe := range stagedMetadata.Pipelines { - // Skip rollup rules unless configured otherwise. - // We only want to consider mapping rules here, - // 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 pipe.IsMappingRule() { - for _, sp := range pipe.StoragePolicies { - a.mappingRuleStoragePolicies = - append(a.mappingRuleStoragePolicies, sp) - } - } else { - a.debugLogMatch( - "skipping rollup rule in populating active mapping rule policies", - debugLogMatchOptions{}, - ) + + return SamplesAppenderResult{ + SamplesAppender: a.multiSamplesAppender, + IsDropPolicyApplied: false, + ShouldDropTimestamp: false, + }, nil + } + + // NB(r): First apply mapping rules to see which storage policies + // have been applied, any that have been applied as part of + // mapping rules that exact match a default storage policy will be + // skipped when applying default rules, so as to avoid storing + // the same metrics in the same namespace with the same metric + // name and tags (i.e. overwriting each other). + var ( + ruleStagedMetadatas = matchResult.ForExistingIDAt(nowNanos) + dropApplyResult metadata.ApplyOrRemoveDropPoliciesResult + dropTimestamp bool + ) + a.mappingRuleStoragePolicies = a.mappingRuleStoragePolicies[:0] + if !ruleStagedMetadatas.IsDefault() && len(ruleStagedMetadatas) != 0 { + a.debugLogMatch("downsampler applying matched rule", + debugLogMatchOptions{Meta: ruleStagedMetadatas}) + + // Collect storage policies for all the current active mapping rules. + // TODO: we should convert this to iterate over pointers + // nolint:gocritic + for _, stagedMetadata := range ruleStagedMetadatas { + for _, pipe := range stagedMetadata.Pipelines { + // Skip rollup rules unless configured otherwise. + // We only want to consider mapping rules here, + // 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 pipe.IsMappingRule() { + for _, sp := range pipe.StoragePolicies { + a.mappingRuleStoragePolicies = + append(a.mappingRuleStoragePolicies, sp) } + } else { + a.debugLogMatch( + "skipping rollup rule in populating active mapping rule policies", + debugLogMatchOptions{}, + ) } } - - // Only sample if going to actually aggregate - pipelines := ruleStagedMetadatas[len(ruleStagedMetadatas)-1] - a.curr.Pipelines = - append(a.curr.Pipelines, pipelines.Pipelines...) } - // Always aggregate any default staged metadatas (unless - // mapping rule has provided an override for a storage policy, - // if so then skip aggregating for that storage policy). + // Only sample if going to actually aggregate + pipelines := ruleStagedMetadatas[len(ruleStagedMetadatas)-1] + a.curr.Pipelines = append(a.curr.Pipelines, pipelines.Pipelines...) + } + + // Always aggregate any default staged metadatas with a few exceptions. + // Exceptions are: + // 1. A mapping rule has provided an override for a storage policy, + // if so then skip aggregating for that storage policy). + // 2. Any type of drop rule has been applied, since they should only + // impact mapping rules not default staged metadatas provided for + // auto-mapping rules (i.e. default namespace aggregation). + if !a.curr.Pipelines.IsDropPolicySet() { + // No drop rule has been applied as part of rule matching. for idx, stagedMetadatasProto := range a.defaultStagedMetadatasProtos { // NB(r): Need to take copy of default staged metadatas as we // sometimes mutate it. @@ -362,40 +371,39 @@ func (a *metricsAppender) SamplesAppender(opts SampleAppenderOptions) (SamplesAp debugLogMatchOptions{Meta: stagedMetadatas}) pipelines := stagedMetadatas[len(stagedMetadatas)-1] - a.curr.Pipelines = - append(a.curr.Pipelines, pipelines.Pipelines...) + a.curr.Pipelines = append(a.curr.Pipelines, pipelines.Pipelines...) } + } - // Apply the custom tags first so that they apply even if mapping - // rules drop the metric. - dropTimestamp = a.curr.Pipelines.ApplyCustomTags() + // Apply the custom tags first so that they apply even if mapping + // rules drop the metric. + dropTimestamp = a.curr.Pipelines.ApplyCustomTags() - // Apply drop policies results - a.curr.Pipelines, dropApplyResult = a.curr.Pipelines.ApplyOrRemoveDropPolicies() + // Apply drop policies results + a.curr.Pipelines, dropApplyResult = a.curr.Pipelines.ApplyOrRemoveDropPolicies() - if len(a.curr.Pipelines) > 0 && !a.curr.IsDropPolicyApplied() { - // Send to downsampler if we have something in the pipeline. - a.debugLogMatch("downsampler using built mapping staged metadatas", - debugLogMatchOptions{Meta: []metadata.StagedMetadata{a.curr}}) + if len(a.curr.Pipelines) > 0 && !a.curr.IsDropPolicyApplied() { + // Send to downsampler if we have something in the pipeline. + a.debugLogMatch("downsampler using built mapping staged metadatas", + debugLogMatchOptions{Meta: []metadata.StagedMetadata{a.curr}}) - if err := a.addSamplesAppenders(tags, a.curr); err != nil { - return SamplesAppenderResult{}, err - } + if err := a.addSamplesAppenders(tags, a.curr); err != nil { + return SamplesAppenderResult{}, err } + } - numRollups := matchResult.NumNewRollupIDs() - for i := 0; i < numRollups; i++ { - rollup := matchResult.ForNewRollupIDsAt(i, nowNanos) - - a.debugLogMatch("downsampler applying matched rollup rule", - debugLogMatchOptions{Meta: rollup.Metadatas, RollupID: rollup.ID}) - a.multiSamplesAppender.addSamplesAppender(samplesAppender{ - agg: a.agg, - clientRemote: a.clientRemote, - unownedID: rollup.ID, - stagedMetadatas: rollup.Metadatas, - }) - } + numRollups := matchResult.NumNewRollupIDs() + for i := 0; i < numRollups; i++ { + rollup := matchResult.ForNewRollupIDsAt(i, nowNanos) + + a.debugLogMatch("downsampler applying matched rollup rule", + debugLogMatchOptions{Meta: rollup.Metadatas, RollupID: rollup.ID}) + a.multiSamplesAppender.addSamplesAppender(samplesAppender{ + agg: a.agg, + clientRemote: a.clientRemote, + unownedID: rollup.ID, + stagedMetadatas: rollup.Metadatas, + }) } dropPolicyApplied := dropApplyResult != metadata.NoDropPolicyPresentResult diff --git a/src/metrics/metadata/metadata.go b/src/metrics/metadata/metadata.go index 5cf5e225eb..7c10a36d8d 100644 --- a/src/metrics/metadata/metadata.go +++ b/src/metrics/metadata/metadata.go @@ -121,7 +121,12 @@ func (m PipelineMetadata) IsDropPolicyApplied() bool { return m.AggregationID.IsDefault() && m.StoragePolicies.IsDefault() && m.Pipeline.IsEmpty() && - !m.DropPolicy.IsDefault() + m.IsDropPolicySet() +} + +// IsDropPolicySet returns whether a drop policy is set. +func (m PipelineMetadata) IsDropPolicySet() bool { + return !m.DropPolicy.IsDefault() } // Clone clones the pipeline metadata. @@ -200,6 +205,17 @@ func (metadatas PipelineMetadatas) Clone() PipelineMetadatas { return cloned } +// IsDropPolicySet returns whether any drop policies are set (but +// does not discriminate if they have been applied or not). +func (metadatas PipelineMetadatas) IsDropPolicySet() bool { + for i := range metadatas { + if metadatas[i].IsDropPolicySet() { + return true + } + } + return false +} + // ApplyOrRemoveDropPoliciesResult is the result of applying or removing // the drop policies for pipelines. type ApplyOrRemoveDropPoliciesResult uint @@ -297,6 +313,17 @@ func (m Metadata) IsDropPolicyApplied() bool { return len(m.Pipelines) == 1 && m.Pipelines[0].IsDropPolicyApplied() } +// IsDropPolicySet returns whether any drop policies are set (but +// does not discriminate if they have been applied or not). +func (m Metadata) IsDropPolicySet() bool { + for i := range m.Pipelines { + if m.Pipelines[i].IsDropPolicySet() { + return true + } + } + return false +} + // Equal returns true if two metadatas are considered equal. func (m Metadata) Equal(other Metadata) bool { return m.Pipelines.Equal(other.Pipelines) From 8c778b424240f2df353369f5e3640c83529a7f90 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Mon, 8 Mar 2021 19:08:35 -0500 Subject: [PATCH 2/3] Add unit test --- .../downsample/downsampler_test.go | 128 +++++++++++++++++- src/query/generated/mocks/generate.go | 2 +- src/query/storage/m3/cluster.go | 11 ++ src/query/storage/m3/m3_mock.go | 71 +++++++++- 4 files changed, 207 insertions(+), 5 deletions(-) diff --git a/src/cmd/services/m3coordinator/downsample/downsampler_test.go b/src/cmd/services/m3coordinator/downsample/downsampler_test.go index f5fd6a49d0..979ef48cae 100644 --- a/src/cmd/services/m3coordinator/downsample/downsampler_test.go +++ b/src/cmd/services/m3coordinator/downsample/downsampler_test.go @@ -447,6 +447,88 @@ func TestDownsamplerAggregationWithRulesConfigMappingRules(t *testing.T) { testDownsamplerAggregation(t, testDownsampler) } +func TestDownsamplerAggregationWithAutoMappingRulesAndRulesConfigMappingRulesAndDropRule(t *testing.T) { + t.Parallel() + + gaugeMetric := testGaugeMetric{ + tags: map[string]string{ + nameTag: "foo_metric", + "app": "nginx_edge", + "env": "staging", + }, + timedSamples: []testGaugeMetricTimedSample{ + {value: 15}, {value: 10}, {value: 30}, {value: 5}, {value: 0}, + }, + expectDropPolicyApplied: true, + } + testDownsampler := newTestDownsampler(t, testDownsamplerOptions{ + autoMappingRules: []m3.ClusterNamespaceOptions{ + m3.NewClusterNamespaceOptions( + storagemetadata.Attributes{ + MetricsType: storagemetadata.AggregatedMetricsType, + Retention: 2 * time.Hour, + Resolution: 1 * time.Second, + }, + nil, + ), + m3.NewClusterNamespaceOptions( + storagemetadata.Attributes{ + MetricsType: storagemetadata.AggregatedMetricsType, + Retention: 12 * time.Hour, + Resolution: 5 * time.Second, + }, + nil, + ), + }, + rulesConfig: &RulesConfiguration{ + MappingRules: []MappingRuleConfiguration{ + { + Filter: "env:staging", + Drop: true, + }, + { + Filter: "app:nginx*", + Aggregations: []aggregation.Type{aggregation.Max}, + StoragePolicies: []StoragePolicyConfiguration{ + { + Resolution: 10 * time.Second, + Retention: 30 * 24 * time.Hour, + }, + }, + }, + }, + }, + ingest: &testDownsamplerOptionsIngest{ + gaugeMetrics: []testGaugeMetric{gaugeMetric}, + }, + expect: &testDownsamplerOptionsExpect{ + allowFilter: &testDownsamplerOptionsExpectAllowFilter{ + attributes: []storagemetadata.Attributes{ + { + MetricsType: storagemetadata.AggregatedMetricsType, + Resolution: 10 * time.Second, + Retention: 30 * 24 * time.Hour, + }, + }, + }, + writes: []testExpectedWrite{ + { + tags: gaugeMetric.tags, + values: []expectedValue{{value: 30}}, + attributes: &storagemetadata.Attributes{ + MetricsType: storagemetadata.AggregatedMetricsType, + Resolution: 10 * time.Second, + Retention: 30 * 24 * time.Hour, + }, + }, + }, + }, + }) + + // Test expected output + testDownsamplerAggregation(t, testDownsampler) +} + func TestDownsamplerAggregationWithRulesConfigMappingRulesPartialReplaceAutoMappingRuleFromNamespacesWatcher(t *testing.T) { t.Parallel() @@ -2270,12 +2352,14 @@ func testDownsamplerAggregation( expectedWrites := append(counterMetricsExpect, gaugeMetricsExpect...) // Allow overrides + var allowFilter *testDownsamplerOptionsExpectAllowFilter if ingest := testOpts.ingest; ingest != nil { counterMetrics = ingest.counterMetrics gaugeMetrics = ingest.gaugeMetrics } if expect := testOpts.expect; expect != nil { expectedWrites = expect.writes + allowFilter = expect.allowFilter } // Ingest points @@ -2393,6 +2477,24 @@ CheckAllWritesArrivedLoop: } } } + + if allowFilter == nil { + return // No allow filter checking required. + } + + for _, write := range testDownsampler.storage.Writes() { + attrs := write.Attributes() + foundMatchingAttribute := false + for _, allowed := range allowFilter.attributes { + if allowed == attrs { + foundMatchingAttribute = true + break + } + } + assert.True(t, foundMatchingAttribute, + fmt.Sprintf("attribute not allowed: allowed=%v, actual=%v", + allowFilter.attributes, attrs)) + } } func testDownsamplerRemoteAggregation( @@ -2618,7 +2720,7 @@ type testDownsamplerOptions struct { identTag string // Options for the test - autoMappingRules []AutoMappingRule + autoMappingRules []m3.ClusterNamespaceOptions sampleAppenderOpts *SampleAppenderOptions remoteClientMock *client.MockClient rulesConfig *RulesConfiguration @@ -2635,7 +2737,12 @@ type testDownsamplerOptionsIngest struct { } type testDownsamplerOptionsExpect struct { - writes []testExpectedWrite + writes []testExpectedWrite + allowFilter *testDownsamplerOptionsExpectAllowFilter +} + +type testDownsamplerOptionsExpectAllowFilter struct { + attributes []storagemetadata.Attributes } func newTestDownsampler(t *testing.T, opts testDownsamplerOptions) testDownsampler { @@ -2719,6 +2826,23 @@ func newTestDownsampler(t *testing.T, opts testDownsamplerOptions) testDownsampl }) require.NoError(t, err) + if len(opts.autoMappingRules) > 0 { + // Simulate the automapping rules being injected into the downsampler. + ctrl := gomock.NewController(t) + + var mockNamespaces m3.ClusterNamespaces + for _, r := range opts.autoMappingRules { + n := m3.NewMockClusterNamespace(ctrl) + n.EXPECT(). + Options(). + Return(r). + AnyTimes() + mockNamespaces = append(mockNamespaces, n) + } + + instance.(*downsampler).OnUpdate(mockNamespaces) + } + downcast, ok := instance.(*downsampler) require.True(t, ok) diff --git a/src/query/generated/mocks/generate.go b/src/query/generated/mocks/generate.go index 199b0eac0b..607d57c1e1 100644 --- a/src/query/generated/mocks/generate.go +++ b/src/query/generated/mocks/generate.go @@ -21,7 +21,7 @@ // mockgen rules for generating mocks for exported interfaces (reflection mode). //go:generate sh -c "mockgen -package=downsample $PACKAGE/src/cmd/services/m3coordinator/downsample Downsampler,MetricsAppender,SamplesAppender | genclean -pkg $PACKAGE/src/cmd/services/m3coordinator/downsample -out $GOPATH/src/$PACKAGE/src/cmd/services/m3coordinator/downsample/downsample_mock.go" //go:generate sh -c "mockgen -package=storage -destination=$GOPATH/src/$PACKAGE/src/query/storage/storage_mock.go $PACKAGE/src/query/storage Storage" -//go:generate sh -c "mockgen -package=m3 -destination=$GOPATH/src/$PACKAGE/src/query/storage/m3/m3_mock.go $PACKAGE/src/query/storage/m3 Storage" +//go:generate sh -c "mockgen -package=m3 -destination=$GOPATH/src/$PACKAGE/src/query/storage/m3/m3_mock.go $PACKAGE/src/query/storage/m3 Storage,ClusterNamespace" //go:generate sh -c "mockgen -package=ts -destination=$GOPATH/src/$PACKAGE/src/query/ts/ts_mock.go $PACKAGE/src/query/ts Values" //go:generate sh -c "mockgen -package=block -destination=$GOPATH/src/$PACKAGE/src/query/block/block_mock.go $PACKAGE/src/query/block Block,StepIter,Builder,Step,SeriesIter" //go:generate sh -c "mockgen -package=ingest -destination=$GOPATH/src/$PACKAGE/src/cmd/services/m3coordinator/ingest/write_mock.go $PACKAGE/src/cmd/services/m3coordinator/ingest DownsamplerAndWriter" diff --git a/src/query/storage/m3/cluster.go b/src/query/storage/m3/cluster.go index 7818188926..2a71f78d49 100644 --- a/src/query/storage/m3/cluster.go +++ b/src/query/storage/m3/cluster.go @@ -100,6 +100,17 @@ type ClusterNamespaceOptions struct { downsample *ClusterNamespaceDownsampleOptions } +// NewClusterNamespaceOptions creates new cluster namespace options. +func NewClusterNamespaceOptions( + attributes storagemetadata.Attributes, + downsample *ClusterNamespaceDownsampleOptions, +) ClusterNamespaceOptions { + return ClusterNamespaceOptions{ + attributes: attributes, + downsample: downsample, + } +} + // Attributes returns the storage attributes of the cluster namespace. func (o ClusterNamespaceOptions) Attributes() storagemetadata.Attributes { return o.attributes diff --git a/src/query/storage/m3/m3_mock.go b/src/query/storage/m3/m3_mock.go index 6ef74722b0..8030d39120 100644 --- a/src/query/storage/m3/m3_mock.go +++ b/src/query/storage/m3/m3_mock.go @@ -1,7 +1,7 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/m3db/m3/src/query/storage/m3 (interfaces: Storage) +// Source: github.com/m3db/m3/src/query/storage/m3 (interfaces: Storage,ClusterNamespace) -// Copyright (c) 2020 Uber Technologies, Inc. +// Copyright (c) 2021 Uber Technologies, Inc. // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -28,9 +28,11 @@ import ( "context" "reflect" + "github.com/m3db/m3/src/dbnode/client" "github.com/m3db/m3/src/query/block" "github.com/m3db/m3/src/query/storage" "github.com/m3db/m3/src/query/storage/m3/consolidators" + "github.com/m3db/m3/src/x/ident" "github.com/golang/mock/gomock" ) @@ -234,3 +236,68 @@ func (mr *MockStorageMockRecorder) Write(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Write", reflect.TypeOf((*MockStorage)(nil).Write), arg0, arg1) } + +// MockClusterNamespace is a mock of ClusterNamespace interface +type MockClusterNamespace struct { + ctrl *gomock.Controller + recorder *MockClusterNamespaceMockRecorder +} + +// MockClusterNamespaceMockRecorder is the mock recorder for MockClusterNamespace +type MockClusterNamespaceMockRecorder struct { + mock *MockClusterNamespace +} + +// NewMockClusterNamespace creates a new mock instance +func NewMockClusterNamespace(ctrl *gomock.Controller) *MockClusterNamespace { + mock := &MockClusterNamespace{ctrl: ctrl} + mock.recorder = &MockClusterNamespaceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockClusterNamespace) EXPECT() *MockClusterNamespaceMockRecorder { + return m.recorder +} + +// NamespaceID mocks base method +func (m *MockClusterNamespace) NamespaceID() ident.ID { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NamespaceID") + ret0, _ := ret[0].(ident.ID) + return ret0 +} + +// NamespaceID indicates an expected call of NamespaceID +func (mr *MockClusterNamespaceMockRecorder) NamespaceID() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NamespaceID", reflect.TypeOf((*MockClusterNamespace)(nil).NamespaceID)) +} + +// Options mocks base method +func (m *MockClusterNamespace) Options() ClusterNamespaceOptions { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Options") + ret0, _ := ret[0].(ClusterNamespaceOptions) + return ret0 +} + +// Options indicates an expected call of Options +func (mr *MockClusterNamespaceMockRecorder) Options() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Options", reflect.TypeOf((*MockClusterNamespace)(nil).Options)) +} + +// Session mocks base method +func (m *MockClusterNamespace) Session() client.Session { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Session") + ret0, _ := ret[0].(client.Session) + return ret0 +} + +// Session indicates an expected call of Session +func (mr *MockClusterNamespaceMockRecorder) Session() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Session", reflect.TypeOf((*MockClusterNamespace)(nil).Session)) +} From b2c60c88c9550daf2ad7eab03575b09db6269da2 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Mon, 8 Mar 2021 19:09:52 -0500 Subject: [PATCH 3/3] Update comments --- .../services/m3coordinator/downsample/metrics_appender.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cmd/services/m3coordinator/downsample/metrics_appender.go b/src/cmd/services/m3coordinator/downsample/metrics_appender.go index d37a23f5b2..428272e3ab 100644 --- a/src/cmd/services/m3coordinator/downsample/metrics_appender.go +++ b/src/cmd/services/m3coordinator/downsample/metrics_appender.go @@ -288,11 +288,11 @@ func (a *metricsAppender) SamplesAppender(opts SampleAppenderOptions) (SamplesAp // Exceptions are: // 1. A mapping rule has provided an override for a storage policy, // if so then skip aggregating for that storage policy). - // 2. Any type of drop rule has been applied, since they should only - // impact mapping rules not default staged metadatas provided for + // 2. Any type of drop rule has been set, since they should only + // impact mapping rules, not default staged metadatas provided from // auto-mapping rules (i.e. default namespace aggregation). if !a.curr.Pipelines.IsDropPolicySet() { - // No drop rule has been applied as part of rule matching. + // No drop rule has been set as part of rule matching. for idx, stagedMetadatasProto := range a.defaultStagedMetadatasProtos { // NB(r): Need to take copy of default staged metadatas as we // sometimes mutate it.