From d71dd3f3cc4ac48a1fbe5ccb5ba0c56d2af8800e Mon Sep 17 00:00:00 2001 From: Marc Netterfield Date: Fri, 17 Jul 2026 16:03:03 -0700 Subject: [PATCH 1/2] fix(otlp_output): parse RFC 7951 numeric JSON strings as metric values json_ietf encodes YANG uint64/int64/decimal64 as JSON strings. The OTLP converter previously treated every string as an enum (value=1 + value attribute), causing wrong Prom values and label cardinality explosion. Prefer numeric datapoints when the string parses, matching prometheus_output. --- docs/user_guide/outputs/otlp_output.md | 2 +- pkg/outputs/otlp_output/otlp_converter.go | 51 ++++++++------ pkg/outputs/otlp_output/otlp_output.go | 8 ++- pkg/outputs/otlp_output/otlp_output_test.go | 78 ++++++++++++++++++--- 4 files changed, 106 insertions(+), 33 deletions(-) diff --git a/docs/user_guide/outputs/otlp_output.md b/docs/user_guide/outputs/otlp_output.md index a8feb753d..ebbc69baf 100644 --- a/docs/user_guide/outputs/otlp_output.md +++ b/docs/user_guide/outputs/otlp_output.md @@ -61,7 +61,7 @@ outputs: | `metric-prefix` | unset | Prefix added to generated metric names. | | `append-subscription-name` | `false` | Adds the subscription name to generated metric names. | | `strip-leading-underscore` | `false` | Removes a leading `/` from the gNMI path before `/` is converted to `_`. | -| `strings-as-attributes` | `false` | Exports string values as gauge metrics with value `1` and a `value` data point attribute. If false, string values are dropped. | +| `strings-as-attributes` | `false` | Exports **non-numeric** string values as gauge metrics with value `1` and a `value` data point attribute. Numeric strings (RFC 7951 encodes `uint64`/`int64`/`decimal64` as JSON strings under `json_ietf`) are always emitted as numeric datapoints. If false, non-numeric string values are dropped. | | `resource-tag-keys` | unset | Tags to place on the OTLP Resource instead of the data point. | | `counter-patterns` | unset | Regex patterns matched against value names. Matching values are exported as monotonic cumulative Sums. | | `resource-attributes` | unset | Static attributes added to every OTLP Resource. | diff --git a/pkg/outputs/otlp_output/otlp_converter.go b/pkg/outputs/otlp_output/otlp_converter.go index ca0781fae..4c7be8c96 100644 --- a/pkg/outputs/otlp_output/otlp_converter.go +++ b/pkg/outputs/otlp_output/otlp_converter.go @@ -193,15 +193,21 @@ func (o *otlpOutput) convertEventToMetrics(cfg *config, divergent map[string]boo Name: metricName, } - // Handle string values - switch v := v.(type) { - case string: + // RFC 7951 (JSON_IETF) encodes YANG uint64/int64/decimal64 as JSON + // strings. Prefer a numeric datapoint whenever the string parses as a + // number; only fall back to value=1 + "value" attribute for true enums. + if strVal, ok := v.(string); ok { + if dataPoint := o.createNumberDataPointWithValue(cfg, event, attributes, strVal); dataPoint != nil { + o.assignDataPoint(cfg, metric, k, dataPoint) + result = append(result, metric) + continue + } if !cfg.StringsAsAttributes { - o.logger.Debug("skipping string value (strings-as-attributes=false)", "event", event.Name) + o.logger.Debug("skipping non-numeric string value (strings-as-attributes=false)", "event", event.Name) continue } metric.Data = &metricspb.Metric_Gauge{ - Gauge: o.createGaugeWithString(event, attributes, v), + Gauge: o.createGaugeWithString(event, attributes, strVal), } result = append(result, metric) continue @@ -213,27 +219,32 @@ func (o *otlpOutput) convertEventToMetrics(cfg *config, divergent map[string]boo continue } - if o.isCounter(cfg, k) { - metric.Data = &metricspb.Metric_Sum{ - Sum: &metricspb.Sum{ - AggregationTemporality: metricspb.AggregationTemporality_AGGREGATION_TEMPORALITY_CUMULATIVE, - IsMonotonic: true, - DataPoints: []*metricspb.NumberDataPoint{dataPoint}, - }, - } - } else { - metric.Data = &metricspb.Metric_Gauge{ - Gauge: &metricspb.Gauge{ - DataPoints: []*metricspb.NumberDataPoint{dataPoint}, - }, - } - } + o.assignDataPoint(cfg, metric, k, dataPoint) result = append(result, metric) } return result, nil } +// assignDataPoint sets metric.Data to either a Sum or Gauge based on counter-patterns. +func (o *otlpOutput) assignDataPoint(cfg *config, metric *metricspb.Metric, valueKey string, dataPoint *metricspb.NumberDataPoint) { + if o.isCounter(cfg, valueKey) { + metric.Data = &metricspb.Metric_Sum{ + Sum: &metricspb.Sum{ + AggregationTemporality: metricspb.AggregationTemporality_AGGREGATION_TEMPORALITY_CUMULATIVE, + IsMonotonic: true, + DataPoints: []*metricspb.NumberDataPoint{dataPoint}, + }, + } + return + } + metric.Data = &metricspb.Metric_Gauge{ + Gauge: &metricspb.Gauge{ + DataPoints: []*metricspb.NumberDataPoint{dataPoint}, + }, + } +} + // buildMetricName creates metric name from event and value key // event.Name contains the subscription name (e.g., "nvos", "arista") // valueKey contains the metric path (e.g., "interfaces/interface/state/counters/in-octets") diff --git a/pkg/outputs/otlp_output/otlp_output.go b/pkg/outputs/otlp_output/otlp_output.go index 5b969c8c3..01c4fd71f 100644 --- a/pkg/outputs/otlp_output/otlp_output.go +++ b/pkg/outputs/otlp_output/otlp_output.go @@ -186,9 +186,11 @@ type config struct { MetricPrefix string `mapstructure:"metric-prefix,omitempty"` // boolean, if true the subscription name will be prepended to the metric name after the prefix. AppendSubscriptionName bool `mapstructure:"append-subscription-name,omitempty"` - // boolean, if true, string type values are exported as gauge metrics with value=1 - // and the string stored as an attribute named "value". - // if false, string values are dropped. + // boolean, if true, non-numeric string values are exported as gauge metrics + // with value=1 and the string stored as an attribute named "value". + // Numeric strings (RFC 7951 encodes uint64/int64/decimal64 as JSON strings) + // are always exported as numeric datapoints regardless of this setting. + // if false, non-numeric string values are dropped. StringsAsAttributes bool `mapstructure:"strings-as-attributes,omitempty"` // boolean, if true, the leading "/" of the metric path is trimmed before the // slash-to-underscore conversion, so a path like "/interfaces/..." becomes diff --git a/pkg/outputs/otlp_output/otlp_output_test.go b/pkg/outputs/otlp_output/otlp_output_test.go index d6cc183ea..f013616c8 100644 --- a/pkg/outputs/otlp_output/otlp_output_test.go +++ b/pkg/outputs/otlp_output/otlp_output_test.go @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/require" metricsv1 "go.opentelemetry.io/proto/otlp/collector/metrics/v1" commonpb "go.opentelemetry.io/proto/otlp/common/v1" + metricspb "go.opentelemetry.io/proto/otlp/metrics/v1" "google.golang.org/grpc" "google.golang.org/grpc/connectivity" "google.golang.org/protobuf/proto" @@ -347,10 +348,7 @@ func TestOTLP_ConfigValidation(t *testing.T) { // Test 7: String Values as Attributes func TestOTLP_StringValuesAsAttributes(t *testing.T) { - t.Skip("Implementation pending") - - // Test strings-as-attributes conversion - // Given: String value metric + // Non-numeric enums stay on the value=1 + "value" attribute path. event := &formatters.EventMsg{ Name: "interfaces_interface_state_oper_status", Timestamp: time.Now().UnixNano(), @@ -362,11 +360,9 @@ func TestOTLP_StringValuesAsAttributes(t *testing.T) { }, } - // When: Converting with strings-as-attributes enabled output := newTestOutput(&config{StringsAsAttributes: true}) otlpMetrics := output.convertToOTLP(output.state.Load().cfg, []*formatters.EventMsg{event}) - // Then: Should create gauge with value=1 and status as attribute metric := otlpMetrics.ResourceMetrics[0].ScopeMetrics[0].Metrics[0] gauge := metric.GetGauge() require.NotNil(t, gauge) @@ -376,6 +372,66 @@ func TestOTLP_StringValuesAsAttributes(t *testing.T) { assert.Equal(t, "up", getDataPointAttribute(dataPoint, "value")) } +// TestOTLP_NumericStringValues_RFC7951 verifies that JSON_IETF-style numeric +// strings (YANG uint64/int64/decimal64) become real numeric datapoints, matching +// prometheus_output behavior, instead of value=1 + a cardinality-exploding label. +func TestOTLP_NumericStringValues_RFC7951(t *testing.T) { + event := &formatters.EventMsg{ + Name: "interface_stats", + Timestamp: time.Now().UnixNano(), + Tags: map[string]string{ + "interface_name": "ethernet-1/1", + "source": "leaf1", + }, + Values: map[string]interface{}{ + "interface/statistics/in-octets": "4597", + "interface/statistics/out-octets": "144925", + "platform/control/memory/free": "9625336000", + }, + } + + output := newTestOutput(&config{StringsAsAttributes: true}) + otlpMetrics := output.convertToOTLP(output.state.Load().cfg, []*formatters.EventMsg{event}) + require.Len(t, otlpMetrics.ResourceMetrics, 1) + metrics := otlpMetrics.ResourceMetrics[0].ScopeMetrics[0].Metrics + require.Len(t, metrics, 3) + + got := map[string]float64{} + for _, m := range metrics { + gauge := m.GetGauge() + require.NotNil(t, gauge, "metric %s should be a gauge", m.Name) + require.Len(t, gauge.DataPoints, 1) + assert.Empty(t, getDataPointAttribute(gauge.DataPoints[0], "value"), + "numeric string must not use value attribute: %s", m.Name) + got[m.Name] = gauge.DataPoints[0].GetAsDouble() + } + + assert.Equal(t, 4597.0, got["interface_statistics_in_octets"]) + assert.Equal(t, 144925.0, got["interface_statistics_out_octets"]) + assert.Equal(t, 9625336000.0, got["platform_control_memory_free"]) +} + +// TestOTLP_NumericStringValues_WithoutStringsAsAttributes ensures RFC7951 +// numeric strings are still emitted when strings-as-attributes is false. +func TestOTLP_NumericStringValues_WithoutStringsAsAttributes(t *testing.T) { + event := &formatters.EventMsg{ + Name: "system_resources", + Timestamp: time.Now().UnixNano(), + Values: map[string]interface{}{ + "memory/free": "1000", + "state": "enabled", // non-numeric; should be dropped + }, + } + + output := newTestOutput(&config{StringsAsAttributes: false}) + otlpMetrics := output.convertToOTLP(output.state.Load().cfg, []*formatters.EventMsg{event}) + require.Len(t, otlpMetrics.ResourceMetrics, 1) + metrics := otlpMetrics.ResourceMetrics[0].ScopeMetrics[0].Metrics + require.Len(t, metrics, 1) + assert.Equal(t, "memory_free", metrics[0].Name) + assert.Equal(t, 1000.0, metrics[0].GetGauge().DataPoints[0].GetAsDouble()) +} + // Test 8: Subscription Name Mapping func TestOTLP_SubscriptionNameMapping(t *testing.T) { t.Skip("Implementation pending") @@ -787,9 +843,13 @@ func getAttributeValue(resource interface{}, key string) string { return "" } -func getDataPointAttribute(dataPoint interface{}, key string) string { - // Helper to extract attribute value from data point - // Will implement when we have the actual OTLP structures +func getDataPointAttribute(dataPoint *metricspb.NumberDataPoint, key string) string { + if dataPoint == nil { + return "" + } + if v, ok := getAttr(dataPoint.Attributes, key); ok { + return v + } return "" } From 28dbc1a2dae3634cf774ece4b5cbde7bfe2c48b7 Mon Sep 17 00:00:00 2001 From: Marc Netterfield Date: Wed, 14 Oct 2026 07:16:34 -0700 Subject: [PATCH 2/2] fix(otlp_output): gate RFC 7951 numeric strings with parse-numeric-strings The review asked for a dedicated option so numeric JSON strings do not bypass strings-as-attributes. The option defaults to true, so json_ietf uint64/int64/decimal64 stay numeric samples on the default config. Set it false to restore the previous drop-or-attribute behavior. Co-authored-by: Cursor --- docs/user_guide/outputs/otlp_output.md | 15 ++++- pkg/outputs/otlp_output/otlp_converter.go | 20 ++++--- pkg/outputs/otlp_output/otlp_output.go | 32 +++++++++-- pkg/outputs/otlp_output/otlp_output_test.go | 62 +++++++++++++++++++++ 4 files changed, 116 insertions(+), 13 deletions(-) diff --git a/docs/user_guide/outputs/otlp_output.md b/docs/user_guide/outputs/otlp_output.md index ebbc69baf..3278d478e 100644 --- a/docs/user_guide/outputs/otlp_output.md +++ b/docs/user_guide/outputs/otlp_output.md @@ -61,7 +61,8 @@ outputs: | `metric-prefix` | unset | Prefix added to generated metric names. | | `append-subscription-name` | `false` | Adds the subscription name to generated metric names. | | `strip-leading-underscore` | `false` | Removes a leading `/` from the gNMI path before `/` is converted to `_`. | -| `strings-as-attributes` | `false` | Exports **non-numeric** string values as gauge metrics with value `1` and a `value` data point attribute. Numeric strings (RFC 7951 encodes `uint64`/`int64`/`decimal64` as JSON strings under `json_ietf`) are always emitted as numeric datapoints. If false, non-numeric string values are dropped. | +| `strings-as-attributes` | `false` | Exports string values that were not parsed as numbers as gauge metrics with value `1` and a `value` data point attribute. If false, those strings are dropped. Numeric JSON strings are handled by `parse-numeric-strings`. | +| `parse-numeric-strings` | `true` | When true, a JSON string that parses as a number is exported as a numeric datapoint, independent of `strings-as-attributes`. RFC 7951 encodes YANG `uint64`, `int64`, and `decimal64` as JSON strings under `json_ietf`, so this is what makes those counters appear on the default config. Set to `false` to subject every string only to `strings-as-attributes`. A YANG string leaf whose text happens to be numeric is also exported as a number; the leaf type is not available after JSON decode. | | `resource-tag-keys` | unset | Tags to place on the OTLP Resource instead of the data point. | | `counter-patterns` | unset | Regex patterns matched against value names. Matching values are exported as monotonic cumulative Sums. | | `resource-attributes` | unset | Static attributes added to every OTLP Resource. | @@ -171,6 +172,18 @@ With `strip-leading-underscore: true`, a value key that begins with `/` has that Boolean gNMI values are exported as numeric data points with `0` or `1` (integer), classified as a Gauge or Sum according to `counter-patterns` like other numeric types. +## Numeric strings + +`json_ietf` encodes YANG `uint64`, `int64`, and `decimal64` as JSON strings ([RFC 7951](https://www.rfc-editor.org/rfc/rfc7951)). With `parse-numeric-strings` left at its default (`true`), those values are exported as numeric datapoints. This matches the Prometheus output, which parses numeric strings before applying `strings-as-labels`. + +Enums and other non-numeric strings are unchanged: dropped when `strings-as-attributes` is false, or exported as a gauge of `1` with a `value` attribute when it is true. + +Set `parse-numeric-strings: false` to restore the previous OTLP behavior, where every string is subject only to `strings-as-attributes`. On that setting, 64-bit counters from a `json_ietf` subscription are dropped, or stored as the attribute `value` with a datapoint of `1` when `strings-as-attributes` is true. + +A string-typed YANG leaf whose text is numeric (for example `"100"`) is also exported as a number when parsing is enabled. Distinguishing that case requires the YANG type, which is not present after the value has been decoded from JSON. + +This default is a behavior change for existing `json_ietf` users: series that were previously absent will appear. + ## Metric Types By default, numeric values are exported as Gauges. diff --git a/pkg/outputs/otlp_output/otlp_converter.go b/pkg/outputs/otlp_output/otlp_converter.go index 4c7be8c96..a1a46cc38 100644 --- a/pkg/outputs/otlp_output/otlp_converter.go +++ b/pkg/outputs/otlp_output/otlp_converter.go @@ -194,16 +194,22 @@ func (o *otlpOutput) convertEventToMetrics(cfg *config, divergent map[string]boo } // RFC 7951 (JSON_IETF) encodes YANG uint64/int64/decimal64 as JSON - // strings. Prefer a numeric datapoint whenever the string parses as a - // number; only fall back to value=1 + "value" attribute for true enums. + // strings. When parse-numeric-strings is on (the default), a string + // that parses as a number becomes a numeric datapoint before + // strings-as-attributes is consulted. That flag only applies to + // strings that do not parse: dropped, or value=1 plus a "value" + // attribute. Set parse-numeric-strings false to skip the parse and + // send every string through strings-as-attributes. if strVal, ok := v.(string); ok { - if dataPoint := o.createNumberDataPointWithValue(cfg, event, attributes, strVal); dataPoint != nil { - o.assignDataPoint(cfg, metric, k, dataPoint) - result = append(result, metric) - continue + if cfg.parseNumericStringsEnabled() { + if dataPoint := o.createNumberDataPointWithValue(cfg, event, attributes, strVal); dataPoint != nil { + o.assignDataPoint(cfg, metric, k, dataPoint) + result = append(result, metric) + continue + } } if !cfg.StringsAsAttributes { - o.logger.Debug("skipping non-numeric string value (strings-as-attributes=false)", "event", event.Name) + o.logger.Debug("skipping string value (parse-numeric-strings did not apply, strings-as-attributes=false)", "event", event.Name) continue } metric.Data = &metricspb.Metric_Gauge{ diff --git a/pkg/outputs/otlp_output/otlp_output.go b/pkg/outputs/otlp_output/otlp_output.go index 01c4fd71f..bdcd624cf 100644 --- a/pkg/outputs/otlp_output/otlp_output.go +++ b/pkg/outputs/otlp_output/otlp_output.go @@ -186,12 +186,20 @@ type config struct { MetricPrefix string `mapstructure:"metric-prefix,omitempty"` // boolean, if true the subscription name will be prepended to the metric name after the prefix. AppendSubscriptionName bool `mapstructure:"append-subscription-name,omitempty"` - // boolean, if true, non-numeric string values are exported as gauge metrics - // with value=1 and the string stored as an attribute named "value". - // Numeric strings (RFC 7951 encodes uint64/int64/decimal64 as JSON strings) - // are always exported as numeric datapoints regardless of this setting. - // if false, non-numeric string values are dropped. + // boolean, if true, string values that were not parsed as numbers are + // exported as gauge metrics with value=1 and the string stored as an + // attribute named "value". If false, those strings are dropped. + // Numeric JSON strings are handled by parse-numeric-strings first. StringsAsAttributes bool `mapstructure:"strings-as-attributes,omitempty"` + // ParseNumericStrings controls whether a JSON string that parses as a + // number is exported as a numeric datapoint. Nil means the default, true. + // RFC 7951 encodes YANG uint64/int64/decimal64 as JSON strings under + // json_ietf; with the default, those counters are numeric samples even + // when strings-as-attributes is false. Set to false to subject every + // string only to strings-as-attributes (the previous OTLP behavior). + // A YANG string leaf whose text happens to be numeric is also exported + // as a number; the leaf type is not available after JSON decode. + ParseNumericStrings *bool `mapstructure:"parse-numeric-strings,omitempty"` // boolean, if true, the leading "/" of the metric path is trimmed before the // slash-to-underscore conversion, so a path like "/interfaces/..." becomes // "interfaces_..." instead of "_interfaces_...". Defaults to false for @@ -830,6 +838,16 @@ func (o *otlpOutput) recordBatchSent(cfg *config, count int, start time.Time) { otlpSendDuration.WithLabelValues(cfg.Name).Observe(time.Since(start).Seconds()) } +// parseNumericStringsEnabled reports whether JSON strings that parse as +// numbers should become numeric datapoints. A nil pointer is the default, +// which is true, including for tests that build a config without Init. +func (c *config) parseNumericStringsEnabled() bool { + if c == nil || c.ParseNumericStrings == nil { + return true + } + return *c.ParseNumericStrings +} + func (o *otlpOutput) setDefaultsFor(c *config) { if c.Timeout == 0 { c.Timeout = defaultTimeout @@ -858,6 +876,10 @@ func (o *otlpOutput) setDefaultsFor(c *config) { if c.BufferSize == 0 { c.BufferSize = c.BatchSize * 2 } + if c.ParseNumericStrings == nil { + v := true + c.ParseNumericStrings = &v + } c.resourceTagSet = make(map[string]bool, len(c.ResourceTagKeys)) for _, k := range c.ResourceTagKeys { c.resourceTagSet[k] = true diff --git a/pkg/outputs/otlp_output/otlp_output_test.go b/pkg/outputs/otlp_output/otlp_output_test.go index f013616c8..d6b1730e6 100644 --- a/pkg/outputs/otlp_output/otlp_output_test.go +++ b/pkg/outputs/otlp_output/otlp_output_test.go @@ -411,8 +411,11 @@ func TestOTLP_NumericStringValues_RFC7951(t *testing.T) { assert.Equal(t, 9625336000.0, got["platform_control_memory_free"]) } +func boolPtr(v bool) *bool { return &v } + // TestOTLP_NumericStringValues_WithoutStringsAsAttributes ensures RFC7951 // numeric strings are still emitted when strings-as-attributes is false. +// parse-numeric-strings defaults to true when unset. func TestOTLP_NumericStringValues_WithoutStringsAsAttributes(t *testing.T) { event := &formatters.EventMsg{ Name: "system_resources", @@ -432,6 +435,65 @@ func TestOTLP_NumericStringValues_WithoutStringsAsAttributes(t *testing.T) { assert.Equal(t, 1000.0, metrics[0].GetGauge().DataPoints[0].GetAsDouble()) } +// TestOTLP_ParseNumericStringsDisabled restores the pre-fix path: with the +// option off, a numeric JSON string is just a string. Dropped when +// strings-as-attributes is false, value=1 plus a value attribute when true. +func TestOTLP_ParseNumericStringsDisabled(t *testing.T) { + event := &formatters.EventMsg{ + Name: "interface_stats", + Timestamp: time.Now().UnixNano(), + Values: map[string]interface{}{ + "interface/statistics/in-octets": "4597", + "state": "up", + }, + } + + dropped := newTestOutput(&config{ParseNumericStrings: boolPtr(false)}) + otlpMetrics := dropped.convertToOTLP(dropped.state.Load().cfg, []*formatters.EventMsg{event}) + require.Empty(t, otlpMetrics.ResourceMetrics) + + asAttr := newTestOutput(&config{ + ParseNumericStrings: boolPtr(false), + StringsAsAttributes: true, + }) + otlpMetrics = asAttr.convertToOTLP(asAttr.state.Load().cfg, []*formatters.EventMsg{event}) + require.Len(t, otlpMetrics.ResourceMetrics, 1) + metrics := otlpMetrics.ResourceMetrics[0].ScopeMetrics[0].Metrics + require.Len(t, metrics, 2) + + got := map[string]string{} + for _, m := range metrics { + gauge := m.GetGauge() + require.NotNil(t, gauge, "metric %s should be a gauge", m.Name) + require.Len(t, gauge.DataPoints, 1) + assert.Equal(t, float64(1), gauge.DataPoints[0].GetAsDouble()) + got[m.Name] = getDataPointAttribute(gauge.DataPoints[0], "value") + } + assert.Equal(t, "4597", got["interface_statistics_in_octets"]) + assert.Equal(t, "up", got["state"]) +} + +func TestSetDefaults_ParseNumericStrings(t *testing.T) { + o := &otlpOutput{} + + unset := &config{} + require.NoError(t, outputs.DecodeConfig(map[string]any{}, unset)) + o.setDefaultsFor(unset) + require.NotNil(t, unset.ParseNumericStrings) + require.True(t, *unset.ParseNumericStrings) + + off := &config{} + require.NoError(t, outputs.DecodeConfig(map[string]any{"parse-numeric-strings": false}, off)) + o.setDefaultsFor(off) + require.NotNil(t, off.ParseNumericStrings) + require.False(t, *off.ParseNumericStrings) + + on := &config{} + require.NoError(t, outputs.DecodeConfig(map[string]any{"parse-numeric-strings": true}, on)) + o.setDefaultsFor(on) + require.True(t, *on.ParseNumericStrings) +} + // Test 8: Subscription Name Mapping func TestOTLP_SubscriptionNameMapping(t *testing.T) { t.Skip("Implementation pending")