Skip to content

Commit

Permalink
Blocked classic prometheus constructors, moved all to promauto; Remov…
Browse files Browse the repository at this point in the history
…ed unnecessary printfs.

Fixes: #2102

Also blocked them on CI side, thanks to fatih/faillint#8

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>
  • Loading branch information
bwplotka committed Mar 7, 2020
1 parent 0fc20b6 commit f30de63
Show file tree
Hide file tree
Showing 41 changed files with 230 additions and 324 deletions.
13 changes: 10 additions & 3 deletions Makefile
Expand Up @@ -78,9 +78,9 @@ ALERTMANAGER ?= $(GOBIN)/alertmanager-$(ALERTMANAGER_VERSION)
MINIO_SERVER_VERSION ?= RELEASE.2018-10-06T00-15-16Z
MINIO_SERVER ?=$(GOBIN)/minio-$(MINIO_SERVER_VERSION)

# TODO(bwplotka): Update to newest as soon as https://github.com/fatih/faillint/pull/8 merges.
FAILLINT_VERSION ?= v1.0.1
FAILLINT ?=$(GOBIN)/faillint-$(FAILLINT_VERSION)
MODULES_TO_AVOID ?=errors,github.com/prometheus/tsdb,github.com/prometheus/prometheus/pkg/testutils

# fetch_go_bin_version downloads (go gets) the binary from specific version and installs it in $(GOBIN)/<bin>-<version>
# arguments:
Expand Down Expand Up @@ -139,7 +139,6 @@ assets: $(GOBINDATA)
@$(GOBINDATA) $(bindata_flags) -pkg ui -o pkg/ui/bindata.go -ignore '(.*\.map|bootstrap\.js|bootstrap-theme\.css|bootstrap\.css)' pkg/ui/templates/... pkg/ui/static/...
@go fmt ./pkg/ui


.PHONY: build
build: ## Builds Thanos binary using `promu`.
build: check-git deps $(PROMU)
Expand Down Expand Up @@ -295,7 +294,15 @@ lint: ## Runs various static analysis against our code.
lint: check-git deps $(GOLANGCILINT) $(MISSPELL) $(FAILLINT)
$(call require_clean_work_tree,"detected not clean master before running lint")
@echo ">> verifying modules being imported"
@$(FAILLINT) -paths $(MODULES_TO_AVOID) ./...
# TODO(bwplotka): Add, Printf, DefaultRegisterer, NewGaugeFunc and MustRegister once execption are accepted. Add fmt.{Errorf}=github.com/pkg/errors.{Errorf} once https://github.com/fatih/faillint/issues/10 is addressed.
@$(FAILLINT) -paths "errors=github.com/pkg/errors,\
github.com/prometheus/tsdb=github.com/prometheus/prometheus/tsdb,\
github.com/prometheus/prometheus/pkg/testutils=github.com/thanos-io/thanos/pkg/testutil,\
github.com/prometheus/client_golang/prometheus.{DefaultGatherer,DefBuckets,NewUntypedFunc,UntypedFunc},\
github.com/prometheus/client_golang/prometheus.{NewCounter,NewCounterVec,NewCounterVec,NewGauge,NewGaugeVec,NewGaugeFunc,\
NewHistorgram,NewHistogramVec,NewSummary,NewSummaryVec}=github.com/prometheus/client_golang/prometheus/promauto.{NewCounter,\
NewCounterVec,NewCounterVec,NewGauge,NewGaugeVec,NewGaugeFunc,NewHistorgram,NewHistogramVec,NewSummary,NewSummaryVec}" ./...
@$(FAILLINT) -paths "fmt.{Print,Println,Sprint}" -ignore-tests ./...
@echo ">> examining all of the Go files"
@go vet -stdmethods=false ./pkg/... ./cmd/... && go vet doc.go
@echo ">> linting all of the Go files GOGC=${GOGC}"
Expand Down
4 changes: 2 additions & 2 deletions cmd/thanos/bucket.go
Expand Up @@ -269,7 +269,7 @@ func registerBucketInspect(m map[string]setupFunc, root *kingpin.CmdClause, name
// Parse selector.
selectorLabels, err := parseFlagLabels(*selector)
if err != nil {
return fmt.Errorf("error parsing selector flag: %v", err)
return errors.Errorf("error parsing selector flag: %v", err)
}

confContentYaml, err := objStoreConfig.Content()
Expand Down Expand Up @@ -523,7 +523,7 @@ func printTable(blockMetas []*metadata.Meta, selectorLabels labels.Labels, sortB
for _, col := range sortBy {
index := getIndex(header, col)
if index == -1 {
return fmt.Errorf("column %s not found", col)
return errors.Errorf("column %s not found", col)
}
sortByColNum = append(sortByColNum, index)
}
Expand Down
14 changes: 6 additions & 8 deletions cmd/thanos/compact.go
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/opentracing/opentracing-go"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/prometheus/tsdb"
"github.com/thanos-io/thanos/pkg/block"
"github.com/thanos-io/thanos/pkg/block/indexheader"
Expand Down Expand Up @@ -170,25 +171,23 @@ func runCompact(
concurrency int,
selectorRelabelConf *extflag.PathOrContent,
) error {
halted := prometheus.NewGauge(prometheus.GaugeOpts{
halted := promauto.With(reg).NewGauge(prometheus.GaugeOpts{
Name: "thanos_compactor_halted",
Help: "Set to 1 if the compactor halted due to an unexpected error.",
})
halted.Set(0)
retried := prometheus.NewCounter(prometheus.CounterOpts{
retried := promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: "thanos_compactor_retries_total",
Help: "Total number of retries after retriable compactor error.",
})
iterations := prometheus.NewCounter(prometheus.CounterOpts{
iterations := promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: "thanos_compactor_iterations_total",
Help: "Total number of iterations that were executed successfully.",
})
partialUploadDeleteAttempts := prometheus.NewCounter(prometheus.CounterOpts{
partialUploadDeleteAttempts := promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: "thanos_compactor_aborted_partial_uploads_deletion_attempts_total",
Help: "Total number of started deletions of blocks that are assumed aborted and only partially uploaded.",
})
reg.MustRegister(halted, retried, iterations, partialUploadDeleteAttempts)

downsampleMetrics := newDownsampleMetrics(reg)

httpProbe := prober.NewHTTP()
Expand Down Expand Up @@ -390,11 +389,10 @@ func runCompact(

// genMissingIndexCacheFiles scans over all blocks, generates missing index cache files and uploads them to object storage.
func genMissingIndexCacheFiles(ctx context.Context, logger log.Logger, reg *prometheus.Registry, bkt objstore.Bucket, fetcher block.MetadataFetcher, dir string) error {
genIndex := prometheus.NewCounter(prometheus.CounterOpts{
genIndex := promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: metricIndexGenerateName,
Help: metricIndexGenerateHelp,
})
reg.MustRegister(genIndex)

if err := os.RemoveAll(dir); err != nil {
return errors.Wrap(err, "clean index cache directory")
Expand Down
8 changes: 3 additions & 5 deletions cmd/thanos/downsample.go
Expand Up @@ -16,6 +16,7 @@ import (
opentracing "github.com/opentracing/opentracing-go"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/prometheus/tsdb"
"github.com/prometheus/prometheus/tsdb/chunkenc"
"github.com/thanos-io/thanos/pkg/block"
Expand Down Expand Up @@ -57,18 +58,15 @@ type DownsampleMetrics struct {
func newDownsampleMetrics(reg *prometheus.Registry) *DownsampleMetrics {
m := new(DownsampleMetrics)

m.downsamples = prometheus.NewCounterVec(prometheus.CounterOpts{
m.downsamples = promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
Name: "thanos_compact_downsample_total",
Help: "Total number of downsampling attempts.",
}, []string{"group"})
m.downsampleFailures = prometheus.NewCounterVec(prometheus.CounterOpts{
m.downsampleFailures = promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
Name: "thanos_compact_downsample_failures_total",
Help: "Total number of failed downsampling attempts.",
}, []string{"group"})

reg.MustRegister(m.downsamples)
reg.MustRegister(m.downsampleFailures)

return m
}

Expand Down
1 change: 1 addition & 0 deletions cmd/thanos/main.go
Expand Up @@ -119,6 +119,7 @@ func main() {
prometheus.NewProcessCollector(prometheus.ProcessCollectorOpts{}),
)

// Some packages still use default Register. Replace to have those metrics.
prometheus.DefaultRegisterer = metrics
// Memberlist uses go-metrics.
sink, err := gprom.NewPrometheusSink()
Expand Down
5 changes: 2 additions & 3 deletions cmd/thanos/main_test.go
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/go-kit/kit/log"
"github.com/oklog/ulid"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
promtest "github.com/prometheus/client_golang/prometheus/testutil"
"github.com/prometheus/prometheus/pkg/labels"
"github.com/thanos-io/thanos/pkg/block"
Expand Down Expand Up @@ -71,12 +72,10 @@ func TestCleanupIndexCacheFolder(t *testing.T) {

reg := prometheus.NewRegistry()
expReg := prometheus.NewRegistry()
genIndexExp := prometheus.NewCounter(prometheus.CounterOpts{
genIndexExp := promauto.With(expReg).NewCounter(prometheus.CounterOpts{
Name: metricIndexGenerateName,
Help: metricIndexGenerateHelp,
})
expReg.MustRegister(genIndexExp)

metaFetcher, err := block.NewMetaFetcher(nil, 32, bkt, "", nil)
testutil.Ok(t, err)

Expand Down
4 changes: 2 additions & 2 deletions cmd/thanos/query.go
Expand Up @@ -18,6 +18,7 @@ import (
opentracing "github.com/opentracing/opentracing-go"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/common/route"
"github.com/prometheus/prometheus/discovery/file"
"github.com/prometheus/prometheus/discovery/targetgroup"
Expand Down Expand Up @@ -204,11 +205,10 @@ func runQuery(
comp component.Component,
) error {
// TODO(bplotka in PR #513 review): Move arguments into struct.
duplicatedStores := prometheus.NewCounter(prometheus.CounterOpts{
duplicatedStores := promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: "thanos_query_duplicated_store_addresses_total",
Help: "The number of times a duplicated store addresses is detected from the different configs in query",
})
reg.MustRegister(duplicatedStores)

dialOpts, err := extgrpc.StoreClientGRPCOpts(logger, reg, tracer, secure, cert, key, caCert, serverName)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions cmd/thanos/sidecar.go
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/opentracing/opentracing-go"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/pkg/labels"
"github.com/thanos-io/thanos/pkg/block/metadata"
Expand Down Expand Up @@ -183,15 +184,14 @@ func runSidecar(

// Setup all the concurrent groups.
{
promUp := prometheus.NewGauge(prometheus.GaugeOpts{
promUp := promauto.With(reg).NewGauge(prometheus.GaugeOpts{
Name: "thanos_sidecar_prometheus_up",
Help: "Boolean indicator whether the sidecar can reach its Prometheus peer.",
})
lastHeartbeat := prometheus.NewGauge(prometheus.GaugeOpts{
lastHeartbeat := promauto.With(reg).NewGauge(prometheus.GaugeOpts{
Name: "thanos_sidecar_last_heartbeat_success_time_seconds",
Help: "Second timestamp of the last successful heartbeat.",
})
reg.MustRegister(promUp, lastHeartbeat)

ctx, cancel := context.WithCancel(context.Background())
g.Add(func() error {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -38,7 +38,7 @@ require (
github.com/opentracing/opentracing-go v1.1.1-0.20200124165624-2876d2018785
github.com/pkg/errors v0.9.1
github.com/prometheus/alertmanager v0.20.0
github.com/prometheus/client_golang v1.4.2-0.20200214154132-b25ce2693a6d
github.com/prometheus/client_golang v1.5.0
github.com/prometheus/client_model v0.2.0
github.com/prometheus/common v0.9.1
github.com/prometheus/prometheus v1.8.2-0.20200213233353-b90be6f32a33
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Expand Up @@ -592,8 +592,8 @@ github.com/prometheus/client_golang v1.1.0/go.mod h1:I1FGZT9+L76gKKOs5djB6ezCbFQ
github.com/prometheus/client_golang v1.2.0/go.mod h1:XMU6Z2MjaRKVu/dC1qupJI9SiNkDYzz3xecMgSW/F+U=
github.com/prometheus/client_golang v1.2.1 h1:JnMpQc6ppsNgw9QPAGF6Dod479itz7lvlsMzzNayLOI=
github.com/prometheus/client_golang v1.2.1/go.mod h1:XMU6Z2MjaRKVu/dC1qupJI9SiNkDYzz3xecMgSW/F+U=
github.com/prometheus/client_golang v1.4.2-0.20200214154132-b25ce2693a6d h1:6GpNaEnOxPO8IxMm5zmXdIpCGayuQmp7udttdxB2BbM=
github.com/prometheus/client_golang v1.4.2-0.20200214154132-b25ce2693a6d/go.mod h1:e9GMxYsXl05ICDXkRhurwBS4Q3OK1iX/F2sw+iXX5zU=
github.com/prometheus/client_golang v1.5.0 h1:Ctq0iGpCmr3jeP77kbF2UxgvRwzWWz+4Bh9/vJTyg1A=
github.com/prometheus/client_golang v1.5.0/go.mod h1:e9GMxYsXl05ICDXkRhurwBS4Q3OK1iX/F2sw+iXX5zU=
github.com/prometheus/client_model v0.0.0-20170216185247-6f3806018612/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
github.com/prometheus/client_model v0.0.0-20190115171406-56726106282f/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
Expand Down
25 changes: 10 additions & 15 deletions pkg/alert/alert.go
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/pkg/errors"
"github.com/prometheus/alertmanager/api/v2/models"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/prometheus/pkg/labels"

"github.com/thanos-io/thanos/pkg/runutil"
Expand Down Expand Up @@ -133,34 +134,31 @@ func NewQueue(logger log.Logger, reg prometheus.Registerer, capacity, maxBatchSi
toAddLset: toAdd,
toExcludeLabels: toExclude,

dropped: prometheus.NewCounter(prometheus.CounterOpts{
dropped: promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: "thanos_alert_queue_alerts_dropped_total",
Help: "Total number of alerts that were dropped from the queue.",
}),
pushed: prometheus.NewCounter(prometheus.CounterOpts{
pushed: promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: "thanos_alert_queue_alerts_pushed_total",
Help: "Total number of alerts pushed to the queue.",
}),
popped: prometheus.NewCounter(prometheus.CounterOpts{
popped: promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: "thanos_alert_queue_alerts_popped_total",
Help: "Total number of alerts popped from the queue.",
}),
}
capMetric := prometheus.NewGaugeFunc(prometheus.GaugeOpts{
_ = promauto.With(reg).NewGaugeFunc(prometheus.GaugeOpts{
Name: "thanos_alert_queue_capacity",
Help: "Capacity of the alert queue.",
}, func() float64 {
return float64(q.Cap())
})
lenMetric := prometheus.NewGaugeFunc(prometheus.GaugeOpts{
_ = promauto.With(reg).NewGaugeFunc(prometheus.GaugeOpts{
Name: "thanos_alert_queue_length",
Help: "Length of the alert queue.",
}, func() float64 {
return float64(q.Len())
})
if reg != nil {
reg.MustRegister(q.pushed, q.popped, q.dropped, lenMetric, capMetric)
}
return q
}

Expand Down Expand Up @@ -292,29 +290,26 @@ func NewSender(
alertmanagers: alertmanagers,
versions: versions,

sent: prometheus.NewCounterVec(prometheus.CounterOpts{
sent: promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
Name: "thanos_alert_sender_alerts_sent_total",
Help: "Total number of alerts sent by alertmanager.",
}, []string{"alertmanager"}),

errs: prometheus.NewCounterVec(prometheus.CounterOpts{
errs: promauto.With(reg).NewCounterVec(prometheus.CounterOpts{
Name: "thanos_alert_sender_errors_total",
Help: "Total number of errors while sending alerts to alertmanager.",
}, []string{"alertmanager"}),

dropped: prometheus.NewCounter(prometheus.CounterOpts{
dropped: promauto.With(reg).NewCounter(prometheus.CounterOpts{
Name: "thanos_alert_sender_alerts_dropped_total",
Help: "Total number of alerts dropped in case of all sends to alertmanagers failed.",
}),

latency: prometheus.NewHistogramVec(prometheus.HistogramOpts{
latency: promauto.With(reg).NewHistogramVec(prometheus.HistogramOpts{
Name: "thanos_alert_sender_latency_seconds",
Help: "Latency for sending alert notifications (not including dropped notifications).",
}, []string{"alertmanager"}),
}
if reg != nil {
reg.MustRegister(s.sent, s.errs, s.dropped, s.latency)
}
return s
}

Expand Down
22 changes: 7 additions & 15 deletions pkg/block/fetcher.go
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/oklog/ulid"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/prometheus/pkg/labels"
"github.com/prometheus/prometheus/pkg/relabel"
"github.com/prometheus/prometheus/tsdb"
Expand Down Expand Up @@ -54,26 +55,26 @@ const (
duplicateMeta = "duplicate"
)

func newSyncMetrics(r prometheus.Registerer) *syncMetrics {
func newSyncMetrics(reg prometheus.Registerer) *syncMetrics {
var m syncMetrics

m.syncs = prometheus.NewCounter(prometheus.CounterOpts{
m.syncs = promauto.With(reg).NewCounter(prometheus.CounterOpts{
Subsystem: syncMetricSubSys,
Name: "syncs_total",
Help: "Total blocks metadata synchronization attempts",
})
m.syncFailures = prometheus.NewCounter(prometheus.CounterOpts{
m.syncFailures = promauto.With(reg).NewCounter(prometheus.CounterOpts{
Subsystem: syncMetricSubSys,
Name: "sync_failures_total",
Help: "Total blocks metadata synchronization failures",
})
m.syncDuration = prometheus.NewHistogram(prometheus.HistogramOpts{
m.syncDuration = promauto.With(reg).NewHistogram(prometheus.HistogramOpts{
Subsystem: syncMetricSubSys,
Name: "sync_duration_seconds",
Help: "Duration of the blocks metadata synchronization in seconds",
Buckets: []float64{0.01, 1, 10, 100, 1000},
})
m.synced = extprom.NewTxGaugeVec(prometheus.GaugeOpts{
m.synced = extprom.NewTxGaugeVec(reg, prometheus.GaugeOpts{
Subsystem: syncMetricSubSys,
Name: "synced",
Help: "Number of block metadata synced",
Expand All @@ -88,14 +89,6 @@ func newSyncMetrics(r prometheus.Registerer) *syncMetrics {
[]string{timeExcludedMeta},
[]string{duplicateMeta},
)
if r != nil {
r.MustRegister(
m.syncs,
m.syncFailures,
m.syncDuration,
m.synced,
)
}
return &m
}

Expand Down Expand Up @@ -544,13 +537,12 @@ func NewConsistencyDelayMetaFilter(logger log.Logger, consistencyDelay time.Dura
if logger == nil {
logger = log.NewNopLogger()
}
consistencyDelayMetric := prometheus.NewGaugeFunc(prometheus.GaugeOpts{
_ = promauto.With(reg).NewGaugeFunc(prometheus.GaugeOpts{
Name: "consistency_delay_seconds",
Help: "Configured consistency delay in seconds.",
}, func() float64 {
return consistencyDelay.Seconds()
})
reg.MustRegister(consistencyDelayMetric)

return &ConsistencyDelayMetaFilter{
logger: logger,
Expand Down

0 comments on commit f30de63

Please sign in to comment.