Skip to content

Commit

Permalink
[NR-302034] fix: handle DECIMAL values in custom queries (#173)
Browse files Browse the repository at this point in the history
  • Loading branch information
sigilioso authored Sep 16, 2024
1 parent deb4b89 commit 7e7d405
Show file tree
Hide file tree
Showing 8 changed files with 300 additions and 91 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 8 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
// <https://cs.opensource.google/go/go/+/refs/tags/go1.23.1:src/crypto/x509/parser.go;l=1019>
x509negativeserial=1
)
183 changes: 101 additions & 82 deletions src/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
package metrics

import (
"errors"
"fmt"
"io/ioutil"
"os"
"reflect"
"strconv"
"sync"
Expand All @@ -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",
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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()
Expand All @@ -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},
Expand All @@ -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 {
Expand All @@ -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
Expand Down
Loading

0 comments on commit 7e7d405

Please sign in to comment.