Datasource: Improve default timeout settings for HTTP client provider (#36621)

Make sure that default timeout settings are based on configuration
parameters. This now applies for core data sources using old TSDB
contracts and new SDK contracts. Before it was only applied for old TSDB
contracts.
Also moves global setting variables to non-global (setting.Cfg).

pkg/api/pluginproxy/ds_auth_provider.go

@@ -64,7 +64,7 @@ func ApplyRoute(ctx context.Context, req *http.Request, proxyPath string, route
 		}
 	}
 
-	if setting.DataProxyLogging {
+	if cfg.DataProxyLogging {
 		logger.Debug("Requesting", "url", req.URL.String())
 	}
 }

pkg/api/pluginproxy/ds_proxy.go

@@ -304,7 +304,7 @@ func (proxy *DataSourceProxy) validateRequest() error {
 }
 
 func (proxy *DataSourceProxy) logRequest() {
-	if !setting.DataProxyLogging {
+	if !proxy.cfg.DataProxyLogging {
 		return
 	}
 

pkg/infra/httpclient/httpclientprovider/http_client_provider.go

@@ -3,6 +3,7 @@ package httpclientprovider
 import (
 	"fmt"
 	"net/http"
+	"time"
 
 	sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
 	"github.com/grafana/grafana/pkg/infra/httpclient"
@@ -18,6 +19,7 @@ var newProviderFunc = sdkhttpclient.NewProvider
 func New(cfg *setting.Cfg) httpclient.Provider {
 	logger := log.New("httpclient")
 	userAgent := fmt.Sprintf("Grafana/%s", cfg.BuildVersion)
+
 	middlewares := []sdkhttpclient.Middleware{
 		TracingMiddleware(logger),
 		DataSourceMetricsMiddleware(),
@@ -30,6 +32,8 @@ func New(cfg *setting.Cfg) httpclient.Provider {
 		middlewares = append(middlewares, SigV4Middleware())
 	}
 
+	setDefaultTimeoutOptions(cfg)
+
 	return newProviderFunc(sdkhttpclient.ProviderOptions{
 		Middlewares: middlewares,
 		ConfigureTransport: func(opts sdkhttpclient.Options, transport *http.Transport) {
@@ -56,3 +60,20 @@ func newConntrackRoundTripper(name string, transport *http.Transport) *http.Tran
 	)
 	return transport
 }
+
+// setDefaultTimeoutOptions overrides the default timeout options for the SDK.
+//
+// Note: Not optimal changing global state, but hard to not do in this case.
+func setDefaultTimeoutOptions(cfg *setting.Cfg) {
+	sdkhttpclient.DefaultTimeoutOptions = sdkhttpclient.TimeoutOptions{
+		Timeout:               time.Duration(cfg.DataProxyTimeout) * time.Second,
+		DialTimeout:           time.Duration(cfg.DataProxyDialTimeout) * time.Second,
+		KeepAlive:             time.Duration(cfg.DataProxyKeepAlive) * time.Second,
+		TLSHandshakeTimeout:   time.Duration(cfg.DataProxyTLSHandshakeTimeout) * time.Second,
+		ExpectContinueTimeout: time.Duration(cfg.DataProxyExpectContinueTimeout) * time.Second,
+		MaxConnsPerHost:       cfg.DataProxyMaxConnsPerHost,
+		MaxIdleConns:          cfg.DataProxyMaxIdleConns,
+		MaxIdleConnsPerHost:   cfg.DataProxyMaxIdleConns,
+		IdleConnTimeout:       time.Duration(cfg.DataProxyIdleConnTimeout) * time.Second,
+	}
+}

pkg/models/datasource_cache.go

@@ -11,7 +11,6 @@ import (
 	sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/infra/httpclient"
-	"github.com/grafana/grafana/pkg/setting"
 )
 
 func (ds *DataSource) getTimeout() time.Duration {
@@ -27,8 +26,9 @@ func (ds *DataSource) getTimeout() time.Duration {
 		}
 	}
 	if timeout <= 0 {
-		timeout = setting.DataProxyTimeout
+		return sdkhttpclient.DefaultTimeoutOptions.Timeout
 	}
+
 	return time.Duration(timeout) * time.Second
 }
 
@@ -86,14 +86,14 @@ func (ds *DataSource) HTTPClientOptions() sdkhttpclient.Options {
 	tlsOptions := ds.TLSOptions()
 	timeouts := &sdkhttpclient.TimeoutOptions{
 		Timeout:               ds.getTimeout(),
-		DialTimeout:           time.Duration(setting.DataProxyDialTimeout) * time.Second,
+		DialTimeout:           sdkhttpclient.DefaultTimeoutOptions.DialTimeout,
-		KeepAlive:             time.Duration(setting.DataProxyKeepAlive) * time.Second,
+		KeepAlive:             sdkhttpclient.DefaultTimeoutOptions.KeepAlive,
-		TLSHandshakeTimeout:   time.Duration(setting.DataProxyTLSHandshakeTimeout) * time.Second,
+		TLSHandshakeTimeout:   sdkhttpclient.DefaultTimeoutOptions.TLSHandshakeTimeout,
-		ExpectContinueTimeout: time.Duration(setting.DataProxyExpectContinueTimeout) * time.Second,
+		ExpectContinueTimeout: sdkhttpclient.DefaultTimeoutOptions.ExpectContinueTimeout,
-		MaxConnsPerHost:       setting.DataProxyMaxConnsPerHost,
+		MaxConnsPerHost:       sdkhttpclient.DefaultTimeoutOptions.MaxConnsPerHost,
-		MaxIdleConns:          setting.DataProxyMaxIdleConns,
+		MaxIdleConns:          sdkhttpclient.DefaultTimeoutOptions.MaxIdleConns,
-		MaxIdleConnsPerHost:   setting.DataProxyMaxIdleConns,
+		MaxIdleConnsPerHost:   sdkhttpclient.DefaultTimeoutOptions.MaxIdleConnsPerHost,
-		IdleConnTimeout:       time.Duration(setting.DataProxyIdleConnTimeout) * time.Second,
+		IdleConnTimeout:       sdkhttpclient.DefaultTimeoutOptions.IdleConnTimeout,
 	}
 	opts := sdkhttpclient.Options{
 		Timeouts: timeouts,

pkg/models/datasource_cache_test.go

@@ -312,14 +312,19 @@ func TestDataSource_GetHttpTransport(t *testing.T) {
 }
 
 func TestDataSource_getTimeout(t *testing.T) {
-	setting.DataProxyTimeout = 30
+	originalTimeout := sdkhttpclient.DefaultTimeoutOptions.Timeout
+	sdkhttpclient.DefaultTimeoutOptions.Timeout = 60 * time.Second
+	t.Cleanup(func() {
+		sdkhttpclient.DefaultTimeoutOptions.Timeout = originalTimeout
+	})
+
 	testCases := []struct {
 		jsonData        *simplejson.Json
 		expectedTimeout time.Duration
 	}{
-		{jsonData: simplejson.New(), expectedTimeout: 30 * time.Second},
+		{jsonData: simplejson.New(), expectedTimeout: 60 * time.Second},
-		{jsonData: simplejson.NewFromAny(map[string]interface{}{"timeout": nil}), expectedTimeout: 30 * time.Second},
+		{jsonData: simplejson.NewFromAny(map[string]interface{}{"timeout": nil}), expectedTimeout: 60 * time.Second},
-		{jsonData: simplejson.NewFromAny(map[string]interface{}{"timeout": 0}), expectedTimeout: 30 * time.Second},
+		{jsonData: simplejson.NewFromAny(map[string]interface{}{"timeout": 0}), expectedTimeout: 60 * time.Second},
 		{jsonData: simplejson.NewFromAny(map[string]interface{}{"timeout": 1}), expectedTimeout: time.Second},
 		{jsonData: simplejson.NewFromAny(map[string]interface{}{"timeout": "2"}), expectedTimeout: 2 * time.Second},
 	}

pkg/setting/setting.go

@@ -78,17 +78,7 @@ var (
 	CustomInitPath = "conf/custom.ini"
 
 	// HTTP server options
-	DataProxyLogging               bool
+	StaticRootPath string
-	DataProxyTimeout               int
-	DataProxyDialTimeout           int
-	DataProxyTLSHandshakeTimeout   int
-	DataProxyExpectContinueTimeout int
-	DataProxyMaxConnsPerHost       int
-	DataProxyMaxIdleConns          int
-	DataProxyMaxIdleConnsPerHost   int
-	DataProxyKeepAlive             int
-	DataProxyIdleConnTimeout       int
-	StaticRootPath                 string
 
 	// Security settings.
 	SecretKey              string
@@ -322,7 +312,16 @@ type Cfg struct {
 	JWTAuthJWKSetFile    string
 
 	// Dataproxy
-	SendUserHeader bool
+	SendUserHeader                 bool
+	DataProxyLogging               bool
+	DataProxyTimeout               int
+	DataProxyDialTimeout           int
+	DataProxyTLSHandshakeTimeout   int
+	DataProxyExpectContinueTimeout int
+	DataProxyMaxConnsPerHost       int
+	DataProxyMaxIdleConns          int
+	DataProxyKeepAlive             int
+	DataProxyIdleConnTimeout       int
 
 	// DistributedCache
 	RemoteCacheOptions *RemoteCacheOptions
@@ -843,22 +842,8 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error {
 		return err
 	}
 
-	// read data proxy settings
+	if err := readDataProxySettings(iniFile, cfg); err != nil {
-	dataproxy := iniFile.Section("dataproxy")
+		return err
-	DataProxyLogging = dataproxy.Key("logging").MustBool(false)
-	DataProxyTimeout = dataproxy.Key("timeout").MustInt(10)
-	DataProxyDialTimeout = dataproxy.Key("dialTimeout").MustInt(30)
-	DataProxyKeepAlive = dataproxy.Key("keep_alive_seconds").MustInt(30)
-	DataProxyTLSHandshakeTimeout = dataproxy.Key("tls_handshake_timeout_seconds").MustInt(10)
-	DataProxyExpectContinueTimeout = dataproxy.Key("expect_continue_timeout_seconds").MustInt(1)
-	DataProxyMaxConnsPerHost = dataproxy.Key("max_conns_per_host").MustInt(0)
-	DataProxyMaxIdleConns = dataproxy.Key("max_idle_connections").MustInt()
-	DataProxyIdleConnTimeout = dataproxy.Key("idle_conn_timeout_seconds").MustInt(90)
-	cfg.SendUserHeader = dataproxy.Key("send_user_header").MustBool(false)
-
-	if val, err := dataproxy.Key("max_idle_connections_per_host").Int(); err == nil {
-		cfg.Logger.Warn("[Deprecated] the configuration setting 'max_idle_connections_per_host' is deprecated, please use 'max_idle_connections' instead")
-		DataProxyMaxIdleConns = val
 	}
 
 	if err := readSecuritySettings(iniFile, cfg); err != nil {

pkg/setting/setting_data_proxy.go

@@ -0,0 +1,24 @@
+package setting
+
+import "gopkg.in/ini.v1"
+
+func readDataProxySettings(iniFile *ini.File, cfg *Cfg) error {
+	dataproxy := iniFile.Section("dataproxy")
+	cfg.SendUserHeader = dataproxy.Key("send_user_header").MustBool(false)
+	cfg.DataProxyLogging = dataproxy.Key("logging").MustBool(false)
+	cfg.DataProxyTimeout = dataproxy.Key("timeout").MustInt(10)
+	cfg.DataProxyDialTimeout = dataproxy.Key("dialTimeout").MustInt(30)
+	cfg.DataProxyKeepAlive = dataproxy.Key("keep_alive_seconds").MustInt(30)
+	cfg.DataProxyTLSHandshakeTimeout = dataproxy.Key("tls_handshake_timeout_seconds").MustInt(10)
+	cfg.DataProxyExpectContinueTimeout = dataproxy.Key("expect_continue_timeout_seconds").MustInt(1)
+	cfg.DataProxyMaxConnsPerHost = dataproxy.Key("max_conns_per_host").MustInt(0)
+	cfg.DataProxyMaxIdleConns = dataproxy.Key("max_idle_connections").MustInt()
+	cfg.DataProxyIdleConnTimeout = dataproxy.Key("idle_conn_timeout_seconds").MustInt(90)
+
+	if val, err := dataproxy.Key("max_idle_connections_per_host").Int(); err == nil {
+		cfg.Logger.Warn("[Deprecated] the configuration setting 'max_idle_connections_per_host' is deprecated, please use 'max_idle_connections' instead")
+		cfg.DataProxyMaxIdleConns = val
+	}
+
+	return nil
+}