MS SQL: Datasource health check error message improvements (#97643)

improve mssql healthcheck error messages

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))
 }

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))
+}

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)
+		})
+	}
+}

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))

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))
 }