Plugins: Optimize creation of Golang errors and slices (#69448)

* tidy up

* fix tests

pkg/plugins/envvars/envvars.go

@@ -64,15 +64,14 @@ func (s *Service) tracingEnvVars(plugin *plugins.Plugin) []string {
 		return nil
 	}
 
-	var vars []string
+	vars := []string{
+		fmt.Sprintf("GF_INSTANCE_OTLP_ADDRESS=%s", s.cfg.Tracing.OpenTelemetry.Address),
+		fmt.Sprintf("GF_INSTANCE_OTLP_PROPAGATION=%s", s.cfg.Tracing.OpenTelemetry.Propagation),
+	}
 	if plugin.Info.Version != "" {
 		vars = append(vars, fmt.Sprintf("GF_PLUGIN_VERSION=%s", plugin.Info.Version))
 	}
-	return append(
-		vars,
-		fmt.Sprintf("GF_INSTANCE_OTLP_ADDRESS=%s", s.cfg.Tracing.OpenTelemetry.Address),
-		fmt.Sprintf("GF_INSTANCE_OTLP_PROPAGATION=%s", s.cfg.Tracing.OpenTelemetry.Propagation),
-	)
+	return vars
 }
 
 func (s *Service) awsEnvVars() []string {
@@ -88,17 +87,17 @@ func (s *Service) awsEnvVars() []string {
 }
 
 func (s *Service) secureSocksProxyEnvVars() []string {
-	var variables []string
 	if s.cfg.ProxySettings.Enabled {
-		variables = append(variables, proxy.PluginSecureSocksProxyClientCert+"="+s.cfg.ProxySettings.ClientCert)
-		variables = append(variables, proxy.PluginSecureSocksProxyClientKey+"="+s.cfg.ProxySettings.ClientKey)
-		variables = append(variables, proxy.PluginSecureSocksProxyRootCACert+"="+s.cfg.ProxySettings.RootCA)
-		variables = append(variables, proxy.PluginSecureSocksProxyProxyAddress+"="+s.cfg.ProxySettings.ProxyAddress)
-		variables = append(variables, proxy.PluginSecureSocksProxyServerName+"="+s.cfg.ProxySettings.ServerName)
-		variables = append(variables, proxy.PluginSecureSocksProxyEnabled+"="+strconv.FormatBool(s.cfg.ProxySettings.Enabled))
+		return []string{
+			proxy.PluginSecureSocksProxyClientCert + "=" + s.cfg.ProxySettings.ClientCert,
+			proxy.PluginSecureSocksProxyClientKey + "=" + s.cfg.ProxySettings.ClientKey,
+			proxy.PluginSecureSocksProxyRootCACert + "=" + s.cfg.ProxySettings.RootCA,
+			proxy.PluginSecureSocksProxyProxyAddress + "=" + s.cfg.ProxySettings.ProxyAddress,
+			proxy.PluginSecureSocksProxyServerName + "=" + s.cfg.ProxySettings.ServerName,
+			proxy.PluginSecureSocksProxyEnabled + "=" + strconv.FormatBool(s.cfg.ProxySettings.Enabled),
+		}
 	}
-
-	return variables
+	return nil
 }
 
 type pluginSettings map[string]string

pkg/plugins/envvars/envvars_test.go

@@ -181,9 +181,9 @@ func TestInitializer_tracingEnvironmentVariables(t *testing.T) {
 				assert.Len(t, envVars, 5)
 				assert.Equal(t, "GF_PLUGIN_TRACING=true", envVars[0])
 				assert.Equal(t, "GF_VERSION=", envVars[1])
-				assert.Equal(t, "GF_PLUGIN_VERSION=1.0.0", envVars[2])
-				assert.Equal(t, "GF_INSTANCE_OTLP_ADDRESS=127.0.0.1:4317", envVars[3])
-				assert.Equal(t, "GF_INSTANCE_OTLP_PROPAGATION=w3c", envVars[4])
+				assert.Equal(t, "GF_INSTANCE_OTLP_ADDRESS=127.0.0.1:4317", envVars[2])
+				assert.Equal(t, "GF_INSTANCE_OTLP_PROPAGATION=w3c", envVars[3])
+				assert.Equal(t, "GF_PLUGIN_VERSION=1.0.0", envVars[4])
 			},
 		},
 		{
@@ -204,9 +204,9 @@ func TestInitializer_tracingEnvironmentVariables(t *testing.T) {
 				assert.Len(t, envVars, 5)
 				assert.Equal(t, "GF_PLUGIN_TRACING=true", envVars[0])
 				assert.Equal(t, "GF_VERSION=", envVars[1])
-				assert.Equal(t, "GF_PLUGIN_VERSION=1.0.0", envVars[2])
-				assert.Equal(t, "GF_INSTANCE_OTLP_ADDRESS=127.0.0.1:4317", envVars[3])
-				assert.Equal(t, "GF_INSTANCE_OTLP_PROPAGATION=w3c,jaeger", envVars[4])
+				assert.Equal(t, "GF_INSTANCE_OTLP_ADDRESS=127.0.0.1:4317", envVars[2])
+				assert.Equal(t, "GF_INSTANCE_OTLP_PROPAGATION=w3c,jaeger", envVars[3])
+				assert.Equal(t, "GF_PLUGIN_VERSION=1.0.0", envVars[4])
 			},
 		},
 		{

pkg/plugins/manager/client/client.go

@@ -25,6 +25,11 @@ const (
 
 var _ plugins.Client = (*Service)(nil)
 
+var (
+	errNilRequest = errors.New("req cannot be nil")
+	errNilSender  = errors.New("sender cannot be nil")
+)
+
 type Service struct {
 	pluginRegistry registry.Service
 	cfg            *config.Cfg
@@ -39,7 +44,7 @@ func ProvideService(pluginRegistry registry.Service, cfg *config.Cfg) *Service {
 
 func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	p, exists := s.plugin(ctx, req.PluginContext.PluginID)
@@ -87,11 +92,11 @@ func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest)
 
 func (s *Service) CallResource(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error {
 	if req == nil {
-		return fmt.Errorf("req cannot be nil")
+		return errNilRequest
 	}
 
 	if sender == nil {
-		return fmt.Errorf("sender cannot be nil")
+		return errNilSender
 	}
 
 	p, exists := s.plugin(ctx, req.PluginContext.PluginID)
@@ -140,7 +145,7 @@ func (s *Service) CallResource(ctx context.Context, req *backend.CallResourceReq
 
 func (s *Service) CollectMetrics(ctx context.Context, req *backend.CollectMetricsRequest) (*backend.CollectMetricsResult, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	p, exists := s.plugin(ctx, req.PluginContext.PluginID)
@@ -165,7 +170,7 @@ func (s *Service) CollectMetrics(ctx context.Context, req *backend.CollectMetric
 
 func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	p, exists := s.plugin(ctx, req.PluginContext.PluginID)
@@ -199,7 +204,7 @@ func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthReque
 
 func (s *Service) SubscribeStream(ctx context.Context, req *backend.SubscribeStreamRequest) (*backend.SubscribeStreamResponse, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	plugin, exists := s.plugin(ctx, req.PluginContext.PluginID)
@@ -212,7 +217,7 @@ func (s *Service) SubscribeStream(ctx context.Context, req *backend.SubscribeStr
 
 func (s *Service) PublishStream(ctx context.Context, req *backend.PublishStreamRequest) (*backend.PublishStreamResponse, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	plugin, exists := s.plugin(ctx, req.PluginContext.PluginID)
@@ -225,11 +230,11 @@ func (s *Service) PublishStream(ctx context.Context, req *backend.PublishStreamR
 
 func (s *Service) RunStream(ctx context.Context, req *backend.RunStreamRequest, sender *backend.StreamSender) error {
 	if req == nil {
-		return fmt.Errorf("req cannot be nil")
+		return errNilRequest
 	}
 
 	if sender == nil {
-		return fmt.Errorf("sender cannot be nil")
+		return errNilSender
 	}
 
 	plugin, exists := s.plugin(ctx, req.PluginContext.PluginID)

pkg/plugins/manager/client/decorator.go

@@ -2,7 +2,7 @@ package client
 
 import (
 	"context"
-	"fmt"
+	"errors"
 
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana/pkg/plugins"
@@ -17,7 +17,7 @@ type Decorator struct {
 // NewDecorator creates a new plugins.client decorator.
 func NewDecorator(client plugins.Client, middlewares ...plugins.ClientMiddleware) (*Decorator, error) {
 	if client == nil {
-		return nil, fmt.Errorf("client cannot be nil")
+		return nil, errors.New("client cannot be nil")
 	}
 
 	return &Decorator{
@@ -28,7 +28,7 @@ func NewDecorator(client plugins.Client, middlewares ...plugins.ClientMiddleware
 
 func (d *Decorator) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	client := clientFromMiddlewares(d.middlewares, d.client)
@@ -37,11 +37,11 @@ func (d *Decorator) QueryData(ctx context.Context, req *backend.QueryDataRequest
 
 func (d *Decorator) CallResource(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error {
 	if req == nil {
-		return fmt.Errorf("req cannot be nil")
+		return errNilRequest
 	}
 
 	if sender == nil {
-		return fmt.Errorf("sender cannot be nil")
+		return errors.New("sender cannot be nil")
 	}
 
 	client := clientFromMiddlewares(d.middlewares, d.client)
@@ -50,7 +50,7 @@ func (d *Decorator) CallResource(ctx context.Context, req *backend.CallResourceR
 
 func (d *Decorator) CollectMetrics(ctx context.Context, req *backend.CollectMetricsRequest) (*backend.CollectMetricsResult, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	client := clientFromMiddlewares(d.middlewares, d.client)
@@ -59,7 +59,7 @@ func (d *Decorator) CollectMetrics(ctx context.Context, req *backend.CollectMetr
 
 func (d *Decorator) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	client := clientFromMiddlewares(d.middlewares, d.client)
@@ -68,7 +68,7 @@ func (d *Decorator) CheckHealth(ctx context.Context, req *backend.CheckHealthReq
 
 func (d *Decorator) SubscribeStream(ctx context.Context, req *backend.SubscribeStreamRequest) (*backend.SubscribeStreamResponse, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	client := clientFromMiddlewares(d.middlewares, d.client)
@@ -77,7 +77,7 @@ func (d *Decorator) SubscribeStream(ctx context.Context, req *backend.SubscribeS
 
 func (d *Decorator) PublishStream(ctx context.Context, req *backend.PublishStreamRequest) (*backend.PublishStreamResponse, error) {
 	if req == nil {
-		return nil, fmt.Errorf("req cannot be nil")
+		return nil, errNilRequest
 	}
 
 	client := clientFromMiddlewares(d.middlewares, d.client)
@@ -86,11 +86,11 @@ func (d *Decorator) PublishStream(ctx context.Context, req *backend.PublishStrea
 
 func (d *Decorator) RunStream(ctx context.Context, req *backend.RunStreamRequest, sender *backend.StreamSender) error {
 	if req == nil {
-		return fmt.Errorf("req cannot be nil")
+		return errNilRequest
 	}
 
 	if sender == nil {
-		return fmt.Errorf("sender cannot be nil")
+		return errors.New("sender cannot be nil")
 	}
 
 	client := clientFromMiddlewares(d.middlewares, d.client)

pkg/plugins/manager/dashboards/filestore.go

@@ -2,7 +2,7 @@ package dashboards
 
 import (
 	"context"
-	"fmt"
+	"errors"
 	"strings"
 
 	"github.com/grafana/grafana/pkg/plugins"
@@ -34,11 +34,11 @@ var openDashboardFile = func(ctx context.Context, pluginFileStore plugins.FileSt
 
 func (m *FileStoreManager) ListPluginDashboardFiles(ctx context.Context, args *ListPluginDashboardFilesArgs) (*ListPluginDashboardFilesResult, error) {
 	if args == nil {
-		return nil, fmt.Errorf("args cannot be nil")
+		return nil, errors.New("args cannot be nil")
 	}
 
 	if len(strings.TrimSpace(args.PluginID)) == 0 {
-		return nil, fmt.Errorf("args.PluginID cannot be empty")
+		return nil, errors.New("args.PluginID cannot be empty")
 	}
 
 	plugin, exists := m.pluginStore.Plugin(ctx, args.PluginID)
@@ -58,15 +58,15 @@ func (m *FileStoreManager) ListPluginDashboardFiles(ctx context.Context, args *L
 
 func (m *FileStoreManager) GetPluginDashboardFileContents(ctx context.Context, args *GetPluginDashboardFileContentsArgs) (*GetPluginDashboardFileContentsResult, error) {
 	if args == nil {
-		return nil, fmt.Errorf("args cannot be nil")
+		return nil, errors.New("args cannot be nil")
 	}
 
 	if len(strings.TrimSpace(args.PluginID)) == 0 {
-		return nil, fmt.Errorf("args.PluginID cannot be empty")
+		return nil, errors.New("args.PluginID cannot be empty")
 	}
 
 	if len(strings.TrimSpace(args.FileReference)) == 0 {
-		return nil, fmt.Errorf("args.FileReference cannot be empty")
+		return nil, errors.New("args.FileReference cannot be empty")
 	}
 
 	plugin, exists := m.pluginStore.Plugin(ctx, args.PluginID)
@@ -83,7 +83,7 @@ func (m *FileStoreManager) GetPluginDashboardFileContents(ctx context.Context, a
 	}
 
 	if includedFile == nil {
-		return nil, fmt.Errorf("plugin dashboard file not found")
+		return nil, errors.New("plugin dashboard file not found")
 	}
 
 	cleanPath, err := util.CleanRelativePath(includedFile.Path)

pkg/plugins/manager/fakes/fakes.go

@@ -180,8 +180,7 @@ func (f *FakePluginRegistry) Plugin(_ context.Context, id string) (*plugins.Plug
 }
 
 func (f *FakePluginRegistry) Plugins(_ context.Context) []*plugins.Plugin {
-	var res []*plugins.Plugin
-
+	res := make([]*plugins.Plugin, 0, len(f.Store))
 	for _, p := range f.Store {
 		res = append(res, p)
 	}

pkg/plugins/manager/loader/finder/local.go

@@ -44,8 +44,9 @@ func (l *Local) Find(ctx context.Context, src plugins.PluginSource) ([]*plugins.
 		return []*plugins.FoundBundle{}, nil
 	}
 
-	var pluginJSONPaths []string
-	for _, path := range src.PluginURIs(ctx) {
+	pluginURIs := src.PluginURIs(ctx)
+	pluginJSONPaths := make([]string, 0, len(pluginURIs))
+	for _, path := range pluginURIs {
 		exists, err := fs.Exists(path)
 		if err != nil {
 			l.log.Warn("Skipping finding plugins as an error occurred", "path", path, "err", err)
@@ -85,7 +86,7 @@ func (l *Local) Find(ctx context.Context, src plugins.PluginSource) ([]*plugins.
 		foundPlugins[filepath.Dir(pluginJSONAbsPath)] = plugin
 	}
 
-	var res = make(map[string]*plugins.FoundBundle)
+	res := make(map[string]*plugins.FoundBundle)
 	for pluginDir, data := range foundPlugins {
 		var pluginFs plugins.FS
 		pluginFs = plugins.NewLocalFS(pluginDir)
@@ -106,7 +107,7 @@ func (l *Local) Find(ctx context.Context, src plugins.PluginSource) ([]*plugins.
 		}
 	}
 
-	var result []*plugins.FoundBundle
+	result := make([]*plugins.FoundBundle, 0, len(foundPlugins))
 	for dir := range foundPlugins {
 		ancestors := strings.Split(dir, string(filepath.Separator))
 		ancestors = ancestors[0 : len(ancestors)-1]

pkg/plugins/manager/loader/finder/local_test.go

@@ -3,7 +3,6 @@ package finder
 import (
 	"context"
 	"errors"
-	"fmt"
 	"os"
 	"path/filepath"
 	"sort"
@@ -317,7 +316,7 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) {
 	t.Run("When scanning a folder that returns a non-handled error should return that error", func(t *testing.T) {
 		origWalk := walk
 		walk = func(path string, followSymlinks, detectSymlinkInfiniteLoop bool, walkFn util.WalkFunc) error {
-			return walkFn(path, nil, fmt.Errorf("random error"))
+			return walkFn(path, nil, errors.New("random error"))
 		}
 		t.Cleanup(func() {
 			walk = origWalk

pkg/plugins/manager/loader/loader.go

@@ -81,7 +81,7 @@ func (l *Loader) Load(ctx context.Context, src plugins.PluginSource) ([]*plugins
 
 // nolint:gocyclo
 func (l *Loader) loadPlugins(ctx context.Context, src plugins.PluginSource, found []*plugins.FoundBundle) ([]*plugins.Plugin, error) {
-	var loadedPlugins []*plugins.Plugin
+	loadedPlugins := make([]*plugins.Plugin, 0, len(found))
 
 	for _, p := range found {
 		if _, exists := l.pluginRegistry.Plugin(ctx, p.Primary.JSONData.ID); exists {
@@ -129,7 +129,7 @@ func (l *Loader) loadPlugins(ctx context.Context, src plugins.PluginSource, foun
 	}
 
 	// validate signatures
-	verifiedPlugins := make([]*plugins.Plugin, 0)
+	verifiedPlugins := make([]*plugins.Plugin, 0, len(loadedPlugins))
 	for _, plugin := range loadedPlugins {
 		signingError := l.signatureValidator.Validate(plugin)
 		if signingError != nil {
@@ -180,7 +180,7 @@ func (l *Loader) loadPlugins(ctx context.Context, src plugins.PluginSource, foun
 	}
 
 	// initialize plugins
-	initializedPlugins := make([]*plugins.Plugin, 0)
+	initializedPlugins := make([]*plugins.Plugin, 0, len(verifiedPlugins))
 	for _, p := range verifiedPlugins {
 		// Detect angular for external plugins
 		if p.IsExternalPlugin() {
@@ -360,7 +360,7 @@ func defaultLogoPath(pluginType plugins.Type) string {
 }
 
 func (l *Loader) PluginErrors() []*plugins.Error {
-	errs := make([]*plugins.Error, 0)
+	errs := make([]*plugins.Error, 0, len(l.errs))
 	for _, err := range l.errs {
 		errs = append(errs, &plugins.Error{
 			PluginID:  err.PluginID,

pkg/plugins/manager/loader/loader_test.go

@@ -2,7 +2,7 @@ package loader
 
 import (
 	"context"
-	"fmt"
+	"errors"
 	"path/filepath"
 	"sort"
 	"testing"
@@ -1048,7 +1048,7 @@ func TestLoader_Load_SkipUninitializedPlugins(t *testing.T) {
 		procPrvdr.BackendFactoryFunc = func(ctx context.Context, p *plugins.Plugin) backendplugin.PluginFactoryFunc {
 			return func(pluginID string, _ log.Logger, _ []string) (backendplugin.Plugin, error) {
 				if pluginID == "test-datasource" {
-					return nil, fmt.Errorf("failed to initialize")
+					return nil, errors.New("failed to initialize")
 				}
 				return &fakes.FakePluginClient{}, nil
 			}

pkg/plugins/manager/registry/in_memory.go

@@ -33,7 +33,7 @@ func (i *InMemory) Plugins(_ context.Context) []*plugins.Plugin {
 	i.mu.RLock()
 	defer i.mu.RUnlock()
 
-	res := make([]*plugins.Plugin, 0)
+	res := make([]*plugins.Plugin, 0, len(i.store))
 	for _, p := range i.store {
 		res = append(res, p)
 	}

pkg/plugins/manager/store/store.go

@@ -96,8 +96,10 @@ func (s *Service) plugin(ctx context.Context, pluginID string) (*plugins.Plugin,
 
 // availablePlugins returns all non-decommissioned plugins from the registry sorted by alphabetic order on `plugin.ID`
 func (s *Service) availablePlugins(ctx context.Context) []*plugins.Plugin {
-	var res []*plugins.Plugin
-	for _, p := range s.pluginRegistry.Plugins(ctx) {
+	ps := s.pluginRegistry.Plugins(ctx)
+
+	res := make([]*plugins.Plugin, 0, len(ps))
+	for _, p := range ps {
 		if !p.IsDecommissioned() {
 			res = append(res, p)
 		}

pkg/plugins/repo/client.go

@@ -7,6 +7,7 @@ import (
 	"crypto/sha256"
 	"crypto/tls"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -129,7 +130,7 @@ func (c *Client) downloadFile(tmpFile *os.File, pluginURL, checksum string, comp
 				c.retryCount = 0
 				failure := fmt.Sprintf("%v", r)
 				if failure == "runtime error: makeslice: len out of range" {
-					err = fmt.Errorf("corrupt HTTP response from source, please try again")
+					err = errors.New("corrupt HTTP response from source, please try again")
 				} else {
 					panic(r)
 				}