From 5b4da3a7dc723af910747699460613c6f1d00ddf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Farkas?= Date: Mon, 11 Dec 2023 09:14:06 +0100 Subject: [PATCH] mssql: prepare logs-handling for decouple-datasource changes (#79214) mssql: refactor logs-handling --- pkg/api/datasource/validation.go | 2 +- pkg/tsdb/mssql/mssql.go | 23 ++++++++++++----------- pkg/tsdb/mssql/mssql_test.go | 11 ++++++++++- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/pkg/api/datasource/validation.go b/pkg/api/datasource/validation.go index b4939352d39..55628e4e930 100644 --- a/pkg/api/datasource/validation.go +++ b/pkg/api/datasource/validation.go @@ -69,7 +69,7 @@ func ValidateURL(typeName, urlStr string) (*url.URL, error) { var err error switch strings.ToLower(typeName) { case "mssql": - u, err = mssql.ParseURL(urlStr) + u, err = mssql.ParseURL(urlStr, logger) default: logger.Debug("Applying default URL parsing for this data source type", "type", typeName, "url", urlStr) diff --git a/pkg/tsdb/mssql/mssql.go b/pkg/tsdb/mssql/mssql.go index df224eddc5a..6a480b95b8c 100644 --- a/pkg/tsdb/mssql/mssql.go +++ b/pkg/tsdb/mssql/mssql.go @@ -28,10 +28,9 @@ import ( "github.com/grafana/grafana/pkg/util" ) -var logger = log.New("tsdb.mssql") - type Service struct { - im instancemgmt.InstanceManager + im instancemgmt.InstanceManager + logger log.Logger } const ( @@ -41,8 +40,10 @@ const ( ) func ProvideService(cfg *setting.Cfg) *Service { + logger := log.New("tsdb.mssql") return &Service{ - im: datasource.NewInstanceManager(newInstanceSettings(cfg)), + im: datasource.NewInstanceManager(newInstanceSettings(cfg, logger)), + logger: logger, } } @@ -63,7 +64,7 @@ func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest) return dsHandler.QueryData(ctx, req) } -func newInstanceSettings(cfg *setting.Cfg) datasource.InstanceFactoryFunc { +func newInstanceSettings(cfg *setting.Cfg, logger log.Logger) datasource.InstanceFactoryFunc { return func(_ context.Context, settings backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) { jsonData := sqleng.JsonData{ MaxOpenConns: cfg.SqlDatasourceMaxOpenConnsDefault, @@ -97,7 +98,7 @@ func newInstanceSettings(cfg *setting.Cfg) datasource.InstanceFactoryFunc { UID: settings.UID, DecryptedSecureJSONData: settings.DecryptedSecureJSONData, } - cnnstr, err := generateConnectionString(dsInfo, cfg, azureCredentials) + cnnstr, err := generateConnectionString(dsInfo, cfg, azureCredentials, logger) if err != nil { return nil, err } @@ -113,7 +114,7 @@ func newInstanceSettings(cfg *setting.Cfg) datasource.InstanceFactoryFunc { // register a new proxy driver if the secure socks proxy is enabled proxyOpts := proxyutil.GetSQLProxyOptions(cfg.SecureSocksDSProxy, dsInfo) if sdkproxy.New(proxyOpts).SecureSocksProxyEnabled() { - URL, err := ParseURL(dsInfo.URL) + URL, err := ParseURL(dsInfo.URL, logger) if err != nil { return nil, err } @@ -140,7 +141,7 @@ func newInstanceSettings(cfg *setting.Cfg) datasource.InstanceFactoryFunc { } // ParseURL tries to parse an MSSQL URL string into a URL object. -func ParseURL(u string) (*url.URL, error) { +func ParseURL(u string, logger log.Logger) (*url.URL, error) { logger.Debug("Parsing MSSQL URL", "url", u) // Recognize ODBC connection strings like host\instance:1234 @@ -160,11 +161,11 @@ func ParseURL(u string) (*url.URL, error) { }, nil } -func generateConnectionString(dsInfo sqleng.DataSourceInfo, cfg *setting.Cfg, azureCredentials azcredentials.AzureCredentials) (string, error) { +func generateConnectionString(dsInfo sqleng.DataSourceInfo, cfg *setting.Cfg, azureCredentials azcredentials.AzureCredentials, logger log.Logger) (string, error) { const dfltPort = "0" var addr util.NetworkAddress if dsInfo.URL != "" { - u, err := ParseURL(dsInfo.URL) + u, err := ParseURL(dsInfo.URL, logger) if err != nil { return "", err } @@ -280,7 +281,7 @@ func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthReque err = dsHandler.Ping() if err != nil { - return &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: dsHandler.TransformQueryError(logger, err).Error()}, nil + return &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: dsHandler.TransformQueryError(s.logger, err).Error()}, nil } return &backend.CheckHealthResult{Status: backend.HealthStatusOk, Message: "Database Connection OK"}, nil diff --git a/pkg/tsdb/mssql/mssql_test.go b/pkg/tsdb/mssql/mssql_test.go index 8ca2668fe7d..abbf79ba767 100644 --- a/pkg/tsdb/mssql/mssql_test.go +++ b/pkg/tsdb/mssql/mssql_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/sqlstore/sqlutil" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/sqleng" @@ -58,6 +59,9 @@ func TestMSSQL(t *testing.T) { MetricColumnTypes: []string{"VARCHAR", "CHAR", "NVARCHAR", "NCHAR"}, RowLimit: 1000000, } + + logger := log.New("mssql.test") + endpoint, err := sqleng.NewQueryDataHandler(setting.NewCfg(), config, &queryResultTransformer, newMssqlMacroEngine(), logger) require.NoError(t, err) @@ -1336,6 +1340,8 @@ func TestTransformQueryError(t *testing.T) { {err: randomErr, expectedErr: randomErr}, } + logger := log.New("mssql.test") + for _, tc := range tests { resultErr := transformer.TransformQueryError(logger, tc.err) assert.ErrorIs(t, resultErr, tc.expectedErr) @@ -1470,9 +1476,12 @@ func TestGenerateConnectionString(t *testing.T) { expConnStr: "server=localhost;database=database;user id=user;password=;", }, } + + logger := log.New("mssql.test") + for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - connStr, err := generateConnectionString(tc.dataSource, nil, nil) + connStr, err := generateConnectionString(tc.dataSource, nil, nil, logger) require.NoError(t, err) assert.Equal(t, tc.expConnStr, connStr) })