Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NR-302034] fix: handle DECIMAL values in custom queries #173

Merged
merged 7 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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

paologallinaharbur marked this conversation as resolved.
Show resolved Hide resolved
## 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
)
paologallinaharbur marked this conversation as resolved.
Show resolved Hide resolved
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]
}
sigilioso marked this conversation as resolved.
Show resolved Hide resolved
if err := rows.Scan(valuesForScanning...); err != nil {
paologallinaharbur marked this conversation as resolved.
Show resolved Hide resolved
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 == "" {
paologallinaharbur marked this conversation as resolved.
Show resolved Hide resolved
if metricName != "" {
sigilioso marked this conversation as resolved.
Show resolved Hide resolved
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
Loading