Skip to content

Commit

Permalink
prometheusreceiver: Add trim_metric_suffixes configuration option (#2…
Browse files Browse the repository at this point in the history
…4256)

**Description:**

Adds a new configuration option: `trim_metric_suffixes` to the
prometheus receiver. When set to true, suffixes will be trimmed from
metrics. This replaces use of
the`pkg.translator.prometheus.NormalizeName` feature-gate in the
prometheus receiver, but leaves it for exporters.

The first commit simplifies the usage of the feature-gate. The tests and
implementation were passing around an entire registry when it wasn't
necessary.

**Link to tracking Issue:**

Part of
#21743
Part of
#8950
  • Loading branch information
dashpole committed Jul 17, 2023
1 parent f0ba02b commit 5a975ab
Show file tree
Hide file tree
Showing 23 changed files with 148 additions and 158 deletions.
23 changes: 23 additions & 0 deletions .chloggen/prometheus-receiver-normalization-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Use this changelog template to create an entry for release notes.
# If your change doesn't affect end users, such as a test fix or a tooling change,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.

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

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Add the `trim_metric_suffixes` configuration option to allow enable metric suffix trimming."

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

# (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: |
When enabled, suffixes for unit and type are trimmed from metric names.
If you previously enabled the `pkg.translator.prometheus.NormalizeName`
feature gate, you will need to enable this option to have suffixes trimmed.
27 changes: 1 addition & 26 deletions pkg/translator/prometheus/normalize_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,36 +177,11 @@ func normalizeName(metric pmetric.Metric, namespace string) string {
return normalizedName
}

type Normalizer struct {
gate *featuregate.Gate
}

func NewNormalizer(registry *featuregate.Registry) *Normalizer {
var normalizeGate *featuregate.Gate
registry.VisitAll(func(gate *featuregate.Gate) {
if gate.ID() == normalizeNameGate.ID() {
normalizeGate = gate
}
})
// the registry didn't contain the flag, fallback to the global
// flag. Overriding the registry is really only done in tests
if normalizeGate == nil {
normalizeGate = normalizeNameGate
}
return &Normalizer{
gate: normalizeGate,
}
}

// TrimPromSuffixes trims type and unit prometheus suffixes from a metric name.
// Following the [OpenTelemetry specs] for converting Prometheus Metric points to OTLP.
//
// [OpenTelemetry specs]: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#metric-metadata
func (n *Normalizer) TrimPromSuffixes(promName string, metricType pmetric.MetricType, unit string) string {
if !n.gate.IsEnabled() {
return promName
}

func TrimPromSuffixes(promName string, metricType pmetric.MetricType, unit string) string {
nameTokens := strings.Split(promName, "_")
if len(nameTokens) == 1 {
return promName
Expand Down
59 changes: 23 additions & 36 deletions pkg/translator/prometheus/normalize_name_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/pdata/pmetric"

"github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil"
Expand Down Expand Up @@ -139,48 +138,36 @@ func TestOtelReceivers(t *testing.T) {
}

func TestTrimPromSuffixes(t *testing.T) {
registry := featuregate.NewRegistry()
_, err := registry.Register(normalizeNameGate.ID(), featuregate.StageBeta)
require.NoError(t, err)
normalizer := NewNormalizer(registry)

assert.Equal(t, "active_directory_ds_replication_network_io", normalizer.TrimPromSuffixes("active_directory_ds_replication_network_io_bytes_total", pmetric.MetricTypeSum, "bytes"))
assert.Equal(t, "active_directory_ds_name_cache_hit_rate", normalizer.TrimPromSuffixes("active_directory_ds_name_cache_hit_rate_percent", pmetric.MetricTypeGauge, "percent"))
assert.Equal(t, "active_directory_ds_ldap_bind_last_successful_time", normalizer.TrimPromSuffixes("active_directory_ds_ldap_bind_last_successful_time_milliseconds", pmetric.MetricTypeGauge, "milliseconds"))
assert.Equal(t, "apache_requests", normalizer.TrimPromSuffixes("apache_requests_total", pmetric.MetricTypeSum, "1"))
assert.Equal(t, "system_cpu_utilization", normalizer.TrimPromSuffixes("system_cpu_utilization_ratio", pmetric.MetricTypeGauge, "ratio"))
assert.Equal(t, "mongodbatlas_process_journaling_data_files", normalizer.TrimPromSuffixes("mongodbatlas_process_journaling_data_files_mebibytes", pmetric.MetricTypeGauge, "mebibytes"))
assert.Equal(t, "mongodbatlas_process_network_io", normalizer.TrimPromSuffixes("mongodbatlas_process_network_io_bytes_per_second", pmetric.MetricTypeGauge, "bytes_per_second"))
assert.Equal(t, "mongodbatlas_process_oplog_rate", normalizer.TrimPromSuffixes("mongodbatlas_process_oplog_rate_gibibytes_per_hour", pmetric.MetricTypeGauge, "gibibytes_per_hour"))
assert.Equal(t, "nsxt_node_memory_usage", normalizer.TrimPromSuffixes("nsxt_node_memory_usage_kilobytes", pmetric.MetricTypeGauge, "kilobytes"))
assert.Equal(t, "redis_latest_fork", normalizer.TrimPromSuffixes("redis_latest_fork_microseconds", pmetric.MetricTypeGauge, "microseconds"))
assert.Equal(t, "up", normalizer.TrimPromSuffixes("up", pmetric.MetricTypeGauge, ""))
assert.Equal(t, "active_directory_ds_replication_network_io", TrimPromSuffixes("active_directory_ds_replication_network_io_bytes_total", pmetric.MetricTypeSum, "bytes"))
assert.Equal(t, "active_directory_ds_name_cache_hit_rate", TrimPromSuffixes("active_directory_ds_name_cache_hit_rate_percent", pmetric.MetricTypeGauge, "percent"))
assert.Equal(t, "active_directory_ds_ldap_bind_last_successful_time", TrimPromSuffixes("active_directory_ds_ldap_bind_last_successful_time_milliseconds", pmetric.MetricTypeGauge, "milliseconds"))
assert.Equal(t, "apache_requests", TrimPromSuffixes("apache_requests_total", pmetric.MetricTypeSum, "1"))
assert.Equal(t, "system_cpu_utilization", TrimPromSuffixes("system_cpu_utilization_ratio", pmetric.MetricTypeGauge, "ratio"))
assert.Equal(t, "mongodbatlas_process_journaling_data_files", TrimPromSuffixes("mongodbatlas_process_journaling_data_files_mebibytes", pmetric.MetricTypeGauge, "mebibytes"))
assert.Equal(t, "mongodbatlas_process_network_io", TrimPromSuffixes("mongodbatlas_process_network_io_bytes_per_second", pmetric.MetricTypeGauge, "bytes_per_second"))
assert.Equal(t, "mongodbatlas_process_oplog_rate", TrimPromSuffixes("mongodbatlas_process_oplog_rate_gibibytes_per_hour", pmetric.MetricTypeGauge, "gibibytes_per_hour"))
assert.Equal(t, "nsxt_node_memory_usage", TrimPromSuffixes("nsxt_node_memory_usage_kilobytes", pmetric.MetricTypeGauge, "kilobytes"))
assert.Equal(t, "redis_latest_fork", TrimPromSuffixes("redis_latest_fork_microseconds", pmetric.MetricTypeGauge, "microseconds"))
assert.Equal(t, "up", TrimPromSuffixes("up", pmetric.MetricTypeGauge, ""))

// These are not necessarily valid OM units, only tested for the sake of completeness.
assert.Equal(t, "active_directory_ds_replication_sync_object_pending", normalizer.TrimPromSuffixes("active_directory_ds_replication_sync_object_pending_total", pmetric.MetricTypeSum, "{objects}"))
assert.Equal(t, "apache_current", normalizer.TrimPromSuffixes("apache_current_connections", pmetric.MetricTypeGauge, "connections"))
assert.Equal(t, "bigip_virtual_server_request_count", normalizer.TrimPromSuffixes("bigip_virtual_server_request_count_total", pmetric.MetricTypeSum, "{requests}"))
assert.Equal(t, "mongodbatlas_process_db_query_targeting_scanned_per_returned", normalizer.TrimPromSuffixes("mongodbatlas_process_db_query_targeting_scanned_per_returned", pmetric.MetricTypeGauge, "{scanned}/{returned}"))
assert.Equal(t, "nginx_connections_accepted", normalizer.TrimPromSuffixes("nginx_connections_accepted", pmetric.MetricTypeGauge, "connections"))
assert.Equal(t, "apache_workers", normalizer.TrimPromSuffixes("apache_workers_connections", pmetric.MetricTypeGauge, "connections"))
assert.Equal(t, "nginx", normalizer.TrimPromSuffixes("nginx_requests", pmetric.MetricTypeGauge, "requests"))
assert.Equal(t, "active_directory_ds_replication_sync_object_pending", TrimPromSuffixes("active_directory_ds_replication_sync_object_pending_total", pmetric.MetricTypeSum, "{objects}"))
assert.Equal(t, "apache_current", TrimPromSuffixes("apache_current_connections", pmetric.MetricTypeGauge, "connections"))
assert.Equal(t, "bigip_virtual_server_request_count", TrimPromSuffixes("bigip_virtual_server_request_count_total", pmetric.MetricTypeSum, "{requests}"))
assert.Equal(t, "mongodbatlas_process_db_query_targeting_scanned_per_returned", TrimPromSuffixes("mongodbatlas_process_db_query_targeting_scanned_per_returned", pmetric.MetricTypeGauge, "{scanned}/{returned}"))
assert.Equal(t, "nginx_connections_accepted", TrimPromSuffixes("nginx_connections_accepted", pmetric.MetricTypeGauge, "connections"))
assert.Equal(t, "apache_workers", TrimPromSuffixes("apache_workers_connections", pmetric.MetricTypeGauge, "connections"))
assert.Equal(t, "nginx", TrimPromSuffixes("nginx_requests", pmetric.MetricTypeGauge, "requests"))

// Units shouldn't be trimmed if the unit is not a direct match with the suffix, i.e, a suffix "_seconds" shouldn't be removed if unit is "sec" or "s"
assert.Equal(t, "system_cpu_load_average_15m_ratio", normalizer.TrimPromSuffixes("system_cpu_load_average_15m_ratio", pmetric.MetricTypeGauge, "1"))
assert.Equal(t, "mongodbatlas_process_asserts_per_second", normalizer.TrimPromSuffixes("mongodbatlas_process_asserts_per_second", pmetric.MetricTypeGauge, "{assertions}/s"))
assert.Equal(t, "memcached_operation_hit_ratio_percent", normalizer.TrimPromSuffixes("memcached_operation_hit_ratio_percent", pmetric.MetricTypeGauge, "%"))
assert.Equal(t, "active_directory_ds_replication_object_rate_per_second", normalizer.TrimPromSuffixes("active_directory_ds_replication_object_rate_per_second", pmetric.MetricTypeGauge, "{objects}/s"))
assert.Equal(t, "system_disk_operation_time_seconds", normalizer.TrimPromSuffixes("system_disk_operation_time_seconds_total", pmetric.MetricTypeSum, "s"))
assert.Equal(t, "system_cpu_load_average_15m_ratio", TrimPromSuffixes("system_cpu_load_average_15m_ratio", pmetric.MetricTypeGauge, "1"))
assert.Equal(t, "mongodbatlas_process_asserts_per_second", TrimPromSuffixes("mongodbatlas_process_asserts_per_second", pmetric.MetricTypeGauge, "{assertions}/s"))
assert.Equal(t, "memcached_operation_hit_ratio_percent", TrimPromSuffixes("memcached_operation_hit_ratio_percent", pmetric.MetricTypeGauge, "%"))
assert.Equal(t, "active_directory_ds_replication_object_rate_per_second", TrimPromSuffixes("active_directory_ds_replication_object_rate_per_second", pmetric.MetricTypeGauge, "{objects}/s"))
assert.Equal(t, "system_disk_operation_time_seconds", TrimPromSuffixes("system_disk_operation_time_seconds_total", pmetric.MetricTypeSum, "s"))

}

func TestTrimPromSuffixesWithFeatureGateDisabled(t *testing.T) {
normalizer := NewNormalizer(featuregate.NewRegistry())

assert.Equal(t, "apache_current_connections", normalizer.TrimPromSuffixes("apache_current_connections", pmetric.MetricTypeGauge, "connections"))
assert.Equal(t, "apache_requests_total", normalizer.TrimPromSuffixes("apache_requests_total", pmetric.MetricTypeSum, "1"))
}

func TestNamespace(t *testing.T) {
require.Equal(t, "space_test", normalizeName(createGauge("test", ""), "space"))
require.Equal(t, "space_test", normalizeName(createGauge("#test", ""), "space"))
Expand Down
22 changes: 22 additions & 0 deletions receiver/prometheusreceiver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,28 @@ receivers:
action: keep
```

The prometheus receiver also supports additional top-level options:

- **trim_metric_suffixes**: [**Experimental**] When set to true, this enables trimming unit and some counter type suffixes from metric names. For example, it would cause `singing_duration_seconds_total` to be trimmed to `singing_duration`. This can be useful when trying to restore the original metric names used in OpenTelemetry instrumentation. Defaults to false.
- **use_start_time_metric**: When set to true, this enables retrieving the start time of all counter metrics from the process_start_time_seconds metric. This is only correct if all counters on that endpoint started after the process start time, and the process is the only actor exporting the metric after the process started. It should not be used in "exporters" which export counters that may have started before the process itself. Use only if you know what you are doing, as this may result in incorrect rate calculations. Defaults to false.
- **start_time_metric_regex**: The regular expression for the start time metric, and is only applied when use_start_time_metric is enabled. Defaults to process_start_time_seconds.

For example,

```yaml
receivers:
prometheus:
trim_metric_suffixes: true
use_start_time_metric: true
start_time_metric_regex: foo_bar_.*
config:
scrape_configs:
- job_name: 'otel-collector'
scrape_interval: 5s
static_configs:
- targets: ['0.0.0.0:8888']
```

## OpenTelemetry Operator
Additional to this static job definitions this receiver allows to query a list of jobs from the
OpenTelemetryOperators TargetAllocator or a compatible endpoint.
Expand Down
7 changes: 4 additions & 3 deletions receiver/prometheusreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ const (

// Config defines configuration for Prometheus receiver.
type Config struct {
PrometheusConfig *promconfig.Config `mapstructure:"-"`
BufferPeriod time.Duration `mapstructure:"buffer_period"`
BufferCount int `mapstructure:"buffer_count"`
PrometheusConfig *promconfig.Config `mapstructure:"-"`
TrimMetricSuffixes bool `mapstructure:"trim_metric_suffixes"`
BufferPeriod time.Duration `mapstructure:"buffer_period"`
BufferCount int `mapstructure:"buffer_count"`
// UseStartTimeMetric enables retrieving the start time of all counter metrics
// from the process_start_time_seconds metric. This is only correct if all counters on that endpoint
// started after the process start time, and the process is the only actor exporting the metric after
Expand Down
1 change: 1 addition & 0 deletions receiver/prometheusreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func TestLoadConfig(t *testing.T) {
assert.Equal(t, r1.PrometheusConfig.ScrapeConfigs[0].JobName, "demo")
assert.Equal(t, time.Duration(r1.PrometheusConfig.ScrapeConfigs[0].ScrapeInterval), 5*time.Second)
assert.Equal(t, r1.UseStartTimeMetric, true)
assert.Equal(t, r1.TrimMetricSuffixes, true)
assert.Equal(t, r1.StartTimeMetricRegex, "^(.+_)*process_start_time_seconds$")

assert.Equal(t, "http:https://my-targetallocator-service", r1.TargetAllocator.Endpoint)
Expand Down
2 changes: 1 addition & 1 deletion receiver/prometheusreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,5 @@ func createMetricsReceiver(
cfg component.Config,
nextConsumer consumer.Metrics,
) (receiver.Metrics, error) {
return newPrometheusReceiver(set, cfg.(*Config), nextConsumer, featuregate.GlobalRegistry()), nil
return newPrometheusReceiver(set, cfg.(*Config), nextConsumer), nil
}
9 changes: 4 additions & 5 deletions receiver/prometheusreceiver/internal/appendable.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/storage"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/obsreport"
"go.opentelemetry.io/collector/receiver"
)
Expand All @@ -21,12 +20,12 @@ type appendable struct {
sink consumer.Metrics
metricAdjuster MetricsAdjuster
useStartTimeMetric bool
trimSuffixes bool
startTimeMetricRegex *regexp.Regexp
externalLabels labels.Labels

settings receiver.CreateSettings
obsrecv *obsreport.Receiver
registry *featuregate.Registry
}

// NewAppendable returns a storage.Appendable instance that emits metrics to the sink.
Expand All @@ -38,7 +37,7 @@ func NewAppendable(
startTimeMetricRegex *regexp.Regexp,
useCreatedMetric bool,
externalLabels labels.Labels,
registry *featuregate.Registry) (storage.Appendable, error) {
trimSuffixes bool) (storage.Appendable, error) {
var metricAdjuster MetricsAdjuster
if !useStartTimeMetric {
metricAdjuster = NewInitialPointAdjuster(set.Logger, gcInterval, useCreatedMetric)
Expand All @@ -59,10 +58,10 @@ func NewAppendable(
startTimeMetricRegex: startTimeMetricRegex,
externalLabels: externalLabels,
obsrecv: obsrecv,
registry: registry,
trimSuffixes: trimSuffixes,
}, nil
}

func (o *appendable) Appender(ctx context.Context) storage.Appender {
return newTransaction(ctx, o.metricAdjuster, o.sink, o.externalLabels, o.settings, o.obsrecv, o.registry)
return newTransaction(ctx, o.metricAdjuster, o.sink, o.externalLabels, o.settings, o.obsrecv, o.trimSuffixes)
}
10 changes: 7 additions & 3 deletions receiver/prometheusreceiver/internal/metricfamily.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,14 @@ func (mf *metricFamily) addSeries(seriesRef uint64, metricName string, ls labels
return nil
}

func (mf *metricFamily) appendMetric(metrics pmetric.MetricSlice, normalizer *prometheus.Normalizer) {
func (mf *metricFamily) appendMetric(metrics pmetric.MetricSlice, trimSuffixes bool) {
metric := pmetric.NewMetric()
// Trims type's and unit's suffixes from metric name
metric.SetName(normalizer.TrimPromSuffixes(mf.name, mf.mtype, mf.metadata.Unit))
// Trims type and unit suffixes from metric name
name := mf.name
if trimSuffixes {
name = prometheus.TrimPromSuffixes(name, mf.mtype, mf.metadata.Unit)
}
metric.SetName(name)
metric.SetDescription(mf.metadata.Help)
metric.SetUnit(mf.metadata.Unit)

Expand Down
9 changes: 3 additions & 6 deletions receiver/prometheusreceiver/internal/metricfamily_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@ import (
"github.com/prometheus/prometheus/model/value"
"github.com/prometheus/prometheus/scrape"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/prometheus"
)

type testMetadataStore map[string]scrape.MetricMetadata
Expand Down Expand Up @@ -252,7 +249,7 @@ func TestMetricGroupData_toDistributionUnitTest(t *testing.T) {
require.Len(t, mp.groups, 1)

sl := pmetric.NewMetricSlice()
mp.appendMetric(sl, prometheus.NewNormalizer(featuregate.GlobalRegistry()))
mp.appendMetric(sl, false)

require.Equal(t, 1, sl.Len(), "Exactly one metric expected")
metric := sl.At(0)
Expand Down Expand Up @@ -540,7 +537,7 @@ func TestMetricGroupData_toSummaryUnitTest(t *testing.T) {
require.Len(t, mp.groups, 1)

sl := pmetric.NewMetricSlice()
mp.appendMetric(sl, prometheus.NewNormalizer(featuregate.GlobalRegistry()))
mp.appendMetric(sl, false)

require.Equal(t, 1, sl.Len(), "Exactly one metric expected")
metric := sl.At(0)
Expand Down Expand Up @@ -646,7 +643,7 @@ func TestMetricGroupData_toNumberDataUnitTest(t *testing.T) {
require.Len(t, mp.groups, 1)

sl := pmetric.NewMetricSlice()
mp.appendMetric(sl, prometheus.NewNormalizer(featuregate.GlobalRegistry()))
mp.appendMetric(sl, false)

require.Equal(t, 1, sl.Len(), "Exactly one metric expected")
metric := sl.At(0)
Expand Down
Loading

0 comments on commit 5a975ab

Please sign in to comment.