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

chore: enable all rules of perfsprint linter #6164

Merged
merged 1 commit into from
Nov 7, 2024
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
8 changes: 3 additions & 5 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ issues:
text: "exitAfterDefer"
- linters: [gocritic]
text: "appendAssign"
- linters: [perfsprint]
text: "fmt.Sprintf can be replaced"
exclude-dirs-use-default: false
exclude-dirs:
- mocks
Expand Down Expand Up @@ -160,15 +158,15 @@ linters-settings:
- shadow
perfsprint:
# Optimizes even if it requires an int or uint type cast.
int-conversion: false
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: false
sprintf1: true
# Optimizes into strings concatenation.
strconcat: false
strconcat: true
revive:
ignore-generated-header: true
severity: error
Expand Down
3 changes: 1 addition & 2 deletions cmd/agent/app/reporter/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package reporter

import (
"flag"
"fmt"

"github.com/spf13/viper"
"go.uber.org/zap"
Expand Down Expand Up @@ -34,7 +33,7 @@ type Options struct {

// AddFlags adds flags for Options.
func AddFlags(flagSet *flag.FlagSet) {
flagSet.String(reporterType, string(GRPC), fmt.Sprintf("Reporter type to use e.g. %s", string(GRPC)))
flagSet.String(reporterType, string(GRPC), "Reporter type to use e.g. "+string(GRPC))
if !setupcontext.IsAllInOne() {
flagSet.String(agentTags, "", "One or more tags to be added to the Process tags of all spans passing through this agent. Ex: key1=value1,key2=${envVar:defaultValue}")
}
Expand Down
5 changes: 3 additions & 2 deletions cmd/collector/app/server/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net"
"net/http"
"net/http/httptest"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -207,7 +208,7 @@ func TestSpanCollectorHTTPS(t *testing.T) {
defer test.clientTLS.Close()
require.NoError(t, err0)
dialer := &net.Dialer{Timeout: 2 * time.Second}
conn, clientError := tls.DialWithDialer(dialer, "tcp", "localhost:"+fmt.Sprintf("%d", ports.CollectorHTTP), clientTLSCfg)
conn, clientError := tls.DialWithDialer(dialer, "tcp", "localhost:"+strconv.Itoa(ports.CollectorHTTP), clientTLSCfg)
var clientClose func() error
clientClose = nil
if conn != nil {
Expand All @@ -230,7 +231,7 @@ func TestSpanCollectorHTTPS(t *testing.T) {
},
}

response, requestError := client.Post("https://localhost:"+fmt.Sprintf("%d", ports.CollectorHTTP), "", nil)
response, requestError := client.Post("https://localhost:"+strconv.Itoa(ports.CollectorHTTP), "", nil)

if test.expectClientError {
require.Error(t, requestError)
Expand Down
6 changes: 3 additions & 3 deletions cmd/ingester/app/consumer/consumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package consumer

import (
"errors"
"fmt"
"strconv"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -163,7 +163,7 @@ func TestSaramaConsumerWrapper_start_Messages(t *testing.T) {

tags := map[string]string{
"topic": topic,
"partition": fmt.Sprint(partition),
"partition": strconv.Itoa(int(partition)),
}
localFactory.AssertCounterMetrics(t, metricstest.ExpectedMetric{
Name: "sarama-consumer.messages",
Expand Down Expand Up @@ -217,7 +217,7 @@ func TestSaramaConsumerWrapper_start_Errors(t *testing.T) {

tags := map[string]string{
"topic": topic,
"partition": fmt.Sprint(partition),
"partition": strconv.Itoa(int(partition)),
}
localFactory.AssertCounterMetrics(t, metricstest.ExpectedMetric{
Name: "sarama-consumer.errors",
Expand Down
4 changes: 2 additions & 2 deletions cmd/internal/status/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func flags(flagSet *flag.FlagSet, adminPort int) *flag.FlagSet {

func convert(httpHostPort string) string {
if strings.HasPrefix(httpHostPort, ":") {
return fmt.Sprintf("http://127.0.0.1%s", httpHostPort)
return "http://127.0.0.1" + httpHostPort
}
return fmt.Sprintf("http://%s", httpHostPort)
return "http://" + httpHostPort
}
8 changes: 4 additions & 4 deletions cmd/jaeger/internal/sanitizer/utf8_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var utf8EncodingTests = []struct {
key: invalidUTF8(),
value: "value",
expectedKey: "invalid-tag-key-1",
expectedValue: getBytesValueFromString(fmt.Sprintf("%s:value", invalidUTF8())),
expectedValue: getBytesValueFromString(invalidUTF8() + ":value"),
},
{
name: "valid key + invalid value",
Expand Down Expand Up @@ -264,7 +264,7 @@ func TestUTF8Sanitizer_SanitizesNonStringAttributeValueWithInvalidKey(t *testing
Attributes().
Get("invalid-tag-key-1")
require.True(t, ok)
require.EqualValues(t, getBytesValueFromString(fmt.Sprintf("%s:99", invalidUTF8())), value)
require.EqualValues(t, getBytesValueFromString(invalidUTF8()+":99"), value)
}

func TestUTF8Sanitizer_SanitizesMultipleAttributesWithInvalidKeys(t *testing.T) {
Expand All @@ -290,8 +290,8 @@ func TestUTF8Sanitizer_SanitizesMultipleAttributesWithInvalidKeys(t *testing.T)
require.Equal(t, 2, got.Len())

expectedValues := []pcommon.Value{
getBytesValueFromString(fmt.Sprintf("%s:v1", k1)),
getBytesValueFromString(fmt.Sprintf("%s:v2", k2)),
getBytesValueFromString(k1 + ":v1"),
getBytesValueFromString(k2 + ":v2"),
}
value, ok := got.
Get("invalid-tag-key-1")
Expand Down
6 changes: 1 addition & 5 deletions crossdock/services/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@

package services

import (
"fmt"
)

func getTracerServiceName(service string) string {
return fmt.Sprintf("crossdock-%s", service)
return "crossdock-" + service
}
4 changes: 2 additions & 2 deletions crossdock/services/tracehandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ package services
import (
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"strconv"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -414,7 +414,7 @@ func TestAdaptiveSamplingTestInternal(t *testing.T) {
}

for i, test := range tests {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
t.Run(strconv.Itoa(i), func(t *testing.T) {
query := &mocks.QueryService{}
agent := &mocks.AgentService{}

Expand Down
5 changes: 3 additions & 2 deletions examples/hotrod/pkg/log/spanlogger.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package log

import (
"fmt"
"strconv"
"time"

"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -138,11 +139,11 @@ func (e *bridgeFieldEncoder) AddTime(key string, value time.Time) {
}

func (e *bridgeFieldEncoder) AddUint(key string, value uint) {
e.pairs = append(e.pairs, attribute.String(key, fmt.Sprintf("%d", value)))
e.pairs = append(e.pairs, attribute.String(key, strconv.FormatUint(uint64(value), 10)))
Copy link
Member

Choose a reason for hiding this comment

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

why is this better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is more performant

}

func (e *bridgeFieldEncoder) AddUint64(key string, value uint64) {
e.pairs = append(e.pairs, attribute.String(key, fmt.Sprintf("%d", value)))
e.pairs = append(e.pairs, attribute.String(key, strconv.FormatUint(value, 10)))
Copy link
Member

@yurishkuro yurishkuro Nov 7, 2024

Choose a reason for hiding this comment

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

this looks way uglier than sprintf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand, this is more performant

}

func (e *bridgeFieldEncoder) AddUint32(key string, value uint32) {
Expand Down
3 changes: 2 additions & 1 deletion model/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package model_test
import (
"bytes"
"fmt"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -482,7 +483,7 @@ func TestGetSamplerParams(t *testing.T) {

for i, test := range tests {
tt := test
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
t.Run(strconv.Itoa(i), func(t *testing.T) {
span := &model.Span{}
span.Tags = tt.tags
actualType, actualParam := span.GetSamplerParams(logger)
Expand Down
2 changes: 1 addition & 1 deletion pkg/es/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (c *Client) request(esRequest elasticRequest) ([]byte, error) {

func (c *Client) setAuthorization(r *http.Request) {
if c.BasicAuth != "" {
r.Header.Add("Authorization", fmt.Sprintf("Basic %s", c.BasicAuth))
r.Header.Add("Authorization", "Basic "+c.BasicAuth)
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/es/client/ilm_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type ILMClient struct {
// Exists verify if a ILM policy exists
func (i ILMClient) Exists(name string) (bool, error) {
_, err := i.request(elasticRequest{
endpoint: fmt.Sprintf("_ilm/policy/%s", name),
endpoint: "_ilm/policy/" + name,
method: http.MethodGet,
})

Expand Down
16 changes: 8 additions & 8 deletions pkg/es/client/index_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (i *IndicesClient) GetJaegerIndices(prefix string) ([]Index, error) {
prefix += "jaeger-*"

body, err := i.request(elasticRequest{
endpoint: fmt.Sprintf("%s?flat_settings=true&filter_path=*.aliases,*.settings", prefix),
endpoint: prefix + "?flat_settings=true&filter_path=*.aliases,*.settings",
method: http.MethodGet,
})
if err != nil {
Expand Down Expand Up @@ -101,7 +101,7 @@ func (i *IndicesClient) indexDeleteRequest(concatIndices string) error {
var responseError ResponseError
if errors.As(err, &responseError) {
if responseError.StatusCode != http.StatusOK {
return responseError.prefixMessage(fmt.Sprintf("failed to delete indices: %s", concatIndices))
return responseError.prefixMessage("failed to delete indices: " + concatIndices)
}
}
return fmt.Errorf("failed to delete indices: %w", err)
Expand Down Expand Up @@ -146,7 +146,7 @@ func (i *IndicesClient) CreateIndex(index string) error {
var responseError ResponseError
if errors.As(err, &responseError) {
if responseError.StatusCode != http.StatusOK {
return responseError.prefixMessage(fmt.Sprintf("failed to create index: %s", index))
return responseError.prefixMessage("failed to create index: " + index)
}
}
return fmt.Errorf("failed to create index: %w", err)
Expand All @@ -161,7 +161,7 @@ func (i *IndicesClient) CreateAlias(aliases []Alias) error {
var responseError ResponseError
if errors.As(err, &responseError) {
if responseError.StatusCode != http.StatusOK {
return responseError.prefixMessage(fmt.Sprintf("failed to create aliases: %s", i.aliasesString(aliases)))
return responseError.prefixMessage("failed to create aliases: " + i.aliasesString(aliases))
}
}
return fmt.Errorf("failed to create aliases: %w", err)
Expand All @@ -176,7 +176,7 @@ func (i *IndicesClient) DeleteAlias(aliases []Alias) error {
var responseError ResponseError
if errors.As(err, &responseError) {
if responseError.StatusCode != http.StatusOK {
return responseError.prefixMessage(fmt.Sprintf("failed to delete aliases: %s", i.aliasesString(aliases)))
return responseError.prefixMessage("failed to delete aliases: " + i.aliasesString(aliases))
}
}
return fmt.Errorf("failed to delete aliases: %w", err)
Expand Down Expand Up @@ -247,7 +247,7 @@ func (i IndicesClient) CreateTemplate(template, name string) error {
var responseError ResponseError
if errors.As(err, &responseError) {
if responseError.StatusCode != http.StatusOK {
return responseError.prefixMessage(fmt.Sprintf("failed to create template: %s", name))
return responseError.prefixMessage("failed to create template: " + name)
}
}
return fmt.Errorf("failed to create template: %w", err)
Expand All @@ -258,7 +258,7 @@ func (i IndicesClient) CreateTemplate(template, name string) error {
// Rollover create a rollover for certain index/alias
func (i IndicesClient) Rollover(rolloverTarget string, conditions map[string]any) error {
esReq := elasticRequest{
endpoint: fmt.Sprintf("%s/_rollover/", rolloverTarget),
endpoint: rolloverTarget + "/_rollover/",
method: http.MethodPost,
}
if len(conditions) > 0 {
Expand All @@ -276,7 +276,7 @@ func (i IndicesClient) Rollover(rolloverTarget string, conditions map[string]any
var responseError ResponseError
if errors.As(err, &responseError) {
if responseError.StatusCode != http.StatusOK {
return responseError.prefixMessage(fmt.Sprintf("failed to create rollover target: %s", rolloverTarget))
return responseError.prefixMessage("failed to create rollover target: " + rolloverTarget)
}
}
return fmt.Errorf("failed to create rollover: %w", err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/fswatcher/fswatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package fswatcher

import (
"crypto/sha256"
"fmt"
"encoding/hex"
"io"
"os"
"path"
Expand Down Expand Up @@ -154,5 +154,5 @@ func hashFile(file string) (string, error) {
return "", err
}

return fmt.Sprintf("%x", h.Sum(nil)), nil
return hex.EncodeToString(h.Sum(nil)), nil
}
4 changes: 2 additions & 2 deletions pkg/testutils/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
package testutils

import (
"fmt"
"strconv"
"sync"
"testing"

Expand Down Expand Up @@ -79,7 +79,7 @@ func TestLogMatcher(t *testing.T) {
}
for i, tt := range tests {
test := tt
t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
t.Run(strconv.Itoa(i), func(t *testing.T) {
match, errMsg := LogMatcher(test.occurrences, test.subStr, test.logs)
assert.Equal(t, test.expected, match)
assert.Equal(t, test.errMsg, errMsg)
Expand Down
4 changes: 2 additions & 2 deletions plugin/sampling/leaderelection/leader_election_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ package leaderelection

import (
"errors"
"fmt"
"io"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -42,7 +42,7 @@ func TestAcquireLock(t *testing.T) {
}

for i, test := range tests {
t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
t.Run(strconv.Itoa(i), func(t *testing.T) {
logger, logBuffer := testutils.NewLogger()
mockLock := &lmocks.Lock{}
mockLock.On("Acquire", "sampling_lock", followerInterval).Return(test.acquiredLock, test.err)
Expand Down
5 changes: 2 additions & 3 deletions plugin/storage/badger/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package badger

import (
"expvar"
"fmt"
"io"
"os"
"testing"
Expand All @@ -24,8 +23,8 @@ func TestInitializationErrors(t *testing.T) {
f := NewFactory()
v, command := config.Viperize(f.AddFlags)
dir := "/root/this_should_fail" // If this test fails, you have some issues in your system
keyParam := fmt.Sprintf("--badger.directory-key=%s", dir)
valueParam := fmt.Sprintf("--badger.directory-value=%s", dir)
keyParam := "--badger.directory-key=" + dir
valueParam := "--badger.directory-value=" + dir

command.ParseFlags([]string{
"--badger.ephemeral=false",
Expand Down
8 changes: 4 additions & 4 deletions plugin/storage/badger/spanstore/read_write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,8 @@ func TestPersist(t *testing.T) {
cfg := badger.DefaultConfig()
v, command := config.Viperize(cfg.AddFlags)

keyParam := fmt.Sprintf("--badger.directory-key=%s", dir)
valueParam := fmt.Sprintf("--badger.directory-value=%s", dir)
keyParam := "--badger.directory-key=" + dir
valueParam := "--badger.directory-value=" + dir

command.ParseFlags([]string{
"--badger.ephemeral=false",
Expand Down Expand Up @@ -604,8 +604,8 @@ func runLargeFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Wr
dir := filepath.Join(tb.TempDir(), "badger-testRun")
err := os.MkdirAll(dir, 0o700)
assertion.NoError(err)
keyParam := fmt.Sprintf("--badger.directory-key=%s", dir)
valueParam := fmt.Sprintf("--badger.directory-value=%s", dir)
keyParam := "--badger.directory-key=" + dir
valueParam := "--badger.directory-value=" + dir

command.ParseFlags([]string{
"--badger.ephemeral=false",
Expand Down
Loading
Loading