From b4927ff1cd25c343348dad4ebb23eeace7915e78 Mon Sep 17 00:00:00 2001 From: Sriram <153843+yesoreyeram@users.noreply.github.com> Date: Tue, 10 Dec 2024 13:13:32 +0000 Subject: [PATCH] MS SQL: Datasource health check error message improvements (#97643) improve mssql healthcheck error messages --- .../sqleng/handler_checkhealth.go | 9 +- pkg/tsdb/mssql/sqleng/handler_checkhealth.go | 93 +++++++++++++++++++ .../mssql/sqleng/handler_checkhealth_test.go | 60 ++++++++++++ pkg/tsdb/mssql/sqleng/sql_engine.go | 9 -- pkg/tsdb/mysql/sqleng/handler_checkhealth.go | 9 +- 5 files changed, 163 insertions(+), 17 deletions(-) create mode 100644 pkg/tsdb/mssql/sqleng/handler_checkhealth.go create mode 100644 pkg/tsdb/mssql/sqleng/handler_checkhealth_test.go diff --git a/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go b/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go index 6dedac9bdf8..a8584dfc59a 100644 --- a/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go +++ b/pkg/tsdb/grafana-postgresql-datasource/sqleng/handler_checkhealth.go @@ -16,7 +16,7 @@ import ( func (e *DataSourceHandler) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { err := e.Ping() if err != nil { - logCheckHealthError(ctx, e.dsInfo, err, e.log) + logCheckHealthError(ctx, e.dsInfo, err) if strings.EqualFold(req.PluginContext.User.Role, "Admin") { return ErrToHealthCheckResult(err) } @@ -73,7 +73,8 @@ func ErrToHealthCheckResult(err error) (*backend.CheckHealthResult, error) { return res, nil } -func logCheckHealthError(_ context.Context, dsInfo DataSourceInfo, err error, logger log.Logger) { +func logCheckHealthError(ctx context.Context, dsInfo DataSourceInfo, err error) { + logger := log.DefaultLogger.FromContext(ctx) configSummary := map[string]any{ "config_url_length": len(dsInfo.URL), "config_user_length": len(dsInfo.User), @@ -104,8 +105,8 @@ func logCheckHealthError(_ context.Context, dsInfo DataSourceInfo, err error, lo } configSummaryJson, marshalError := json.Marshal(configSummary) if marshalError != nil { - logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error", "plugin_id", "grafana-postgresql-datasource") + logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error") return } - logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error_detailed", "plugin_id", "grafana-postgresql-datasource", "details", string(configSummaryJson)) + logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error_detailed", "details", string(configSummaryJson)) } diff --git a/pkg/tsdb/mssql/sqleng/handler_checkhealth.go b/pkg/tsdb/mssql/sqleng/handler_checkhealth.go new file mode 100644 index 00000000000..ade94fe7349 --- /dev/null +++ b/pkg/tsdb/mssql/sqleng/handler_checkhealth.go @@ -0,0 +1,93 @@ +package sqleng + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net" + "strings" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" +) + +func (e *DataSourceHandler) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { + if err := e.db.Ping(); err != nil { + logCheckHealthError(ctx, e.dsInfo, err) + if strings.EqualFold(req.PluginContext.User.Role, "Admin") { + return ErrToHealthCheckResult(err) + } + return &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: e.TransformQueryError(e.log, err).Error()}, nil + } + return &backend.CheckHealthResult{Status: backend.HealthStatusOk, Message: "Database Connection OK"}, nil +} + +// ErrToHealthCheckResult converts error into user friendly health check message +// This should be called with non nil error. If the err parameter is empty, we will send Internal Server Error +func ErrToHealthCheckResult(err error) (*backend.CheckHealthResult, error) { + if err == nil { + return &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: "Internal Server Error"}, nil + } + res := &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: err.Error()} + details := map[string]string{ + "verboseMessage": err.Error(), + "errorDetailsLink": "https://grafana.com/docs/grafana/latest/datasources/mssql", + } + var opErr *net.OpError + if errors.As(err, &opErr) { + res.Message = "Network error: Failed to connect to the server" + if opErr != nil && opErr.Err != nil { + res.Message += fmt.Sprintf(". Error message: %s", opErr.Err.Error()) + } + } + if strings.HasPrefix(err.Error(), "mssql: ") { + res.Message = "Database error: Failed to connect to the mssql server" + if unwrappedErr := errors.Unwrap(err); unwrappedErr != nil { + details["verboseMessage"] = unwrappedErr.Error() + } + } + detailBytes, marshalErr := json.Marshal(details) + if marshalErr != nil { + return res, nil + } + res.JSONDetails = detailBytes + return res, nil +} + +func logCheckHealthError(ctx context.Context, dsInfo DataSourceInfo, err error) { + logger := log.DefaultLogger.FromContext(ctx) + configSummary := map[string]any{ + "config_url_length": len(dsInfo.URL), + "config_user_length": len(dsInfo.User), + "config_database_length": len(dsInfo.Database), + "config_json_data_database_length": len(dsInfo.JsonData.Database), + "config_max_open_conns": dsInfo.JsonData.MaxOpenConns, + "config_max_idle_conns": dsInfo.JsonData.MaxIdleConns, + "config_conn_max_life_time": dsInfo.JsonData.ConnMaxLifetime, + "config_conn_timeout": dsInfo.JsonData.ConnectionTimeout, + "config_ssl_mode": dsInfo.JsonData.Mode, + "config_tls_configuration_method": dsInfo.JsonData.ConfigurationMethod, + "config_tls_skip_verify": dsInfo.JsonData.TlsSkipVerify, + "config_timezone": dsInfo.JsonData.Timezone, + "config_time_interval": dsInfo.JsonData.TimeInterval, + "config_enable_secure_proxy": dsInfo.JsonData.SecureDSProxy, + "config_allow_clear_text_passwords": dsInfo.JsonData.AllowCleartextPasswords, + "config_authentication_type": dsInfo.JsonData.AuthenticationType, + "config_ssl_root_cert_file_length": len(dsInfo.JsonData.RootCertFile), + "config_ssl_cert_file_length": len(dsInfo.JsonData.CertFile), + "config_ssl_key_file_length": len(dsInfo.JsonData.CertKeyFile), + "config_encrypt_length": len(dsInfo.JsonData.Encrypt), + "config_server_name_length": len(dsInfo.JsonData.Servername), + "config_password_length": len(dsInfo.DecryptedSecureJSONData["password"]), + "config_tls_ca_cert_length": len(dsInfo.DecryptedSecureJSONData["tlsCACert"]), + "config_tls_client_cert_length": len(dsInfo.DecryptedSecureJSONData["tlsClientCert"]), + "config_tls_client_key_length": len(dsInfo.DecryptedSecureJSONData["tlsClientKey"]), + } + configSummaryJson, marshalError := json.Marshal(configSummary) + if marshalError != nil { + logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error") + return + } + logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error_detailed", "details", string(configSummaryJson)) +} diff --git a/pkg/tsdb/mssql/sqleng/handler_checkhealth_test.go b/pkg/tsdb/mssql/sqleng/handler_checkhealth_test.go new file mode 100644 index 00000000000..de41ffe1c58 --- /dev/null +++ b/pkg/tsdb/mssql/sqleng/handler_checkhealth_test.go @@ -0,0 +1,60 @@ +package sqleng + +import ( + "errors" + "net" + "testing" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + mssql "github.com/microsoft/go-mssqldb" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestErrToHealthCheckResult(t *testing.T) { + tests := []struct { + name string + err error + want *backend.CheckHealthResult + }{ + { + name: "without error", + want: &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: "Internal Server Error"}, + }, + { + name: "network error", + err: errors.Join(errors.New("foo"), &net.OpError{Op: "read", Net: "tcp", Err: errors.New("some op")}), + want: &backend.CheckHealthResult{ + Status: backend.HealthStatusError, + Message: "Network error: Failed to connect to the server. Error message: some op", + JSONDetails: []byte(`{"errorDetailsLink":"https://grafana.com/docs/grafana/latest/datasources/mssql","verboseMessage":"foo\nread tcp: some op"}`), + }, + }, + { + name: "db error", + err: errors.Join(errors.New("foo"), &mssql.Error{Message: "error foo occurred in mssql server"}), + want: &backend.CheckHealthResult{ + Status: backend.HealthStatusError, + Message: "foo\nmssql: error foo occurred in mssql server", + JSONDetails: []byte(`{"errorDetailsLink":"https://grafana.com/docs/grafana/latest/datasources/mssql","verboseMessage":"foo\nmssql: error foo occurred in mssql server"}`), + }, + }, + { + name: "regular error", + err: errors.New("internal server error"), + want: &backend.CheckHealthResult{ + Status: backend.HealthStatusError, + Message: "internal server error", + JSONDetails: []byte(`{"errorDetailsLink":"https://grafana.com/docs/grafana/latest/datasources/mssql","verboseMessage":"internal server error"}`), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ErrToHealthCheckResult(tt.err) + require.Nil(t, err) + assert.Equal(t, string(tt.want.JSONDetails), string(got.JSONDetails)) + require.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/tsdb/mssql/sqleng/sql_engine.go b/pkg/tsdb/mssql/sqleng/sql_engine.go index 153c5351258..27e34e52c11 100644 --- a/pkg/tsdb/mssql/sqleng/sql_engine.go +++ b/pkg/tsdb/mssql/sqleng/sql_engine.go @@ -152,15 +152,6 @@ func (e *DataSourceHandler) Dispose() { e.log.Debug("DB disposed") } -func (e *DataSourceHandler) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { - err := e.db.Ping() - - if err != nil { - return &backend.CheckHealthResult{Status: backend.HealthStatusError, Message: e.TransformQueryError(e.log, err).Error()}, nil - } - return &backend.CheckHealthResult{Status: backend.HealthStatusOk, Message: "Database Connection OK"}, nil -} - func (e *DataSourceHandler) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { result := backend.NewQueryDataResponse() ch := make(chan DBDataResponse, len(req.Queries)) diff --git a/pkg/tsdb/mysql/sqleng/handler_checkhealth.go b/pkg/tsdb/mysql/sqleng/handler_checkhealth.go index d224bb9d7ee..fea2b0b8f8f 100644 --- a/pkg/tsdb/mysql/sqleng/handler_checkhealth.go +++ b/pkg/tsdb/mysql/sqleng/handler_checkhealth.go @@ -16,7 +16,7 @@ import ( func (e *DataSourceHandler) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { err := e.db.Ping() if err != nil { - logCheckHealthError(ctx, e.dsInfo, err, e.log) + logCheckHealthError(ctx, e.dsInfo, err) if strings.EqualFold(req.PluginContext.User.Role, "Admin") { return ErrToHealthCheckResult(err) } @@ -63,7 +63,8 @@ func ErrToHealthCheckResult(err error) (*backend.CheckHealthResult, error) { return res, nil } -func logCheckHealthError(_ context.Context, dsInfo DataSourceInfo, err error, logger log.Logger) { +func logCheckHealthError(ctx context.Context, dsInfo DataSourceInfo, err error) { + logger := log.DefaultLogger.FromContext(ctx) configSummary := map[string]any{ "config_url_length": len(dsInfo.URL), "config_user_length": len(dsInfo.User), @@ -94,8 +95,8 @@ func logCheckHealthError(_ context.Context, dsInfo DataSourceInfo, err error, lo } configSummaryJson, marshalError := json.Marshal(configSummary) if marshalError != nil { - logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error", "plugin_id", "mysql") + logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error") return } - logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error_detailed", "plugin_id", "mysql", "details", string(configSummaryJson)) + logger.Error("Check health failed", "error", err, "message_type", "ds_config_health_check_error_detailed", "details", string(configSummaryJson)) }