Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blocked classic prometheus constructors, moved all to promauto; Removed unnecessary printfs. #2228

Merged
merged 1 commit into from
Mar 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ ALERTMANAGER ?= $(GOBIN)/alertmanager-$(ALERTMANAGER_VERSION)
MINIO_SERVER_VERSION ?= RELEASE.2018-10-06T00-15-16Z
MINIO_SERVER ?=$(GOBIN)/minio-$(MINIO_SERVER_VERSION)

FAILLINT_VERSION ?= v1.0.1
FAILLINT_VERSION ?= v1.2.0
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 +138,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 +293,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 exception 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to do _ = func() or just func()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I do _ to indicate that this returns value, but we don't need it. Do you think it's wrong pattern?

Copy link
Member Author

@bwplotka bwplotka Mar 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it as it helps avoiding surprises (: But happy to change this in the following PRs 🤗

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wanted to make sure we were being consistent in the project one way or the other so 👍

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
Original file line number Diff line number Diff line change
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
Loading