From 4b556bd701b3d01b0c2d21206b76f232a93eae57 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Mon, 28 Oct 2019 15:09:26 -0400 Subject: [PATCH 1/4] [coordinator] Remove carbon debug flag and rely on log debug level --- .../m3coordinator/ingest/carbon/ingest.go | 22 +++++++++---------- src/cmd/services/m3query/config/config.go | 1 - src/query/server/server.go | 1 - 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/cmd/services/m3coordinator/ingest/carbon/ingest.go b/src/cmd/services/m3coordinator/ingest/carbon/ingest.go index 3ea41595af..6d414bd2bb 100644 --- a/src/cmd/services/m3coordinator/ingest/carbon/ingest.go +++ b/src/cmd/services/m3coordinator/ingest/carbon/ingest.go @@ -30,6 +30,8 @@ import ( "sync" "time" + "go.uber.org/zap/zapcore" + "github.com/m3db/m3/src/cmd/services/m3coordinator/downsample" "github.com/m3db/m3/src/cmd/services/m3coordinator/ingest" "github.com/m3db/m3/src/cmd/services/m3query/config" @@ -67,7 +69,6 @@ var ( // Options configures the ingester. type Options struct { - Debug bool InstrumentOptions instrument.Options WorkerPool xsync.PooledWorkerPool } @@ -221,10 +222,11 @@ func (i *ingester) write( downsampleAndStoragePolicies.DownsampleMappingRules = rule.mappingRules downsampleAndStoragePolicies.WriteStoragePolicies = rule.storagePolicies - if i.opts.Debug { + debugLog := i.logger.Check(zapcore.DebugLevel, "carbon metric matched by pattern") + if debugLog != nil { i.logger.Info("carbon metric matched by pattern", - zap.String("name", string(resources.name)), - zap.Any("pattern", rule.rule.Pattern), + zap.ByteString("name", resources.name), + zap.String("pattern", rule.rule.Pattern), zap.Any("mappingRules", rule.mappingRules), zap.Any("storagePolicies", rule.storagePolicies)) } @@ -237,10 +239,8 @@ func (i *ingester) write( if len(downsampleAndStoragePolicies.DownsampleMappingRules) == 0 && len(downsampleAndStoragePolicies.WriteStoragePolicies) == 0 { // Nothing to do if none of the policies matched. - if i.opts.Debug { - i.logger.Info("no rules matched carbon metric, skipping", - zap.String("name", string(resources.name))) - } + i.logger.Debug("no rules matched carbon metric, skipping", + zap.ByteString("name", resources.name)) return false } @@ -263,10 +263,8 @@ func (i *ingester) write( return false } - if i.opts.Debug { - i.logger.Info("successfully wrote carbon metric", - zap.String("name", string(resources.name))) - } + i.logger.Debug("successfully wrote carbon metric", + zap.ByteString("name", resources.name)) return true } diff --git a/src/cmd/services/m3query/config/config.go b/src/cmd/services/m3query/config/config.go index 61bfec3b70..b05feeaa16 100644 --- a/src/cmd/services/m3query/config/config.go +++ b/src/cmd/services/m3query/config/config.go @@ -292,7 +292,6 @@ type CarbonConfiguration struct { // CarbonIngesterConfiguration is the configuration struct for carbon ingestion. type CarbonIngesterConfiguration struct { - Debug bool `yaml:"debug"` ListenAddress string `yaml:"listenAddress"` MaxConcurrency int `yaml:"maxConcurrency"` Rules []CarbonIngesterRuleConfiguration `yaml:"rules"` diff --git a/src/query/server/server.go b/src/query/server/server.go index 1ddde0f678..896574b0d3 100644 --- a/src/query/server/server.go +++ b/src/query/server/server.go @@ -953,7 +953,6 @@ func startCarbonIngestion( // Create ingester. ingester, err := ingestcarbon.NewIngester( downsamplerAndWriter, rules, ingestcarbon.Options{ - Debug: ingesterCfg.Debug, InstrumentOptions: carbonIOpts, WorkerPool: workerPool, }) From af78be40fd7777727facb4a47d90c1e9def804cc Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Mon, 28 Oct 2019 15:10:57 -0400 Subject: [PATCH 2/4] Fix import --- src/cmd/services/m3coordinator/ingest/carbon/ingest.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cmd/services/m3coordinator/ingest/carbon/ingest.go b/src/cmd/services/m3coordinator/ingest/carbon/ingest.go index 6d414bd2bb..62f64fefcf 100644 --- a/src/cmd/services/m3coordinator/ingest/carbon/ingest.go +++ b/src/cmd/services/m3coordinator/ingest/carbon/ingest.go @@ -30,8 +30,6 @@ import ( "sync" "time" - "go.uber.org/zap/zapcore" - "github.com/m3db/m3/src/cmd/services/m3coordinator/downsample" "github.com/m3db/m3/src/cmd/services/m3coordinator/ingest" "github.com/m3db/m3/src/cmd/services/m3query/config" @@ -49,6 +47,7 @@ import ( "github.com/uber-go/tally" "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) const ( From d310787779d74f5aed49a7358fc8e5be8c35d1b5 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Mon, 28 Oct 2019 15:15:37 -0400 Subject: [PATCH 3/4] Use checked entries everywhere --- .../m3coordinator/ingest/carbon/ingest.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/cmd/services/m3coordinator/ingest/carbon/ingest.go b/src/cmd/services/m3coordinator/ingest/carbon/ingest.go index 62f64fefcf..df5b9b4bc2 100644 --- a/src/cmd/services/m3coordinator/ingest/carbon/ingest.go +++ b/src/cmd/services/m3coordinator/ingest/carbon/ingest.go @@ -223,8 +223,7 @@ func (i *ingester) write( debugLog := i.logger.Check(zapcore.DebugLevel, "carbon metric matched by pattern") if debugLog != nil { - i.logger.Info("carbon metric matched by pattern", - zap.ByteString("name", resources.name), + debugLog.Write(zap.ByteString("name", resources.name), zap.String("pattern", rule.rule.Pattern), zap.Any("mappingRules", rule.mappingRules), zap.Any("storagePolicies", rule.storagePolicies)) @@ -238,8 +237,10 @@ func (i *ingester) write( if len(downsampleAndStoragePolicies.DownsampleMappingRules) == 0 && len(downsampleAndStoragePolicies.WriteStoragePolicies) == 0 { // Nothing to do if none of the policies matched. - i.logger.Debug("no rules matched carbon metric, skipping", - zap.ByteString("name", resources.name)) + debugLog := i.logger.Check(zapcore.DebugLevel, "no rules matched carbon metric, skipping") + if debugLog != nil { + debugLog.Write(zap.ByteString("name", resources.name)) + } return false } @@ -262,8 +263,10 @@ func (i *ingester) write( return false } - i.logger.Debug("successfully wrote carbon metric", - zap.ByteString("name", resources.name)) + debugLog := i.logger.Check(zapcore.DebugLevel, "successfully wrote carbon metric") + if debugLog != nil { + debugLog.Write(zap.ByteString("name", resources.name)) + } return true } From 2a819aad7ea70bdfd52e36c0dd6ba36e77fe0313 Mon Sep 17 00:00:00 2001 From: Rob Skillington Date: Mon, 28 Oct 2019 16:38:46 -0400 Subject: [PATCH 4/4] Add deprecated notice to debug flag on ingester --- src/cmd/services/m3query/config/config.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/cmd/services/m3query/config/config.go b/src/cmd/services/m3query/config/config.go index b05feeaa16..ce3efcc834 100644 --- a/src/cmd/services/m3query/config/config.go +++ b/src/cmd/services/m3query/config/config.go @@ -292,9 +292,12 @@ type CarbonConfiguration struct { // CarbonIngesterConfiguration is the configuration struct for carbon ingestion. type CarbonIngesterConfiguration struct { - ListenAddress string `yaml:"listenAddress"` - MaxConcurrency int `yaml:"maxConcurrency"` - Rules []CarbonIngesterRuleConfiguration `yaml:"rules"` + // Deprecated: simply use the logger debug level, this has been deprecated + // in favor of setting the log level to debug. + DeprecatedDebug bool `yaml:"debug"` + ListenAddress string `yaml:"listenAddress"` + MaxConcurrency int `yaml:"maxConcurrency"` + Rules []CarbonIngesterRuleConfiguration `yaml:"rules"` } // LookbackDurationOrDefault validates the LookbackDuration