Skip to content

Commit

Permalink
[processor/metricsgeneration] Fix panic when using a sum metric for c…
Browse files Browse the repository at this point in the history
…alculations (#35428)

**Description:** 
The metrics generation processor would previously hit a panic when the
calculation rule was operating on a `sum` metric, instead of a `gauge`.
The [component
proposal](#2722),
[initial
implementation](#3433),
nor the
[README](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/metricsgenerationprocessor)
state that it's a requirement for the metric to be a `gauge`, nor do I
think this requirement makes sense.

I've updated the logic to account for sum metrics being used and added a
test.

**Testing:** 
Added a test for this situation, the test panics before the change and
passes after.
  • Loading branch information
crobert-1 authored Sep 30, 2024
1 parent 7503861 commit c8f2553
Show file tree
Hide file tree
Showing 22 changed files with 2,712 additions and 21 deletions.
27 changes: 27 additions & 0 deletions .chloggen/metricsgeneration_relax_type_req.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: metricsgenerationprocessor

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Allow metric calculations to be done on sum metrics

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [35428]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
10 changes: 6 additions & 4 deletions processor/metricsgenerationprocessor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@

## Description

The metrics generation processor (`experimental_metricsgenerationprocessor`) can be used to create new metrics using existing metrics following a given rule. Currently it supports following two approaches for creating a new metric.
The metrics generation processor (`experimental_metricsgenerationprocessor`) can be used to create new metrics using existing metrics following a given rule. This processor currently supports the following two approaches for creating a new metric.

1. It can create a new metric from two existing metrics by applying one of the following arithmetic operations: add, subtract, multiply, divide and percent. One use case is to calculate the `pod.memory.utilization` metric like the following equation-
1. It can create a new metric from two existing metrics by applying one of the following arithmetic operations: add, subtract, multiply, divide, or percent. One use case is to calculate the `pod.memory.utilization` metric like the following equation-
`pod.memory.utilization` = (`pod.memory.usage.bytes` / `node.memory.limit`)
1. It can create a new metric by scaling the value of an existing metric with a given constant number. One use case is to convert `pod.memory.usage` metric values from Megabytes to Bytes (multiply the existing metric's value by 1,048,576)

Note: The created metric's type is inherited from the metric configured as `metric1`.

## Configuration

Configuration is specified through a list of generation rules. Generation rules find the metrics which
Expand All @@ -43,10 +45,10 @@ processors:
# type describes how the new metric will be generated. It can be one of `calculate` or `scale`. calculate generates a metric applying the given operation on two operand metrics. scale operates only on operand1 metric to generate the new metric.
type: {calculate, scale}

# This is a required field.
# This is a required field. This must be a gauge or sum metric.
metric1: <first_operand_metric>

# This field is required only if the type is "calculate".
# This field is required only if the type is "calculate". When required, this must be a gauge or sum metric.
metric2: <second_operand_metric>

# Operation specifies which arithmetic operation to apply. It must be one of the five supported operations.
Expand Down
10 changes: 10 additions & 0 deletions processor/metricsgenerationprocessor/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ module github.com/open-telemetry/opentelemetry-collector-contrib/processor/metri
go 1.22.0

require (
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden v0.110.0
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest v0.110.0
github.com/stretchr/testify v1.9.0
go.opentelemetry.io/collector/component v0.110.0
go.opentelemetry.io/collector/confmap v1.16.0
Expand All @@ -15,6 +17,7 @@ require (
)

require (
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
Expand All @@ -29,6 +32,7 @@ require (
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil v0.110.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.12.0 // indirect
go.opentelemetry.io/collector/component/componentstatus v0.110.0 // indirect
Expand Down Expand Up @@ -59,3 +63,9 @@ retract (
v0.76.1
v0.65.0
)

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest

replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil => ../../pkg/pdatautil
2 changes: 2 additions & 0 deletions processor/metricsgenerationprocessor/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

110 changes: 110 additions & 0 deletions processor/metricsgenerationprocessor/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,21 @@ package metricsgenerationprocessor

import (
"context"
"fmt"
"path/filepath"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/confmap/confmaptest"
"go.opentelemetry.io/collector/consumer/consumertest"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/processor/processortest"

"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest/pmetrictest"
)

type testMetric struct {
Expand Down Expand Up @@ -384,3 +390,107 @@ func getOutputForIntGaugeTest() pmetric.Metrics {

return intGaugeOutputMetrics
}

func TestSumCalculateNewMetric(t *testing.T) {
next := new(consumertest.MetricsSink)
cfg := &Config{
Rules: []Rule{
{
Name: "system.filesystem.capacity",
Unit: "bytes",
Type: "calculate",
Metric1: "system.filesystem.usage",
Metric2: "system.filesystem.utilization",
Operation: "divide",
},
},
}
factory := NewFactory()
mgp, err := factory.CreateMetricsProcessor(
context.Background(),
processortest.NewNopSettings(),
cfg,
next,
)
assert.NotNil(t, mgp)
assert.NoError(t, err)

assert.True(t, mgp.Capabilities().MutatesData)
require.NoError(t, mgp.Start(context.Background(), nil))

inputMetrics, err := golden.ReadMetrics(filepath.Join("testdata", "filesystem_metrics_input.yaml"))
assert.NoError(t, err)

err = mgp.ConsumeMetrics(context.Background(), inputMetrics)
assert.NoError(t, err)

got := next.AllMetrics()
// golden.WriteMetrics(t, filepath.Join(".", "testdata", "filesystem_metrics_expected.yaml"), got[0])
expected, err := golden.ReadMetrics(filepath.Join("testdata", "filesystem_metrics_expected.yaml"))
assert.NoError(t, err)
assert.Len(t, got, 1)
err = pmetrictest.CompareMetrics(expected, got[0],
pmetrictest.IgnoreMetricDataPointsOrder(),
pmetrictest.IgnoreStartTimestamp(),
pmetrictest.IgnoreTimestamp())
assert.NoError(t, err)
}

func TestResultingMetricTypes(t *testing.T) {
testCaseNames := []string{
"add_sum_sum",
"add_gauge_gauge",
"add_gauge_sum",
"add_sum_gauge",
"multiply_gauge_sum",
"multiply_sum_gauge",
"divide_gauge_sum",
"divide_sum_gauge",
"subtract_gauge_sum",
"subtract_sum_gauge",
"percent_sum_gauge",
"percent_gauge_sum",
}

cm, err := confmaptest.LoadConf(filepath.Join("testdata", "metric_types", "gauge_sum_metrics_config.yaml"))
assert.NoError(t, err)

for _, testCase := range testCaseNames {
next := new(consumertest.MetricsSink)
factory := NewFactory()
cfg := factory.CreateDefaultConfig()

sub, err := cm.Sub(fmt.Sprintf("%s/%s", "experimental_metricsgeneration", testCase))
require.NoError(t, err)
require.NoError(t, sub.Unmarshal(cfg))

mgp, err := factory.CreateMetricsProcessor(
context.Background(),
processortest.NewNopSettings(),
cfg,
next,
)
assert.NotNil(t, mgp)
assert.NoError(t, err)

assert.True(t, mgp.Capabilities().MutatesData)
require.NoError(t, mgp.Start(context.Background(), nil))

inputMetrics, err := golden.ReadMetrics(filepath.Join("testdata", "metric_types", "gauge_sum_metrics_input.yaml"))
assert.NoError(t, err)

err = mgp.ConsumeMetrics(context.Background(), inputMetrics)
assert.NoError(t, err)

got := next.AllMetrics()
// golden.WriteMetrics(t, filepath.Join("testdata", "metric_types", fmt.Sprintf("%s_%s", testCase, "expected.yaml")), got[0])
expected, err := golden.ReadMetrics(filepath.Join("testdata", "metric_types", fmt.Sprintf("%s_%s", testCase, "expected.yaml")))
assert.NoError(t, err)
assert.Len(t, got, 1)
err = pmetrictest.CompareMetrics(expected, got[0],
pmetrictest.IgnoreMetricDataPointsOrder(),
pmetrictest.IgnoreStartTimestamp(),
pmetrictest.IgnoreTimestamp())
assert.NoError(t, err)
}
}
Loading

0 comments on commit c8f2553

Please sign in to comment.