diff --git a/docs/user_guide/outputs/otlp_output.md b/docs/user_guide/outputs/otlp_output.md index a8feb753d..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 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 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 ca0781fae..a1a46cc38 100644 --- a/pkg/outputs/otlp_output/otlp_converter.go +++ b/pkg/outputs/otlp_output/otlp_converter.go @@ -193,15 +193,27 @@ 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. 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 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 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{ - Gauge: o.createGaugeWithString(event, attributes, v), + Gauge: o.createGaugeWithString(event, attributes, strVal), } result = append(result, metric) continue @@ -213,27 +225,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..bdcd624cf 100644 --- a/pkg/outputs/otlp_output/otlp_output.go +++ b/pkg/outputs/otlp_output/otlp_output.go @@ -186,10 +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, 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, 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 @@ -828,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 @@ -856,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 d6cc183ea..d6b1730e6 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,128 @@ 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"]) +} + +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", + 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()) +} + +// 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") @@ -787,9 +905,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 "" }