Skip to content

Commit

Permalink
prometheus exporters: Add add_metric_suffixes configuration option (#…
Browse files Browse the repository at this point in the history
…24260)

**Description:**
Add add_metric_suffixes configuration option, which can disable the
addition of type and unit suffixes.
This is backwards-compatible, since the default is true.

This is recommended by the specification for sum suffixes in
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/compatibility/prometheus_and_openmetrics.md#sums
and allowed in metadata

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/compatibility/prometheus_and_openmetrics.md#metric-metadata-1:

`Exporters SHOULD provide a configuration option to disable the addition
of _total suffixes.`

`The resulting unit SHOULD be added to the metric as OpenMetrics UNIT
metadata and as a suffix to the metric name unless the metric name
already contains the unit, or the unit MUST be omitted`

This deprecates the BuildPromCompliantName function in-favor of
BuildCompliantName, which includes the additional argument for
configuring suffixes.

**Link to tracking Issue:**

Fixes
#21743
Part of
#8950

---------

Co-authored-by: Dmitrii Anoshin <[email protected]>
  • Loading branch information
dashpole and dmitryax committed Jul 18, 2023
1 parent 227ed82 commit 2bc9904
Show file tree
Hide file tree
Showing 19 changed files with 97 additions and 37 deletions.
20 changes: 20 additions & 0 deletions .chloggen/prometheusexporters-normalization-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# 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: 'enhancement'

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Add `add_metric_suffixes` configuration option, which can disable the addition of type and unit suffixes."

# 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:
2 changes: 2 additions & 0 deletions exporter/prometheusexporter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ The following settings can be optionally configured:
- `resource_to_telemetry_conversion`
- `enabled` (default = false): If `enabled` is `true`, all the resource attributes will be converted to metric labels by default.
- `enable_open_metrics`: (default = `false`): If true, metrics will be exported using the OpenMetrics format. Exemplars are only exported in the OpenMetrics format, and only for histogram and monotonic sum (i.e. counter) metrics.
- `add_metric_suffixes`: (default = `true`): If false, addition of type and unit suffixes is disabled.

Example:

Expand All @@ -51,6 +52,7 @@ exporters:
send_timestamps: true
metric_expiration: 180m
enable_open_metrics: true
add_metric_suffixes: false
resource_to_telemetry_conversion:
enabled: true
```
Expand Down
20 changes: 11 additions & 9 deletions exporter/prometheusexporter/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,20 @@ type collector struct {
accumulator accumulator
logger *zap.Logger

sendTimestamps bool
namespace string
constLabels prometheus.Labels
sendTimestamps bool
addMetricSuffixes bool
namespace string
constLabels prometheus.Labels
}

func newCollector(config *Config, logger *zap.Logger) *collector {
return &collector{
accumulator: newAccumulator(logger, config.MetricExpiration),
logger: logger,
namespace: prometheustranslator.CleanUpString(config.Namespace),
sendTimestamps: config.SendTimestamps,
constLabels: config.ConstLabels,
accumulator: newAccumulator(logger, config.MetricExpiration),
logger: logger,
namespace: prometheustranslator.CleanUpString(config.Namespace),
sendTimestamps: config.SendTimestamps,
constLabels: config.ConstLabels,
addMetricSuffixes: config.AddMetricSuffixes,
}
}

Expand Down Expand Up @@ -127,7 +129,7 @@ func (c *collector) getMetricMetadata(metric pmetric.Metric, attributes pcommon.
}

return prometheus.NewDesc(
prometheustranslator.BuildPromCompliantName(metric, c.namespace),
prometheustranslator.BuildCompliantName(metric, c.namespace, c.addMetricSuffixes),
metric.Description(),
keys,
c.constLabels,
Expand Down
3 changes: 3 additions & 0 deletions exporter/prometheusexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ type Config struct {

// EnableOpenMetrics enables the use of the OpenMetrics encoding option for the prometheus exporter.
EnableOpenMetrics bool `mapstructure:"enable_open_metrics"`

// AddMetricSuffixes controls whether suffixes are added to metric names. Defaults to true.
AddMetricSuffixes bool `mapstructure:"add_metric_suffixes"`
}

var _ component.Config = (*Config)(nil)
Expand Down
5 changes: 3 additions & 2 deletions exporter/prometheusexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ func TestLoadConfig(t *testing.T) {
"label1": "value1",
"another label": "spaced value",
},
SendTimestamps: true,
MetricExpiration: 60 * time.Minute,
SendTimestamps: true,
MetricExpiration: 60 * time.Minute,
AddMetricSuffixes: false,
},
},
}
Expand Down
1 change: 1 addition & 0 deletions exporter/prometheusexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func createDefaultConfig() component.Config {
SendTimestamps: false,
MetricExpiration: time.Minute * 5,
EnableOpenMetrics: false,
AddMetricSuffixes: true,
}
}

Expand Down
1 change: 1 addition & 0 deletions exporter/prometheusexporter/testdata/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ prometheus/2:
"another label": spaced value
send_timestamps: true
metric_expiration: 60m
add_metric_suffixes: false
1 change: 1 addition & 0 deletions exporter/prometheusremotewriteexporter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ The following settings can be optionally configured:
- `headers`: additional headers attached to each HTTP request.
- *Note the following headers cannot be changed: `Content-Encoding`, `Content-Type`, `X-Prometheus-Remote-Write-Version`, and `User-Agent`.*
- `namespace`: prefix attached to each exported metric name.
- `add_metric_suffixes`: If set to false, type and unit suffixes will not be added to metrics. Default: true.
- `remote_write_queue`: fine tuning for queueing and sending of the outgoing remote writes.
- `enabled`: enable the sending queue
- `queue_size`: number of OTLP metrics that can be queued. Ignored if `enabled` is `false`
Expand Down
3 changes: 3 additions & 0 deletions exporter/prometheusremotewriteexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ type Config struct {

// CreatedMetric allows customizing creation of _created metrics
CreatedMetric *CreatedMetric `mapstructure:"export_created_metric,omitempty"`

// AddMetricSuffixes controls whether unit and type suffixes are added to metrics on export
AddMetricSuffixes bool `mapstructure:"add_metric_suffixes"`
}

type CreatedMetric struct {
Expand Down
5 changes: 3 additions & 2 deletions exporter/prometheusremotewriteexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ func TestLoadConfig(t *testing.T) {
QueueSize: 2000,
NumConsumers: 10,
},
Namespace: "test-space",
ExternalLabels: map[string]string{"key1": "value1", "key2": "value2"},
AddMetricSuffixes: false,
Namespace: "test-space",
ExternalLabels: map[string]string{"key1": "value1", "key2": "value2"},
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: "localhost:8888",
TLSSetting: configtls.TLSClientSetting{
Expand Down
1 change: 1 addition & 0 deletions exporter/prometheusremotewriteexporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func newPRWExporter(cfg *Config, set exporter.CreateSettings) (*prwExporter, err
ExternalLabels: sanitizedLabels,
DisableTargetInfo: !cfg.TargetInfo.Enabled,
ExportCreatedMetric: cfg.CreatedMetric.Enabled,
AddMetricSuffixes: cfg.AddMetricSuffixes,
},
}
if cfg.WAL == nil {
Expand Down
1 change: 1 addition & 0 deletions exporter/prometheusremotewriteexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func createDefaultConfig() component.Config {
RandomizationFactor: backoff.DefaultRandomizationFactor,
Multiplier: backoff.DefaultMultiplier,
},
AddMetricSuffixes: true,
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: "http:https://some.url:9411/api/prom/push",
// We almost read 0 bytes, so no need to tune ReadBufferSize.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ prometheusremotewrite/2:
tls:
ca_file: "/var/lib/mycert.pem"
write_buffer_size: 524288
add_metric_suffixes: false
headers:
Prometheus-Remote-Write-Version: "0.1.0"
X-Scope-OrgID: 234
Expand Down
11 changes: 8 additions & 3 deletions pkg/translator/prometheus/normalize_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,24 @@ var normalizeNameGate = featuregate.GlobalRegistry().MustRegister(
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/8950"),
)

// Build a Prometheus-compliant metric name for the specified metric
// Deprecated: use BuildCompliantName instead.
func BuildPromCompliantName(metric pmetric.Metric, namespace string) string {
return BuildCompliantName(metric, namespace, true)
}

// BuildCompliantName builds a Prometheus-compliant metric name for the specified metric
//
// Metric name is prefixed with specified namespace and underscore (if any).
// Namespace is not cleaned up. Make sure specified namespace follows Prometheus
// naming convention.
//
// See rules at https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
// and https://prometheus.io/docs/practices/naming/#metric-and-label-naming
func BuildPromCompliantName(metric pmetric.Metric, namespace string) string {
func BuildCompliantName(metric pmetric.Metric, namespace string, addMetricSuffixes bool) string {
var metricName string

// Full normalization following standard Prometheus naming conventions
if normalizeNameGate.IsEnabled() {
if addMetricSuffixes && normalizeNameGate.IsEnabled() {
return normalizeName(metric, namespace)
}

Expand Down
46 changes: 31 additions & 15 deletions pkg/translator/prometheus/normalize_name_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,27 +210,43 @@ func TestRemoveItem(t *testing.T) {

}

func TestBuildPromCompliantNameWithNormalize(t *testing.T) {
func TestBuildCompliantNameWithNormalize(t *testing.T) {

defer testutil.SetFeatureGateForTest(t, normalizeNameGate, true)()
require.Equal(t, "system_io_bytes_total", BuildPromCompliantName(createCounter("system.io", "By"), ""))
require.Equal(t, "system_network_io_bytes_total", BuildPromCompliantName(createCounter("network.io", "By"), "system"))
require.Equal(t, "_3_14_digits", BuildPromCompliantName(createGauge("3.14 digits", ""), ""))
require.Equal(t, "envoy_rule_engine_zlib_buf_error", BuildPromCompliantName(createGauge("envoy__rule_engine_zlib_buf_error", ""), ""))
require.Equal(t, "foo_bar", BuildPromCompliantName(createGauge(":foo::bar", ""), ""))
require.Equal(t, "foo_bar_total", BuildPromCompliantName(createCounter(":foo::bar", ""), ""))
addUnitAndTypeSuffixes := true
require.Equal(t, "system_io_bytes_total", BuildCompliantName(createCounter("system.io", "By"), "", addUnitAndTypeSuffixes))
require.Equal(t, "system_network_io_bytes_total", BuildCompliantName(createCounter("network.io", "By"), "system", addUnitAndTypeSuffixes))
require.Equal(t, "_3_14_digits", BuildCompliantName(createGauge("3.14 digits", ""), "", addUnitAndTypeSuffixes))
require.Equal(t, "envoy_rule_engine_zlib_buf_error", BuildCompliantName(createGauge("envoy__rule_engine_zlib_buf_error", ""), "", addUnitAndTypeSuffixes))
require.Equal(t, "foo_bar", BuildCompliantName(createGauge(":foo::bar", ""), "", addUnitAndTypeSuffixes))
require.Equal(t, "foo_bar_total", BuildCompliantName(createCounter(":foo::bar", ""), "", addUnitAndTypeSuffixes))

}

func TestBuildPromCompliantNameWithoutNormalize(t *testing.T) {
func TestBuildCompliantNameWithSuffixesFeatureGateDisabled(t *testing.T) {

defer testutil.SetFeatureGateForTest(t, normalizeNameGate, false)()
require.Equal(t, "system_io", BuildPromCompliantName(createCounter("system.io", "By"), ""))
require.Equal(t, "system_network_io", BuildPromCompliantName(createCounter("network.io", "By"), "system"))
require.Equal(t, "system_network_I_O", BuildPromCompliantName(createCounter("network (I/O)", "By"), "system"))
require.Equal(t, "_3_14_digits", BuildPromCompliantName(createGauge("3.14 digits", "By"), ""))
require.Equal(t, "envoy__rule_engine_zlib_buf_error", BuildPromCompliantName(createGauge("envoy__rule_engine_zlib_buf_error", ""), ""))
require.Equal(t, ":foo::bar", BuildPromCompliantName(createGauge(":foo::bar", ""), ""))
require.Equal(t, ":foo::bar", BuildPromCompliantName(createCounter(":foo::bar", ""), ""))
addUnitAndTypeSuffixes := true
require.Equal(t, "system_io", BuildCompliantName(createCounter("system.io", "By"), "", addUnitAndTypeSuffixes))
require.Equal(t, "system_network_io", BuildCompliantName(createCounter("network.io", "By"), "system", addUnitAndTypeSuffixes))
require.Equal(t, "system_network_I_O", BuildCompliantName(createCounter("network (I/O)", "By"), "system", addUnitAndTypeSuffixes))
require.Equal(t, "_3_14_digits", BuildCompliantName(createGauge("3.14 digits", "By"), "", addUnitAndTypeSuffixes))
require.Equal(t, "envoy__rule_engine_zlib_buf_error", BuildCompliantName(createGauge("envoy__rule_engine_zlib_buf_error", ""), "", addUnitAndTypeSuffixes))
require.Equal(t, ":foo::bar", BuildCompliantName(createGauge(":foo::bar", ""), "", addUnitAndTypeSuffixes))
require.Equal(t, ":foo::bar", BuildCompliantName(createCounter(":foo::bar", ""), "", addUnitAndTypeSuffixes))

}

func TestBuildCompliantNameWithoutSuffixes(t *testing.T) {

defer testutil.SetFeatureGateForTest(t, normalizeNameGate, false)()
addUnitAndTypeSuffixes := false
require.Equal(t, "system_io", BuildCompliantName(createCounter("system.io", "By"), "", addUnitAndTypeSuffixes))
require.Equal(t, "system_network_io", BuildCompliantName(createCounter("network.io", "By"), "system", addUnitAndTypeSuffixes))
require.Equal(t, "system_network_I_O", BuildCompliantName(createCounter("network (I/O)", "By"), "system", addUnitAndTypeSuffixes))
require.Equal(t, "_3_14_digits", BuildCompliantName(createGauge("3.14 digits", "By"), "", addUnitAndTypeSuffixes))
require.Equal(t, "envoy__rule_engine_zlib_buf_error", BuildCompliantName(createGauge("envoy__rule_engine_zlib_buf_error", ""), "", addUnitAndTypeSuffixes))
require.Equal(t, ":foo::bar", BuildCompliantName(createGauge(":foo::bar", ""), "", addUnitAndTypeSuffixes))
require.Equal(t, ":foo::bar", BuildCompliantName(createCounter(":foo::bar", ""), "", addUnitAndTypeSuffixes))

}
4 changes: 2 additions & 2 deletions pkg/translator/prometheusremotewrite/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func isValidAggregationTemporality(metric pmetric.Metric) bool {
func addSingleHistogramDataPoint(pt pmetric.HistogramDataPoint, resource pcommon.Resource, metric pmetric.Metric, settings Settings, tsMap map[string]*prompb.TimeSeries) {
timestamp := convertTimeStamp(pt.Timestamp())
// sum, count, and buckets of the histogram should append suffix to baseName
baseName := prometheustranslator.BuildPromCompliantName(metric, settings.Namespace)
baseName := prometheustranslator.BuildCompliantName(metric, settings.Namespace, settings.AddMetricSuffixes)

// If the sum is unset, it indicates the _sum metric point should be
// omitted
Expand Down Expand Up @@ -442,7 +442,7 @@ func addSingleSummaryDataPoint(pt pmetric.SummaryDataPoint, resource pcommon.Res
tsMap map[string]*prompb.TimeSeries) {
timestamp := convertTimeStamp(pt.Timestamp())
// sum and count of the summary should append suffix to baseName
baseName := prometheustranslator.BuildPromCompliantName(metric, settings.Namespace)
baseName := prometheustranslator.BuildCompliantName(metric, settings.Namespace, settings.AddMetricSuffixes)
// treat sum as a sample in an individual TimeSeries
sum := &prompb.Sample{
Value: pt.Sum(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/translator/prometheusremotewrite/histograms_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ func TestAddSingleExponentialHistogramDataPoint(t *testing.T) {

for x := 0; x < metric.ExponentialHistogram().DataPoints().Len(); x++ {
err := addSingleExponentialHistogramDataPoint(
prometheustranslator.BuildPromCompliantName(metric, ""),
prometheustranslator.BuildCompliantName(metric, "", true),
metric.ExponentialHistogram().DataPoints().At(x),
pcommon.NewResource(),
Settings{},
Expand Down
3 changes: 2 additions & 1 deletion pkg/translator/prometheusremotewrite/metrics_to_prw.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type Settings struct {
ExternalLabels map[string]string
DisableTargetInfo bool
ExportCreatedMetric bool
AddMetricSuffixes bool
}

// FromMetrics converts pmetric.Metrics to prometheus remote write format.
Expand Down Expand Up @@ -79,7 +80,7 @@ func FromMetrics(md pmetric.Metrics, settings Settings) (tsMap map[string]*promp
if dataPoints.Len() == 0 {
errs = multierr.Append(errs, fmt.Errorf("empty data points. %s is dropped", metric.Name()))
}
name := prometheustranslator.BuildPromCompliantName(metric, settings.Namespace)
name := prometheustranslator.BuildCompliantName(metric, settings.Namespace, settings.AddMetricSuffixes)
for x := 0; x < dataPoints.Len(); x++ {
errs = multierr.Append(
errs,
Expand Down
4 changes: 2 additions & 2 deletions pkg/translator/prometheusremotewrite/number_data_points.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func addSingleGaugeNumberDataPoint(
settings Settings,
series map[string]*prompb.TimeSeries,
) {
name := prometheustranslator.BuildPromCompliantName(metric, settings.Namespace)
name := prometheustranslator.BuildCompliantName(metric, settings.Namespace, settings.AddMetricSuffixes)
labels := createAttributes(
resource,
pt.Attributes(),
Expand Down Expand Up @@ -58,7 +58,7 @@ func addSingleSumNumberDataPoint(
settings Settings,
series map[string]*prompb.TimeSeries,
) {
name := prometheustranslator.BuildPromCompliantName(metric, settings.Namespace)
name := prometheustranslator.BuildCompliantName(metric, settings.Namespace, settings.AddMetricSuffixes)
labels := createAttributes(
resource,
pt.Attributes(),
Expand Down

0 comments on commit 2bc9904

Please sign in to comment.