diff --git a/CHANGELOG.md b/CHANGELOG.md index 762c81c..62f825a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,13 +8,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/). Unreleased section should follow [Release Toolkit](https://github.com/newrelic/release-toolkit#render-markdown-and-update-markdown) ## Unreleased +### bugfix +- Handle DECIMAL values in custom queries ## v2.12.7 - 2024-09-10 ### ⛓️ Dependencies - Updated golang version to v1.23.1 - -## bugfix - (deps) move from github.com/denisenkom/go-mssqldb to github.com/microsoft/go-mssqldb ## v2.12.6 - 2024-07-09 diff --git a/Makefile b/Makefile index e299a84..fdd0c9b 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ test: integration-test: @echo "=== $(INTEGRATION) === [ test ]: running integration tests..." @docker compose -f tests/docker-compose.yml pull - @go test -v -tags=integration ./tests/. || (ret=$$?; docker compose -f tests/docker-compose.yml down && exit $$ret) + @go test -count=1 -v -tags=integration ./tests/. || (ret=$$?; docker compose -f tests/docker-compose.yml down && exit $$ret) @docker compose -f tests/docker-compose.yml down compile: diff --git a/go.mod b/go.mod index e64cea3..ad56966 100644 --- a/go.mod +++ b/go.mod @@ -24,3 +24,11 @@ require ( golang.org/x/text v0.14.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +godebug ( +// Allows TLS certs with negative serial numbers. +// Before go 1.23 these certificates where accepted, now the corresponding go debug variable is needed +// to restore the previous behavior +// + x509negativeserial=1 +) diff --git a/src/metrics/metrics.go b/src/metrics/metrics.go index 69ecf72..d32f8ca 100644 --- a/src/metrics/metrics.go +++ b/src/metrics/metrics.go @@ -2,8 +2,9 @@ package metrics import ( + "errors" "fmt" - "io/ioutil" + "os" "reflect" "strconv" "sync" @@ -26,6 +27,15 @@ type customQuery struct { Database string } +// customQueryMetricValue represents a metric value fetched from the results of a custom query +type customQueryMetricValue struct { + value any + sourceType metric.SourceType +} + +var errMissingMetricValueCustomQuery = errors.New("missing 'metric_value' for custom query") +var errMissingMetricNameCustomQuery = errors.New("missing 'metric_name' for custom query") + // PopulateInstanceMetrics creates instance-level metrics func PopulateInstanceMetrics(instanceEntity *integration.Entity, connection *connection.SQLConnection, arguments args.ArgumentList) { metricSet := instanceEntity.NewMetricSet("MssqlInstanceSample", @@ -86,7 +96,7 @@ func PopulateInstanceMetrics(instanceEntity *integration.Entity, connection *con func parseCustomQueries(arguments args.ArgumentList) ([]customQuery, error) { // load YAML config file - b, err := ioutil.ReadFile(arguments.CustomMetricsConfig) + b, err := os.ReadFile(arguments.CustomMetricsConfig) if err != nil { return nil, fmt.Errorf("failed to read custom_metrics_config: %s", err) } @@ -152,6 +162,11 @@ func populateCustomMetrics(instanceEntity *integration.Entity, connection *conne log.Error("Could not execute custom query: %s", err) return } + columns, err := rows.Columns() + if err != nil { + log.Error("Could not fetch types information from custom query", err) + return + } defer func() { _ = rows.Close() @@ -160,72 +175,16 @@ func populateCustomMetrics(instanceEntity *integration.Entity, connection *conne var rowCount = 0 for rows.Next() { rowCount++ - row := make(map[string]interface{}) - err := rows.MapScan(row) - if err != nil { + values := make([]string, len(columns)) // All values are represented as strings (the corresponding conversion is handled while scanning) + valuesForScanning := make([]interface{}, len(columns)) // the `rows.Scan` function requires an array of interface{} + for i := range valuesForScanning { + valuesForScanning[i] = &values[i] + } + if err := rows.Scan(valuesForScanning...); err != nil { log.Error("Failed to scan custom query row: %s", err) return } - nameInterface, ok := row["metric_name"] - var name string - if !ok { - if len(query.Name) > 0 { - name = query.Name - } - } else { - name, ok = nameInterface.(string) - if !ok { - log.Error("Non-string type %T for custom query 'metric_name' column", nameInterface) - return - } - } - - value, ok := row["metric_value"] - var valueString string - if !ok { - if len(name) > 0 { - log.Error("Missing 'metric_value' for %s in custom query", name) - return - } - } else { - valueString = fmt.Sprintf("%v", value) - if len(name) == 0 { - log.Error("Missing 'metric_name' for %s in custom query", valueString) - return - } - } - - if len(query.Prefix) > 0 { - name = query.Prefix + name - } - - var metricType metric.SourceType - metricTypeInterface, ok := row["metric_type"] - if !ok { - if len(query.Type) > 0 { - metricType, err = metric.SourceTypeForName(query.Type) - if err != nil { - log.Error("Invalid metric type %s in YAML: %s", query.Type, err) - return - } - } else { - metricType = detectMetricType(valueString) - } - } else { - // metric type was specified - metricTypeString, ok := metricTypeInterface.(string) - if !ok { - log.Error("Non-string type %T for custom query 'metric_type' column", metricTypeInterface) - return - } - metricType, err = metric.SourceTypeForName(metricTypeString) - if err != nil { - log.Error("Invalid metric type %s in query 'metric_type' column: %s", metricTypeString, err) - return - } - } - attributes := []attribute.Attribute{ {Key: "displayName", Value: instanceEntity.Metadata.Name}, {Key: "entityName", Value: instanceEntity.Metadata.Namespace + ":" + instanceEntity.Metadata.Name}, @@ -237,30 +196,19 @@ func populateCustomMetrics(instanceEntity *integration.Entity, connection *conne } ms := instanceEntity.NewMetricSet("MssqlCustomQuerySample", attributes...) - for k, v := range row { - if k == "metric_name" || k == "metric_type" || k == "metric_value" { - continue - } - vString := fmt.Sprintf("%v", v) - - if len(query.Prefix) > 0 { - k = query.Prefix + k - } - - err = ms.SetMetric(k, vString, detectMetricType(vString)) - if err != nil { - log.Error("Failed to set metric: %s", err) - continue - } + dbMetrics, err := metricsFromCustomQueryRow(values, columns, query) + if err != nil { + log.Error("Error fetching metrics from query %s (query: %s)", err, query.Query) } - - if len(valueString) > 0 { - err = ms.SetMetric(name, valueString, metricType) + for name, dbMetric := range dbMetrics { + err = ms.SetMetric(name, dbMetric.value, dbMetric.sourceType) if err != nil { log.Error("Failed to set metric: %s", err) + continue } } } + if rowCount == 0 { log.Warn("No result set found for custom query: %+v", query) } else { @@ -272,6 +220,77 @@ func populateCustomMetrics(instanceEntity *integration.Entity, connection *conne } } +// metricsFromCustomQueryRow obtains a map of metrics from a row resulting from a custom query. +// A particular metric can be configured either with: +// - Specific columns in the query: metric_name, metric_type, metric_value +// - The corresponding `query.Name` and `query.Type` +// When both are defined, the query columns have precedence. Besides, if type is not defined it is automatically deteced. +// The rest of the query columns are also taken as metrics/attributes (detecting their types automatically). +// Besides, if `query.Prefix` is defined, all metric and attribute names will include the corresponding prefix. +func metricsFromCustomQueryRow(row []string, columns []string, query customQuery) (map[string]customQueryMetricValue, error) { + metrics := map[string]customQueryMetricValue{} + + var metricValue string + metricType := query.Type + metricName := query.Name + + for i, columnName := range columns { // Scan the query columns to extract the corresponding metrics + switch columnName { + // Handle columns with 'special' meaning + case "metric_name": + metricName = row[i] + case "metric_type": + metricType = row[i] + case "metric_value": + metricValue = row[i] + // The rest of the values are taken as metrics/attributes with automatically detected type. + default: + name := query.Prefix + columnName + value := row[i] + metrics[name] = customQueryMetricValue{value: value, sourceType: detectMetricType(value)} + } + } + + customQueryMetric, err := metricFromTargetColumns(metricValue, metricName, metricType, query) + if err != nil { + return nil, fmt.Errorf("could not extract metric from query: %w", err) + } + if customQueryMetric != nil { + metricName = query.Prefix + metricName + metrics[metricName] = *customQueryMetric + } + return metrics, nil +} + +// metricFromTargetColumns builds a customQueryMetricValue from the values in target columns (or defaults in the yaml +// configuration). It returns an error if values are inconsistent (Ex: metricName is set but metricValue is not) and it +// can be nil the metric was not defined. +func metricFromTargetColumns(metricValue, metricName, metricType string, query customQuery) (*customQueryMetricValue, error) { + if metricValue == "" { + if metricName != "" { + return nil, fmt.Errorf("%w: name %q, query %q", errMissingMetricValueCustomQuery, metricName, query.Query) + } + return nil, nil // Ignored when there is no value and no name + } + + if metricName == "" { + return nil, fmt.Errorf("%w: query %q", errMissingMetricNameCustomQuery, query.Query) + } + + var sourceType metric.SourceType + if metricType != "" { + sourceTypeFromQuery, err := metric.SourceTypeForName(metricType) + if err != nil { + return nil, fmt.Errorf("invalid metric type %s: %w", metricType, err) + } + sourceType = sourceTypeFromQuery + } else { + sourceType = detectMetricType(metricValue) + } + + return &customQueryMetricValue{value: metricValue, sourceType: sourceType}, nil +} + // PopulateDatabaseMetrics collects per-database metrics func PopulateDatabaseMetrics(i *integration.Integration, instanceName string, connection *connection.SQLConnection, arguments args.ArgumentList) error { // create database entities diff --git a/src/metrics/metrics_test.go b/src/metrics/metrics_test.go index 9c25917..32605fd 100644 --- a/src/metrics/metrics_test.go +++ b/src/metrics/metrics_test.go @@ -2,7 +2,7 @@ package metrics import ( "flag" - "io/ioutil" + "os" "path/filepath" "sync" "testing" @@ -23,7 +23,7 @@ var ( func updateGoldenFile(data []byte, sourceFile string) error { if *update { - return ioutil.WriteFile(sourceFile, data, 0644) + return os.WriteFile(sourceFile, data, 0600) } return nil } @@ -44,7 +44,7 @@ func createTestEntity(t *testing.T) (i *integration.Integration, e *integration. } func checkAgainstFile(t *testing.T, data []byte, expectedFile string) { - expectedData, err := ioutil.ReadFile(expectedFile) + expectedData, err := os.ReadFile(expectedFile) if err != nil { t.Errorf("Could not read expected file: %v", err.Error()) } @@ -218,3 +218,110 @@ func Test_populateWaitTimeMetrics(t *testing.T) { checkAgainstFile(t, actual, expectedFile) } + +func Test_populateCustomQuery(t *testing.T) { //nolint: funlen + cases := []struct { + Name string + cq customQuery + setupMock func(sqlmock.Sqlmock, customQuery) + expectedFileName string + }{ + { + Name: "Custom metrics in query", + setupMock: func(mock sqlmock.Sqlmock, cq customQuery) { + customQueryRows := sqlmock.NewRows([]string{"metric_name", "metric_value", "metric_type", "otherValue", "attrValue"}). + AddRow("myMetric", 0.5, "gauge", 42, "aa"). + AddRow("myMetric", 1.5, "gauge", 43, "bb") + mock.ExpectQuery(cq.Query).WillReturnRows(customQueryRows) + mock.ExpectClose() + }, + cq: customQuery{ + Query: `SELECT + 'myMetric' as metric_name, + value as metric_value, + 'gauge' as metric_type, + value2 as 'otherValue' + attr as 'attrValue' + FROM my_table`, + }, + expectedFileName: "customQuery.json", + }, + { + Name: "Custom metrics in config", + setupMock: func(mock sqlmock.Sqlmock, cq customQuery) { + customQueryRows := sqlmock.NewRows([]string{"metric_value", "otherValue", "attrValue"}). + AddRow(0.5, 42, "aa"). + AddRow(1.5, 43, "bb") + mock.ExpectQuery(cq.Query).WillReturnRows(customQueryRows) + mock.ExpectClose() + }, + cq: customQuery{ + Query: `SELECT + value as metric_value, + value2 as 'otherValue' + attr as 'attrValue' + FROM my_table`, + Name: "myMetric", + Type: "gauge", + Prefix: "prefix_", + }, + expectedFileName: "customQueryPrefix.json", + }, + { + Name: "Custom metrics in config, detecting type", + setupMock: func(mock sqlmock.Sqlmock, cq customQuery) { + customQueryRows := sqlmock.NewRows([]string{"metric_value", "otherValue", "attrValue"}). + AddRow(0.5, 42, "aa"). + AddRow(1.5, 43, "bb") + mock.ExpectQuery(cq.Query).WillReturnRows(customQueryRows) + mock.ExpectClose() + }, + cq: customQuery{ + Query: `SELECT + value as metric_value, + value2 as 'otherValue' + attr as 'attrValue' + FROM my_table`, + Name: "myMetric", + Prefix: "prefix_", + }, + expectedFileName: "customQueryPrefix.json", + }, + { + Name: "Custom metrics, query has precedence", + setupMock: func(mock sqlmock.Sqlmock, cq customQuery) { + customQueryRows := sqlmock.NewRows([]string{"metric_name", "metric_value", "metric_type", "otherValue", "attrValue"}). + AddRow("myMetric", 0.5, "gauge", 42, "aa"). + AddRow("myMetric", 1.5, "gauge", 43, "bb") + mock.ExpectQuery(cq.Query).WillReturnRows(customQueryRows) + mock.ExpectClose() + }, + cq: customQuery{ + Query: `SELECT + 'myMetric' as metric_name, + value as metric_value, + 'gauge' as metric_type, + value2 as 'otherValue' + attr as 'attrValue' + FROM my_table`, + Name: "other", + Type: "delta", + Prefix: "prefix_", + }, + expectedFileName: "customQueryPrefix.json", + }, + } + + for _, tc := range cases { + t.Run(tc.Name, func(t *testing.T) { //nolint: paralleltest // setup mocks + i, e := createTestEntity(t) + conn, mock := connection.CreateMockSQL(t) + defer conn.Close() + tc.setupMock(mock, tc.cq) + populateCustomMetrics(e, conn, tc.cq) + actual, _ := i.MarshalJSON() + expectedFile := filepath.Join("..", "testdata", tc.expectedFileName) + checkAgainstFile(t, actual, expectedFile) + }) + } +} diff --git a/src/testdata/customQuery.json b/src/testdata/customQuery.json new file mode 100644 index 0000000..b662627 --- /dev/null +++ b/src/testdata/customQuery.json @@ -0,0 +1,38 @@ +{ + "name": "test", + "protocol_version": "3", + "integration_version": "1.0.0", + "data": [ + { + "entity": { + "name": "test", + "type": "instance", + "id_attributes": [] + }, + "metrics": [ + { + "attrValue": "aa", + "displayName": "test", + "entityName": "instance:test", + "event_type": "MssqlCustomQuerySample", + "host": "testhost", + "instance": "test", + "myMetric": 0.5, + "otherValue": 42 + }, + { + "attrValue": "bb", + "displayName": "test", + "entityName": "instance:test", + "event_type": "MssqlCustomQuerySample", + "host": "testhost", + "instance": "test", + "myMetric": 1.5, + "otherValue": 43 + } + ], + "inventory": {}, + "events": [] + } + ] +} diff --git a/src/testdata/customQueryPrefix.json b/src/testdata/customQueryPrefix.json new file mode 100644 index 0000000..db3450d --- /dev/null +++ b/src/testdata/customQueryPrefix.json @@ -0,0 +1,38 @@ +{ + "name": "test", + "protocol_version": "3", + "integration_version": "1.0.0", + "data": [ + { + "entity": { + "name": "test", + "type": "instance", + "id_attributes": [] + }, + "metrics": [ + { + "prefix_attrValue": "aa", + "displayName": "test", + "entityName": "instance:test", + "event_type": "MssqlCustomQuerySample", + "host": "testhost", + "instance": "test", + "prefix_myMetric": 0.5, + "prefix_otherValue": 42 + }, + { + "prefix_attrValue": "bb", + "displayName": "test", + "entityName": "instance:test", + "event_type": "MssqlCustomQuerySample", + "host": "testhost", + "instance": "test", + "prefix_myMetric": 1.5, + "prefix_otherValue": 43 + } + ], + "inventory": {}, + "events": [] + } + ] +} diff --git a/tests/mssql_test.go b/tests/mssql_test.go index 5367548..7017381 100644 --- a/tests/mssql_test.go +++ b/tests/mssql_test.go @@ -1,4 +1,4 @@ -// +build integration +//go:build integration package tests @@ -42,8 +42,8 @@ func waitForMSSQLIsUpAndRunning(maxTries int) bool { } fmt.Println(stdout) fmt.Println(stderr) - log.Info("try to establish de connection with the mssql database...") for ; maxTries > 0; maxTries-- { + time.Sleep(5 * time.Second) log.Info("try to establish de connection with the mssql database...") conn, err := connection.NewConnection(&args.ArgumentList{ @@ -54,7 +54,6 @@ func waitForMSSQLIsUpAndRunning(maxTries int) bool { }) if err != nil { log.Warn(err.Error()) - time.Sleep(5 * time.Second) continue } if conn != nil {