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

Enable same linters as the Prometheus repo itself #1056

Merged
merged 6 commits into from Jun 17, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/golangci-lint.yml
@@ -1,3 +1,4 @@
---
name: golangci-lint
on:
push:
Expand Down
32 changes: 29 additions & 3 deletions .golangci.yml
@@ -1,5 +1,31 @@
# Run only staticcheck for now. Additional linters will be enabled one-by-one.
---

run:
deadline: 5m

output:
sort-results: true

linters:
enable:
- staticcheck
disable-all: true
- gofumpt
- goimports
- revive
- misspell

issues:
max-same-issues: 0
exclude-rules:
- path: _test.go
linters:
Copy link
Member

Choose a reason for hiding this comment

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

These linters don't seem to be enabled, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

golangci-lint has a list of default linters that are enabled by default, unless being disabled explicitly or implicitly. in the previous config they were disabled implicitly by using disable-all: true in this line

the default linters list is:

  • deadcode
  • errcheck
  • gosimple
  • govet
  • ineffassign
  • staticcheck
  • structcheck
  • typecheck
  • unused
  • varcheck

since in the main prometheus repo they left them enabled implicitly i did the same too but i understand that it could reduce readability.

do you want me to enable them in the config explicitly or leave them as they are?

Copy link
Member

Choose a reason for hiding this comment

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

Having them explicit would be amazing. Also, this would prevent any unwanted issues, if we upgrade golangci-lint and decide to change their default preset.

- errcheck
- govet
- structcheck

linters-settings:
errcheck:
exclude: scripts/errcheck_excludes.txt
goimports:
local-prefixes: github.com/prometheus/client_golang
gofumpt:
extra-rules: true
1 change: 0 additions & 1 deletion api/client_test.go
Expand Up @@ -134,7 +134,6 @@ func BenchmarkClient(b *testing.B) {

for _, sizeKB := range []int{4, 50, 1000, 2000} {
b.Run(fmt.Sprintf("%dKB", sizeKB), func(b *testing.B) {

testServer := httptest.NewServer(serveSpaces{sizeKB})
defer testServer.Close()

Expand Down
31 changes: 14 additions & 17 deletions api/prometheus/v1/api.go
Expand Up @@ -109,7 +109,6 @@ func marshalPointJSON(ptr unsafe.Pointer, stream *json.Stream) {

stream.WriteRaw(`"`)
stream.WriteArrayEnd()

}

func marshalPointJSONIsEmpty(ptr unsafe.Pointer) bool {
Expand Down Expand Up @@ -230,25 +229,25 @@ type API interface {
// Config returns the current Prometheus configuration.
Config(ctx context.Context) (ConfigResult, error)
// DeleteSeries deletes data for a selection of series in a time range.
DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) error
DeleteSeries(ctx context.Context, matches []string, startTime, endTime time.Time) error
// Flags returns the flag values that Prometheus was launched with.
Flags(ctx context.Context) (FlagsResult, error)
// LabelNames returns the unique label names present in the block in sorted order by given time range and matchers.
LabelNames(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]string, Warnings, error)
LabelNames(ctx context.Context, matches []string, startTime, endTime time.Time) ([]string, Warnings, error)
// LabelValues performs a query for the values of the given label, time range and matchers.
LabelValues(ctx context.Context, label string, matches []string, startTime time.Time, endTime time.Time) (model.LabelValues, Warnings, error)
LabelValues(ctx context.Context, label string, matches []string, startTime, endTime time.Time) (model.LabelValues, Warnings, error)
// Query performs a query for the given time.
Query(ctx context.Context, query string, ts time.Time, opts ...Option) (model.Value, Warnings, error)
// QueryRange performs a query for the given range.
QueryRange(ctx context.Context, query string, r Range, opts ...Option) (model.Value, Warnings, error)
// QueryExemplars performs a query for exemplars by the given query and time range.
QueryExemplars(ctx context.Context, query string, startTime time.Time, endTime time.Time) ([]ExemplarQueryResult, error)
QueryExemplars(ctx context.Context, query string, startTime, endTime time.Time) ([]ExemplarQueryResult, error)
// Buildinfo returns various build information properties about the Prometheus server
Buildinfo(ctx context.Context) (BuildinfoResult, error)
// Runtimeinfo returns the various runtime information properties about the Prometheus server.
Runtimeinfo(ctx context.Context) (RuntimeinfoResult, error)
// Series finds series by label matchers.
Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, Warnings, error)
Series(ctx context.Context, matches []string, startTime, endTime time.Time) ([]model.LabelSet, Warnings, error)
// Snapshot creates a snapshot of all current data into snapshots/<datetime>-<rand>
// under the TSDB's data directory and returns the directory as response.
Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, error)
Expand All @@ -257,9 +256,9 @@ type API interface {
// Targets returns an overview of the current state of the Prometheus target discovery.
Targets(ctx context.Context) (TargetsResult, error)
// TargetsMetadata returns metadata about metrics currently scraped by the target.
TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, error)
TargetsMetadata(ctx context.Context, matchTarget, metric, limit string) ([]MetricMetadata, error)
// Metadata returns metadata about metrics currently scraped by the metric name.
Metadata(ctx context.Context, metric string, limit string) (map[string][]Metadata, error)
Metadata(ctx context.Context, metric, limit string) (map[string][]Metadata, error)
// TSDB returns the cardinality statistics.
TSDB(ctx context.Context) (TSDBResult, error)
// WalReplay returns the current replay status of the wal.
Expand Down Expand Up @@ -699,7 +698,7 @@ func (h *httpAPI) Config(ctx context.Context) (ConfigResult, error) {
return res, json.Unmarshal(body, &res)
}

func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) error {
func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime, endTime time.Time) error {
u := h.client.URL(epDeleteSeries, nil)
q := u.Query()

Expand Down Expand Up @@ -772,7 +771,7 @@ func (h *httpAPI) Runtimeinfo(ctx context.Context) (RuntimeinfoResult, error) {
return res, json.Unmarshal(body, &res)
}

func (h *httpAPI) LabelNames(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]string, Warnings, error) {
func (h *httpAPI) LabelNames(ctx context.Context, matches []string, startTime, endTime time.Time) ([]string, Warnings, error) {
u := h.client.URL(epLabels, nil)
q := u.Query()
q.Set("start", formatTime(startTime))
Expand All @@ -795,7 +794,7 @@ func (h *httpAPI) LabelNames(ctx context.Context, matches []string, startTime ti
return labelNames, w, json.Unmarshal(body, &labelNames)
}

func (h *httpAPI) LabelValues(ctx context.Context, label string, matches []string, startTime time.Time, endTime time.Time) (model.LabelValues, Warnings, error) {
func (h *httpAPI) LabelValues(ctx context.Context, label string, matches []string, startTime, endTime time.Time) (model.LabelValues, Warnings, error) {
u := h.client.URL(epLabelValues, map[string]string{"name": label})
q := u.Query()
q.Set("start", formatTime(startTime))
Expand Down Expand Up @@ -833,7 +832,6 @@ func WithTimeout(timeout time.Duration) Option {
}

func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time, opts ...Option) (model.Value, Warnings, error) {

u := h.client.URL(epQuery, nil)
q := u.Query()

Expand Down Expand Up @@ -890,7 +888,7 @@ func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range, opts ..
return model.Value(qres.v), warnings, json.Unmarshal(body, &qres)
}

func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, Warnings, error) {
func (h *httpAPI) Series(ctx context.Context, matches []string, startTime, endTime time.Time) ([]model.LabelSet, Warnings, error) {
u := h.client.URL(epSeries, nil)
q := u.Query()

Expand Down Expand Up @@ -973,7 +971,7 @@ func (h *httpAPI) Targets(ctx context.Context) (TargetsResult, error) {
return res, json.Unmarshal(body, &res)
}

func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, error) {
func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget, metric, limit string) ([]MetricMetadata, error) {
u := h.client.URL(epTargetsMetadata, nil)
q := u.Query()

Expand All @@ -997,7 +995,7 @@ func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metri
return res, json.Unmarshal(body, &res)
}

func (h *httpAPI) Metadata(ctx context.Context, metric string, limit string) (map[string][]Metadata, error) {
func (h *httpAPI) Metadata(ctx context.Context, metric, limit string) (map[string][]Metadata, error) {
u := h.client.URL(epMetadata, nil)
q := u.Query()

Expand Down Expand Up @@ -1054,7 +1052,7 @@ func (h *httpAPI) WalReplay(ctx context.Context) (WalReplayStatus, error) {
return res, json.Unmarshal(body, &res)
}

func (h *httpAPI) QueryExemplars(ctx context.Context, query string, startTime time.Time, endTime time.Time) ([]ExemplarQueryResult, error) {
func (h *httpAPI) QueryExemplars(ctx context.Context, query string, startTime, endTime time.Time) ([]ExemplarQueryResult, error) {
u := h.client.URL(epQueryExemplars, nil)
q := u.Query()

Expand Down Expand Up @@ -1162,7 +1160,6 @@ func (h *apiClientImpl) Do(ctx context.Context, req *http.Request) (*http.Respon
}

return resp, []byte(result.Data), result.Warnings, err

}

// DoGetFallback will attempt to do the request as-is, and on a 405 or 501 it
Expand Down
25 changes: 12 additions & 13 deletions api/prometheus/v1/api_test.go
Expand Up @@ -65,7 +65,6 @@ func (c *apiTestClient) URL(ep string, args map[string]string) *url.URL {
}

func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, Warnings, error) {

test := c.curTest

if req.URL.Path != test.reqPath {
Expand Down Expand Up @@ -101,7 +100,6 @@ func (c *apiTestClient) DoGetFallback(ctx context.Context, u *url.URL, args url.
}

func TestAPIs(t *testing.T) {

testTime := time.Now()

tc := &apiTestClient{
Expand Down Expand Up @@ -131,7 +129,7 @@ func TestAPIs(t *testing.T) {
}
}

doDeleteSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, Warnings, error) {
doDeleteSeries := func(matcher string, startTime, endTime time.Time) func() (interface{}, Warnings, error) {
return func() (interface{}, Warnings, error) {
return nil, nil, promAPI.DeleteSeries(context.Background(), []string{matcher}, startTime, endTime)
}
Expand Down Expand Up @@ -182,7 +180,7 @@ func TestAPIs(t *testing.T) {
}
}

doSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, Warnings, error) {
doSeries := func(matcher string, startTime, endTime time.Time) func() (interface{}, Warnings, error) {
return func() (interface{}, Warnings, error) {
return promAPI.Series(context.Background(), []string{matcher}, startTime, endTime)
}
Expand All @@ -209,14 +207,14 @@ func TestAPIs(t *testing.T) {
}
}

doTargetsMetadata := func(matchTarget string, metric string, limit string) func() (interface{}, Warnings, error) {
doTargetsMetadata := func(matchTarget, metric, limit string) func() (interface{}, Warnings, error) {
return func() (interface{}, Warnings, error) {
v, err := promAPI.TargetsMetadata(context.Background(), matchTarget, metric, limit)
return v, nil, err
}
}

doMetadata := func(metric string, limit string) func() (interface{}, Warnings, error) {
doMetadata := func(metric, limit string) func() (interface{}, Warnings, error) {
return func() (interface{}, Warnings, error) {
v, err := promAPI.Metadata(context.Background(), metric, limit)
return v, nil, err
Expand All @@ -237,7 +235,7 @@ func TestAPIs(t *testing.T) {
}
}

doQueryExemplars := func(query string, startTime time.Time, endTime time.Time) func() (interface{}, Warnings, error) {
doQueryExemplars := func(query string, startTime, endTime time.Time) func() (interface{}, Warnings, error) {
return func() (interface{}, Warnings, error) {
v, err := promAPI.QueryExemplars(context.Background(), query, startTime, endTime)
return v, nil, err
Expand Down Expand Up @@ -471,7 +469,8 @@ func TestAPIs(t *testing.T) {
{
"__name__": "up",
"job": "prometheus",
"instance": "localhost:9090"},
"instance": "localhost:9090",
},
},
reqMethod: "GET",
reqPath: "/api/v1/series",
Expand All @@ -495,7 +494,8 @@ func TestAPIs(t *testing.T) {
{
"__name__": "up",
"job": "prometheus",
"instance": "localhost:9090"},
"instance": "localhost:9090",
},
},
inWarnings: []string{"a"},
reqMethod: "GET",
Expand Down Expand Up @@ -586,7 +586,8 @@ func TestAPIs(t *testing.T) {
{
"__name__": "up",
"job": "prometheus",
"instance": "localhost:9090"},
"instance": "localhost:9090",
},
},
reqMethod: "POST",
reqPath: "/api/v1/admin/tsdb/delete_series",
Expand Down Expand Up @@ -1115,7 +1116,7 @@ func TestAPIs(t *testing.T) {
"limit": []string{"1"},
},
res: map[string][]Metadata{
"go_goroutines": []Metadata{
"go_goroutines": {
{
Type: "gauge",
Help: "Number of goroutines that currently exist.",
Expand Down Expand Up @@ -1523,7 +1524,6 @@ func TestAPIClientDo(t *testing.T) {

for i, test := range tests {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {

tc.ch <- test

_, body, warnings, err := client.Do(context.Background(), tc.req)
Expand Down Expand Up @@ -1564,7 +1564,6 @@ func TestAPIClientDo(t *testing.T) {
t.Fatalf("expected body :%v, but got:%v", test.expectedBody, string(body))
}
})

}
}

Expand Down
3 changes: 2 additions & 1 deletion api/prometheus/v1/example_test.go
Expand Up @@ -22,9 +22,10 @@ import (
"os"
"time"

"github.com/prometheus/common/config"

"github.com/prometheus/client_golang/api"
v1 "github.com/prometheus/client_golang/api/prometheus/v1"
"github.com/prometheus/common/config"
)

func ExampleAPI_query() {
Expand Down
1 change: 0 additions & 1 deletion prometheus/collector_test.go
Expand Up @@ -30,7 +30,6 @@ func (c collectorDescribedByCollect) Describe(ch chan<- *Desc) {
}

func TestDescribeByCollect(t *testing.T) {

goodCollector := collectorDescribedByCollect{
cnt: NewCounter(CounterOpts{Name: "c1", Help: "help c1"}),
gge: NewGauge(GaugeOpts{Name: "g1", Help: "help g1"}),
Expand Down
3 changes: 2 additions & 1 deletion prometheus/collectors/go_collector_latest.go
Expand Up @@ -72,7 +72,8 @@ const (
//
// The current default is GoRuntimeMemStatsCollection, so the compatibility mode with
// client_golang pre v1.12 (move to runtime/metrics).
func WithGoCollections(flags GoCollectionOption) goOption {
//nolint:staticcheck // Ignore SA1019 until v2.
Copy link
Member

Choose a reason for hiding this comment

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

Is staticcheck enabled? From the config above, it doesn't seem like it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as #1056 (comment)

func WithGoCollections(flags GoCollectionOption) func(options *prometheus.GoCollectorOptions) {
return func(o *goOptions) {
o.EnabledCollections = uint32(flags)
}
Expand Down
2 changes: 1 addition & 1 deletion prometheus/counter_test.go
Expand Up @@ -231,7 +231,7 @@ func TestCounterExemplar(t *testing.T) {
}
expectedExemplar := &dto.Exemplar{
Label: []*dto.LabelPair{
&dto.LabelPair{Name: proto.String("foo"), Value: proto.String("bar")},
{Name: proto.String("foo"), Value: proto.String("bar")},
},
Value: proto.Float64(42),
Timestamp: ts,
Expand Down
1 change: 1 addition & 0 deletions prometheus/desc.go
Expand Up @@ -20,6 +20,7 @@ import (
"strings"

"github.com/cespare/xxhash/v2"

"github.com/prometheus/client_golang/prometheus/internal"

//nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility.
Expand Down
1 change: 0 additions & 1 deletion prometheus/example_metricvec_test.go
Expand Up @@ -108,7 +108,6 @@ func (v *InfoVec) MustCurryWith(labels prometheus.Labels) *InfoVec {
}

func ExampleMetricVec() {

infoVec := NewInfoVec(
"library_version_info",
"Versions of the libraries used in this binary.",
Expand Down
40 changes: 19 additions & 21 deletions prometheus/example_timer_complex_test.go
Expand Up @@ -19,27 +19,25 @@ import (
"github.com/prometheus/client_golang/prometheus"
)

var (
// apiRequestDuration tracks the duration separate for each HTTP status
// class (1xx, 2xx, ...). This creates a fair amount of time series on
// the Prometheus server. Usually, you would track the duration of
// serving HTTP request without partitioning by outcome. Do something
// like this only if needed. Also note how only status classes are
// tracked, not every single status code. The latter would create an
// even larger amount of time series. Request counters partitioned by
// status code are usually OK as each counter only creates one time
// series. Histograms are way more expensive, so partition with care and
// only where you really need separate latency tracking. Partitioning by
// status class is only an example. In concrete cases, other partitions
// might make more sense.
apiRequestDuration = prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Name: "api_request_duration_seconds",
Help: "Histogram for the request duration of the public API, partitioned by status class.",
Buckets: prometheus.ExponentialBuckets(0.1, 1.5, 5),
},
[]string{"status_class"},
)
// apiRequestDuration tracks the duration separate for each HTTP status
// class (1xx, 2xx, ...). This creates a fair amount of time series on
// the Prometheus server. Usually, you would track the duration of
// serving HTTP request without partitioning by outcome. Do something
// like this only if needed. Also note how only status classes are
// tracked, not every single status code. The latter would create an
// even larger amount of time series. Request counters partitioned by
// status code are usually OK as each counter only creates one time
// series. Histograms are way more expensive, so partition with care and
// only where you really need separate latency tracking. Partitioning by
// status class is only an example. In concrete cases, other partitions
// might make more sense.
var apiRequestDuration = prometheus.NewHistogramVec(
prometheus.HistogramOpts{
Name: "api_request_duration_seconds",
Help: "Histogram for the request duration of the public API, partitioned by status class.",
Buckets: prometheus.ExponentialBuckets(0.1, 1.5, 5),
},
[]string{"status_class"},
)

func handler(w http.ResponseWriter, r *http.Request) {
Expand Down