Skip to content

Commit

Permalink
[chore]: enable perfsprint linter (open-telemetry#11599)
Browse files Browse the repository at this point in the history
#### Description

[perfsprint](https://golangci-lint.run/usage/linters/#perfsprint) checks
that fmt.Sprintf can be replaced with a faster alternative.

Signed-off-by: Matthieu MOREL <[email protected]>
  • Loading branch information
mmorel-35 authored and djaglowski committed Nov 21, 2024
1 parent b27374c commit d5e27f4
Show file tree
Hide file tree
Showing 29 changed files with 87 additions and 65 deletions.
13 changes: 13 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,18 @@ linters-settings:
- kilometre
- kilometres

perfsprint:
# Optimizes even if it requires an int or uint type cast.
int-conversion: true
# Optimizes into `err.Error()` even if it is only equivalent for non-nil errors.
err-error: true
# Optimizes `fmt.Errorf`.
errorf: true
# Optimizes `fmt.Sprintf` with only one argument.
sprintf1: true
# Optimizes into strings concatenation.
strconcat: true

depguard:
rules:
denied-deps:
Expand Down Expand Up @@ -136,6 +148,7 @@ linters:
- gosec
- govet
- misspell
- perfsprint
- revive
- staticcheck
- tenv
Expand Down
5 changes: 3 additions & 2 deletions cmd/mdatagen/internal/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"regexp"
"strconv"
"strings"

"go.opentelemetry.io/collector/filter"
Expand Down Expand Up @@ -272,11 +273,11 @@ func (a Attribute) TestValue() string {
case pcommon.ValueTypeStr:
return fmt.Sprintf(`"%s-val"`, a.FullName)
case pcommon.ValueTypeInt:
return fmt.Sprintf("%d", len(a.FullName))
return strconv.Itoa(len(a.FullName))
case pcommon.ValueTypeDouble:
return fmt.Sprintf("%f", 0.1+float64(len(a.FullName)))
case pcommon.ValueTypeBool:
return fmt.Sprintf("%t", len(a.FullName)%2 == 0)
return strconv.FormatBool(len(a.FullName)%2 == 0)
case pcommon.ValueTypeMap:
return fmt.Sprintf(`map[string]any{"key1": "%s-val1", "key2": "%s-val2"}`, a.FullName, a.FullName)
case pcommon.ValueTypeSlice:
Expand Down
10 changes: 5 additions & 5 deletions component/componenttest/otelchecker.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ func checkReceiverMetrics(reader *sdkmetric.ManualReader, receiver component.ID,
func checkReceiver(reader *sdkmetric.ManualReader, receiver component.ID, datatype, protocol string, acceptedMetricPoints, droppedMetricPoints int64) error {
receiverAttrs := attributesForReceiverMetrics(receiver, protocol)
return multierr.Combine(
checkIntSum(reader, fmt.Sprintf("otelcol_receiver_accepted_%s", datatype), acceptedMetricPoints, receiverAttrs),
checkIntSum(reader, fmt.Sprintf("otelcol_receiver_refused_%s", datatype), droppedMetricPoints, receiverAttrs))
checkIntSum(reader, "otelcol_receiver_accepted_"+datatype, acceptedMetricPoints, receiverAttrs),
checkIntSum(reader, "otelcol_receiver_refused_"+datatype, droppedMetricPoints, receiverAttrs))
}

func checkExporterTraces(reader *sdkmetric.ManualReader, exporter component.ID, sent, sendFailed int64) error {
Expand All @@ -55,10 +55,10 @@ func checkExporterMetrics(reader *sdkmetric.ManualReader, exporter component.ID,

func checkExporter(reader *sdkmetric.ManualReader, exporter component.ID, datatype string, sent, sendFailed int64) error {
exporterAttrs := attributesForExporterMetrics(exporter)
errs := checkIntSum(reader, fmt.Sprintf("otelcol_exporter_sent_%s", datatype), sent, exporterAttrs)
errs := checkIntSum(reader, "otelcol_exporter_sent_"+datatype, sent, exporterAttrs)
if sendFailed > 0 {
errs = multierr.Append(errs,
checkIntSum(reader, fmt.Sprintf("otelcol_exporter_send_failed_%s", datatype), sendFailed, exporterAttrs))
checkIntSum(reader, "otelcol_exporter_send_failed_"+datatype, sendFailed, exporterAttrs))
}
return errs
}
Expand All @@ -68,7 +68,7 @@ func checkExporterEnqueueFailed(reader *sdkmetric.ManualReader, exporter compone
return nil
}
exporterAttrs := attributesForExporterMetrics(exporter)
return checkIntSum(reader, fmt.Sprintf("otelcol_exporter_enqueue_failed_%s", datatype), enqueueFailed, exporterAttrs)
return checkIntSum(reader, "otelcol_exporter_enqueue_failed_"+datatype, enqueueFailed, exporterAttrs)
}

func checkIntGauge(reader *sdkmetric.ManualReader, metric string, expected int64, expectedAttrs attribute.Set) error {
Expand Down
2 changes: 1 addition & 1 deletion component/identifiable.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (t Type) MarshalText() ([]byte, error) {
// - can only contain ASCII alphanumeric characters and '_'.
func NewType(ty string) (Type, error) {
if len(ty) == 0 {
return Type{}, fmt.Errorf("id must not be empty")
return Type{}, errors.New("id must not be empty")
}
if !typeRegexp.MatchString(ty) {
return Type{}, fmt.Errorf("invalid character(s) in type %q", ty)
Expand Down
3 changes: 1 addition & 2 deletions config/confighttp/compression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"compress/zlib"
"context"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -335,7 +334,7 @@ type closeFailBody struct {
}

func (*closeFailBody) Close() error {
return fmt.Errorf("close failed")
return errors.New("close failed")
}

func TestHTTPContentCompressionRequestBodyCloseError(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion config/confighttp/compressor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func BenchmarkCompression(b *testing.B) {

for i := range benchmarks {
benchmark := &benchmarks[i]
b.Run(fmt.Sprint(benchmark.name), func(b *testing.B) {
b.Run(benchmark.name, func(b *testing.B) {
benchmark.function(b, benchmark.codec, &buffer, payload)
})
}
Expand Down
7 changes: 4 additions & 3 deletions config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"net/http/httptest"
"net/url"
"path/filepath"
"strconv"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -813,7 +814,7 @@ func TestHttpCors(t *testing.T) {
_ = s.Serve(ln)
}()

url := fmt.Sprintf("http://%s", ln.Addr().String())
url := "http://" + ln.Addr().String()

expectedStatus := http.StatusNoContent
if tt.CORSConfig == nil || len(tt.AllowedOrigins) == 0 {
Expand Down Expand Up @@ -933,7 +934,7 @@ func TestHttpServerHeaders(t *testing.T) {
_ = s.Serve(ln)
}()

url := fmt.Sprintf("http://%s", ln.Addr().String())
url := "http://" + ln.Addr().String()

// Verify allowed domain gets responses that allow CORS.
verifyHeadersResp(t, url, tt.headers)
Expand Down Expand Up @@ -973,7 +974,7 @@ func verifyCorsResp(t *testing.T, url string, origin string, set *CORSConfig, ex
wantAllowOrigin = origin
wantAllowMethods = "POST"
if set != nil && set.MaxAge != 0 {
wantMaxAge = fmt.Sprintf("%d", set.MaxAge)
wantMaxAge = strconv.Itoa(set.MaxAge)
}
}
assert.Equal(t, wantAllowOrigin, gotAllowOrigin)
Expand Down
3 changes: 2 additions & 1 deletion config/configopaque/opaque_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package configopaque // import "go.opentelemetry.io/collector/config/configopaqu

import (
"encoding"
"encoding/hex"
"encoding/json"
"fmt"
"testing"
Expand Down Expand Up @@ -69,7 +70,7 @@ func TestStringFmt(t *testing.T) {
case "%q", "%#v":
expected = "\"" + string(example) + "\""
case "%x":
expected = fmt.Sprintf("%x", []byte(example))
expected = hex.EncodeToString([]byte(example))
default:
t.Errorf("unexpected verb %q", verb)
}
Expand Down
5 changes: 3 additions & 2 deletions config/configtls/clientcasfilereloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package configtls // import "go.opentelemetry.io/collector/config/configtls"
import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"sync"

Expand Down Expand Up @@ -78,7 +79,7 @@ func (r *clientCAsFileReloader) getLastError() error {

func (r *clientCAsFileReloader) startWatching() error {
if r.shutdownCH != nil {
return fmt.Errorf("client CA file watcher already started")
return errors.New("client CA file watcher already started")
}

watcher, err := fsnotify.NewWatcher()
Expand Down Expand Up @@ -132,7 +133,7 @@ func (r *clientCAsFileReloader) handleWatcherEvents() {

func (r *clientCAsFileReloader) shutdown() error {
if r.shutdownCH == nil {
return fmt.Errorf("client CAs file watcher is not running")
return errors.New("client CAs file watcher is not running")
}
r.shutdownCH <- true
close(r.shutdownCH)
Expand Down
2 changes: 1 addition & 1 deletion config/configtls/clientcasfilereloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func TestErrorRecordedIfFileDeleted(t *testing.T) {
}, 5*time.Second, 10*time.Millisecond)

lastErr := reloader.getLastError()
assert.Equal(t, "test error on reload", fmt.Sprint(lastErr))
require.EqualError(t, lastErr, "test error on reload")

err = reloader.shutdown()
assert.NoError(t, err)
Expand Down
12 changes: 6 additions & 6 deletions config/configtls/configtls.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (r *certReloader) GetCertificate() (*tls.Certificate, error) {

func (c Config) Validate() error {
if c.hasCAFile() && c.hasCAPem() {
return fmt.Errorf("provide either a CA file or the PEM-encoded string, but not both")
return errors.New("provide either a CA file or the PEM-encoded string, but not both")
}

minTLS, err := convertVersion(c.MinVersion, defaultMinTLSVersion)
Expand Down Expand Up @@ -269,7 +269,7 @@ func (c Config) loadCACertPool() (*x509.CertPool, error) {

switch {
case c.hasCAFile() && c.hasCAPem():
return nil, fmt.Errorf("failed to load CA CertPool: provide either a CA file or the PEM-encoded string, but not both")
return nil, errors.New("failed to load CA CertPool: provide either a CA file or the PEM-encoded string, but not both")
case c.hasCAFile():
// Set up user specified truststore from file
certPool, err = c.loadCertFile(c.CAFile)
Expand Down Expand Up @@ -308,21 +308,21 @@ func (c Config) loadCertPem(certPem []byte) (*x509.CertPool, error) {
}
}
if !certPool.AppendCertsFromPEM(certPem) {
return nil, fmt.Errorf("failed to parse cert")
return nil, errors.New("failed to parse cert")
}
return certPool, nil
}

func (c Config) loadCertificate() (tls.Certificate, error) {
switch {
case c.hasCert() != c.hasKey():
return tls.Certificate{}, fmt.Errorf("for auth via TLS, provide both certificate and key, or neither")
return tls.Certificate{}, errors.New("for auth via TLS, provide both certificate and key, or neither")
case !c.hasCert() && !c.hasKey():
return tls.Certificate{}, nil
case c.hasCertFile() && c.hasCertPem():
return tls.Certificate{}, fmt.Errorf("for auth via TLS, provide either a certificate or the PEM-encoded string, but not both")
return tls.Certificate{}, errors.New("for auth via TLS, provide either a certificate or the PEM-encoded string, but not both")
case c.hasKeyFile() && c.hasKeyPem():
return tls.Certificate{}, fmt.Errorf("for auth via TLS, provide either a key or the PEM-encoded string, but not both")
return tls.Certificate{}, errors.New("for auth via TLS, provide either a key or the PEM-encoded string, but not both")
}

var certPem, keyPem []byte
Expand Down
2 changes: 1 addition & 1 deletion confmap/confmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (l *Conf) Marshal(rawVal any, _ ...MarshalOption) error {
}
out, ok := data.(map[string]any)
if !ok {
return fmt.Errorf("invalid config encoding")
return errors.New("invalid config encoding")
}
return l.Merge(NewFromStringMap(out))
}
Expand Down
3 changes: 2 additions & 1 deletion connector/internal/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package internal // import "go.opentelemetry.io/collector/connector/internal"

import (
"errors"
"fmt"

"go.uber.org/multierr"
Expand Down Expand Up @@ -35,7 +36,7 @@ func (r *BaseRouter[T]) PipelineIDs() []pipeline.ID {
func (r *BaseRouter[T]) Consumer(pipelineIDs ...pipeline.ID) (T, error) {
var ret T
if len(pipelineIDs) == 0 {
return ret, fmt.Errorf("missing consumers")
return ret, errors.New("missing consumers")
}
consumers := make([]T, 0, len(pipelineIDs))
var errors error
Expand Down
3 changes: 2 additions & 1 deletion connector/logs_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package connector // import "go.opentelemetry.io/collector/connector"

import (
"errors"
"fmt"

"go.uber.org/multierr"
Expand Down Expand Up @@ -48,7 +49,7 @@ func (r *logsRouter) PipelineIDs() []pipeline.ID {

func (r *logsRouter) Consumer(pipelineIDs ...pipeline.ID) (consumer.Logs, error) {
if len(pipelineIDs) == 0 {
return nil, fmt.Errorf("missing consumers")
return nil, errors.New("missing consumers")
}
consumers := make([]consumer.Logs, 0, len(pipelineIDs))
var errors error
Expand Down
3 changes: 2 additions & 1 deletion exporter/debugexporter/internal/normal/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package normal // import "go.opentelemetry.io/collector/exporter/debugexporter/i
import (
"bytes"
"fmt"
"strconv"
"strings"

"go.opentelemetry.io/collector/pdata/pmetric"
Expand Down Expand Up @@ -60,7 +61,7 @@ func writeNumberDataPoints(metric pmetric.Metric, dataPoints pmetric.NumberDataP
var value string
switch dataPoint.ValueType() {
case pmetric.NumberDataPointValueTypeInt:
value = fmt.Sprintf("%v", dataPoint.IntValue())
value = strconv.FormatInt(dataPoint.IntValue(), 10)
case pmetric.NumberDataPointValueTypeDouble:
value = fmt.Sprintf("%v", dataPoint.DoubleValue())
}
Expand Down
6 changes: 3 additions & 3 deletions exporter/exporterhelper/internal/base_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package internal // import "go.opentelemetry.io/collector/exporter/exporterhelpe

import (
"context"
"fmt"
"errors"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -214,7 +214,7 @@ func WithRetry(config configretry.BackOffConfig) Option {
func WithQueue(config QueueConfig) Option {
return func(o *BaseExporter) error {
if o.Marshaler == nil || o.Unmarshaler == nil {
return fmt.Errorf("WithQueue option is not available for the new request exporters, use WithRequestQueue instead")
return errors.New("WithQueue option is not available for the new request exporters, use WithRequestQueue instead")
}
if !config.Enabled {
o.ExportFailureMessage += " Try enabling sending_queue to survive temporary failures."
Expand All @@ -240,7 +240,7 @@ func WithQueue(config QueueConfig) Option {
func WithRequestQueue(cfg exporterqueue.Config, queueFactory exporterqueue.Factory[internal.Request]) Option {
return func(o *BaseExporter) error {
if o.Marshaler != nil || o.Unmarshaler != nil {
return fmt.Errorf("WithRequestQueue option must be used with the new request exporters only, use WithQueue instead")
return errors.New("WithRequestQueue option must be used with the new request exporters only, use WithQueue instead")
}
if !cfg.Enabled {
o.ExportFailureMessage += " Try enabling sending_queue to survive temporary failures."
Expand Down
4 changes: 2 additions & 2 deletions exporter/internal/queue/persistent_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -802,8 +802,8 @@ func TestPersistentQueue_StorageFull(t *testing.T) {
}

func TestPersistentQueue_ItemDispatchingFinish_ErrorHandling(t *testing.T) {
errDeletingItem := fmt.Errorf("error deleting item")
errUpdatingDispatched := fmt.Errorf("error updating dispatched items")
errDeletingItem := errors.New("error deleting item")
errUpdatingDispatched := errors.New("error updating dispatched items")
testCases := []struct {
storageErrors []error
expectedError error
Expand Down
Loading

0 comments on commit d5e27f4

Please sign in to comment.