From 074a57fabcca4eb8ef66c9ae771e5bea485499c2 Mon Sep 17 00:00:00 2001 From: edmocosta <11836452+edmocosta@users.noreply.github.com> Date: Fri, 13 Dec 2024 11:06:56 +0100 Subject: [PATCH 1/7] Change OTTL contexts to handle paths with context --- pkg/ottl/context_inferrer.go | 25 ++- pkg/ottl/context_inferrer_test.go | 18 ++ pkg/ottl/contexts/internal/metric.go | 6 +- pkg/ottl/contexts/internal/path.go | 13 ++ pkg/ottl/contexts/internal/path_test.go | 76 +++++++ pkg/ottl/contexts/internal/resource.go | 6 +- pkg/ottl/contexts/internal/scope.go | 7 +- pkg/ottl/contexts/internal/scope_test.go | 23 +++ pkg/ottl/contexts/internal/span.go | 17 +- pkg/ottl/contexts/ottldatapoint/datapoint.go | 57 +++++- .../contexts/ottldatapoint/datapoint_test.go | 185 ++++++++++++++++++ pkg/ottl/contexts/ottllog/log.go | 52 ++++- pkg/ottl/contexts/ottllog/log_test.go | 106 +++++++++- pkg/ottl/contexts/ottlmetric/metrics.go | 45 ++++- pkg/ottl/contexts/ottlmetric/metrics_test.go | 92 +++++++++ pkg/ottl/contexts/ottlresource/resource.go | 17 ++ .../contexts/ottlresource/resource_test.go | 11 ++ pkg/ottl/contexts/ottlscope/scope.go | 40 +++- pkg/ottl/contexts/ottlscope/scope_test.go | 95 ++++++++- pkg/ottl/contexts/ottlspan/span.go | 45 ++++- pkg/ottl/contexts/ottlspan/span_test.go | 132 ++++++++++++- .../contexts/ottlspanevent/span_events.go | 56 +++++- .../ottlspanevent/span_events_test.go | 168 +++++++++++++++- pkg/ottl/functions.go | 17 ++ pkg/ottl/functions_test.go | 15 ++ pkg/ottl/parser.go | 4 +- pkg/ottl/parser_test.go | 7 + 27 files changed, 1264 insertions(+), 71 deletions(-) create mode 100644 pkg/ottl/contexts/internal/path_test.go diff --git a/pkg/ottl/context_inferrer.go b/pkg/ottl/context_inferrer.go index da4ade783278..9dcb7afb1edd 100644 --- a/pkg/ottl/context_inferrer.go +++ b/pkg/ottl/context_inferrer.go @@ -37,14 +37,15 @@ func (s *priorityContextInferrer) infer(statements []string) (string, error) { } for _, p := range getParsedStatementPaths(parsed) { - pathContextPriority, ok := s.contextPriority[p.Context] + pathContext := s.getContextCandidate(p) + pathContextPriority, ok := s.contextPriority[pathContext] if !ok { // Lowest priority pathContextPriority = math.MaxInt } if inferredContext == "" || pathContextPriority < inferredContextPriority { - inferredContext = p.Context + inferredContext = pathContext inferredContextPriority = pathContextPriority } } @@ -53,6 +54,26 @@ func (s *priorityContextInferrer) infer(statements []string) (string, error) { return inferredContext, nil } +// When a path has no dots separators (e.g.: resource), the grammar extracts it into the +// path.Fields slice, letting the path.Context empty. This function returns either the +// path.Context string or, if it's eligible and meets certain conditions, the first +// path.Fields name. +func (s *priorityContextInferrer) getContextCandidate(p path) string { + if p.Context != "" { + return p.Context + } + // If it has multiple fields or keys, it means the path has at least one dot on it, + // and isn't a context access. + if len(p.Fields) != 1 || len(p.Fields[0].Keys) > 0 { + return "" + } + _, ok := s.contextPriority[p.Fields[0].Name] + if ok { + return p.Fields[0].Name + } + return "" +} + // defaultPriorityContextInferrer is like newPriorityContextInferrer, but using the default // context priorities and ignoring unknown/non-prioritized contexts. func defaultPriorityContextInferrer() contextInferrer { diff --git a/pkg/ottl/context_inferrer_test.go b/pkg/ottl/context_inferrer_test.go index 4d4455dd0dcf..af8937df86c1 100644 --- a/pkg/ottl/context_inferrer_test.go +++ b/pkg/ottl/context_inferrer_test.go @@ -49,6 +49,24 @@ func Test_NewPriorityContextInferrer_Infer(t *testing.T) { statements: []string{"set(span.foo, true) where span.bar == true"}, expected: "span", }, + { + name: "with context root access", + priority: []string{"resource", "foo"}, + statements: []string{"set(foo.attributes[\"body\"], resource)"}, + expected: "resource", + }, + { + name: "with non-eligible context root access", + priority: []string{"resource", "foo"}, + statements: []string{"set(foo.attributes[\"body\"], resource[\"foo\"])"}, + expected: "foo", + }, + { + name: "with non-prioritized context root access", + priority: []string{"foo"}, + statements: []string{"set(resource, bar.attributes[\"body\"])"}, + expected: "bar", + }, } for _, tt := range tests { diff --git a/pkg/ottl/contexts/internal/metric.go b/pkg/ottl/contexts/internal/metric.go index e2944a73df45..a61d406512be 100644 --- a/pkg/ottl/contexts/internal/metric.go +++ b/pkg/ottl/contexts/internal/metric.go @@ -11,6 +11,10 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) +const ( + MetricContextName = "metric" +) + type MetricContext interface { GetMetric() pmetric.Metric } @@ -28,7 +32,7 @@ var MetricSymbolTable = map[ottl.EnumSymbol]ottl.Enum{ } func MetricPathGetSetter[K MetricContext](path ottl.Path[K]) (ottl.GetSetter[K], error) { - if path == nil { + if isPathToContextRoot(path, MetricContextName) { return accessMetric[K](), nil } switch path.Name() { diff --git a/pkg/ottl/contexts/internal/path.go b/pkg/ottl/contexts/internal/path.go index 954d14329646..99b4ebcc9c35 100644 --- a/pkg/ottl/contexts/internal/path.go +++ b/pkg/ottl/contexts/internal/path.go @@ -55,3 +55,16 @@ func (k *TestKey[K]) String(_ context.Context, _ K) (*string, error) { func (k *TestKey[K]) Int(_ context.Context, _ K) (*int64, error) { return k.I, nil } + +// isPathToContextRoot return true if the given path is accessing the context's root object +// instead of specific fields. +func isPathToContextRoot[T any](path ottl.Path[T], ctx string) bool { + if path == nil { + return true + } + // path with matching context and empty name/keys/next + return path.Context() == ctx && + path.Name() == "" && + len(path.Keys()) == 0 && + path.Next() == nil +} diff --git a/pkg/ottl/contexts/internal/path_test.go b/pkg/ottl/contexts/internal/path_test.go new file mode 100644 index 000000000000..f169fbce6098 --- /dev/null +++ b/pkg/ottl/contexts/internal/path_test.go @@ -0,0 +1,76 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package internal + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" +) + +func Test_isPathToContextRoot(t *testing.T) { + tests := []struct { + name string + context string + path ottl.Path[any] + expected bool + }{ + { + name: "with context and empty path", + context: "resource", + expected: true, + path: &TestPath[any]{ + C: "resource", + N: "", + }, + }, + { + name: "with context and next path", + context: "resource", + expected: false, + path: &TestPath[any]{ + C: "resource", + N: "", + NextPath: &TestPath[any]{ + N: "foo", + }, + }, + }, + { + name: "with context and path keys", + context: "resource", + expected: false, + path: &TestPath[any]{ + C: "resource", + N: "", + KeySlice: []ottl.Key[any]{ + &TestKey[any]{}, + }, + }, + }, + { + name: "with both context and path name", + context: "resource", + expected: false, + path: &TestPath[any]{ + C: "resource", + N: "resource", + }, + }, + { + name: "with nil path", + context: "resource", + expected: true, + path: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, isPathToContextRoot(tt.path, tt.context)) + }) + } +} diff --git a/pkg/ottl/contexts/internal/resource.go b/pkg/ottl/contexts/internal/resource.go index 2dfee7fce9f8..beee3b0cb534 100644 --- a/pkg/ottl/contexts/internal/resource.go +++ b/pkg/ottl/contexts/internal/resource.go @@ -11,13 +11,17 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) +const ( + ResourceContextName = "resource" +) + type ResourceContext interface { GetResource() pcommon.Resource GetResourceSchemaURLItem() SchemaURLItem } func ResourcePathGetSetter[K ResourceContext](path ottl.Path[K]) (ottl.GetSetter[K], error) { - if path == nil { + if isPathToContextRoot(path, ResourceContextName) { return accessResource[K](), nil } switch path.Name() { diff --git a/pkg/ottl/contexts/internal/scope.go b/pkg/ottl/contexts/internal/scope.go index 6bc5d7352005..ebc47f464d1b 100644 --- a/pkg/ottl/contexts/internal/scope.go +++ b/pkg/ottl/contexts/internal/scope.go @@ -11,13 +11,18 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) +const ( + InstrumentationScopeContextName = "instrumentation_scope" + ScopeContextName = "scope" +) + type InstrumentationScopeContext interface { GetInstrumentationScope() pcommon.InstrumentationScope GetScopeSchemaURLItem() SchemaURLItem } func ScopePathGetSetter[K InstrumentationScopeContext](path ottl.Path[K]) (ottl.GetSetter[K], error) { - if path == nil { + if isPathToContextRoot(path, InstrumentationScopeContextName) || isPathToContextRoot(path, ScopeContextName) { return accessInstrumentationScope[K](), nil } switch path.Name() { diff --git a/pkg/ottl/contexts/internal/scope_test.go b/pkg/ottl/contexts/internal/scope_test.go index 1f9837fc8d2b..51f2e3147885 100644 --- a/pkg/ottl/contexts/internal/scope_test.go +++ b/pkg/ottl/contexts/internal/scope_test.go @@ -343,6 +343,29 @@ func TestScopePathGetSetter(t *testing.T) { s.AppendEmpty().SetEmptySlice().AppendEmpty().SetStr("new") }, }, + { + name: "scope", + path: &TestPath[*instrumentationScopeContext]{ + C: "scope", + }, + orig: refIS, + newVal: pcommon.NewInstrumentationScope(), + modified: func(is pcommon.InstrumentationScope) { + pcommon.NewInstrumentationScope().CopyTo(is) + }, + }, + { + name: "scope field", + path: &TestPath[*instrumentationScopeContext]{ + C: "scope", + N: "name", + }, + orig: refIS.Name(), + newVal: "newname", + modified: func(is pcommon.InstrumentationScope) { + is.SetName("newname") + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/ottl/contexts/internal/span.go b/pkg/ottl/contexts/internal/span.go index 607cb2e110f9..63feb19cf69a 100644 --- a/pkg/ottl/contexts/internal/span.go +++ b/pkg/ottl/contexts/internal/span.go @@ -18,7 +18,8 @@ import ( ) const ( - SpanContextName = "Span" + SpanContextName = "span" + SpanContextNameDescription = "Span" ) type SpanContext interface { @@ -38,7 +39,7 @@ var SpanSymbolTable = map[ottl.EnumSymbol]ottl.Enum{ } func SpanPathGetSetter[K SpanContext](path ottl.Path[K]) (ottl.GetSetter[K], error) { - if path == nil { + if isPathToContextRoot(path, SpanContextName) { return accessSpan[K](), nil } switch path.Name() { @@ -48,7 +49,7 @@ func SpanPathGetSetter[K SpanContext](path ottl.Path[K]) (ottl.GetSetter[K], err if nextPath.Name() == "string" { return accessStringTraceID[K](), nil } - return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextName, SpanRef) + return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextNameDescription, SpanRef) } return accessTraceID[K](), nil case "span_id": @@ -57,7 +58,7 @@ func SpanPathGetSetter[K SpanContext](path ottl.Path[K]) (ottl.GetSetter[K], err if nextPath.Name() == "string" { return accessStringSpanID[K](), nil } - return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextName, SpanRef) + return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextNameDescription, SpanRef) } return accessSpanID[K](), nil case "trace_state": @@ -72,7 +73,7 @@ func SpanPathGetSetter[K SpanContext](path ottl.Path[K]) (ottl.GetSetter[K], err if nextPath.Name() == "string" { return accessStringParentSpanID[K](), nil } - return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextName, SpanRef) + return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextNameDescription, SpanRef) } return accessParentSpanID[K](), nil case "name": @@ -86,7 +87,7 @@ func SpanPathGetSetter[K SpanContext](path ottl.Path[K]) (ottl.GetSetter[K], err case "deprecated_string": return accessDeprecatedStringKind[K](), nil default: - return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextName, SpanRef) + return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextNameDescription, SpanRef) } } return accessKind[K](), nil @@ -123,12 +124,12 @@ func SpanPathGetSetter[K SpanContext](path ottl.Path[K]) (ottl.GetSetter[K], err case "message": return accessStatusMessage[K](), nil default: - return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextName, SpanRef) + return nil, FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), SpanContextNameDescription, SpanRef) } } return accessStatus[K](), nil default: - return nil, FormatDefaultErrorMessage(path.Name(), path.String(), SpanContextName, SpanRef) + return nil, FormatDefaultErrorMessage(path.Name(), path.String(), SpanContextNameDescription, SpanRef) } } diff --git a/pkg/ottl/contexts/ottldatapoint/datapoint.go b/pkg/ottl/contexts/ottldatapoint/datapoint.go index 9c50d85cd723..e0fa9bfeda86 100644 --- a/pkg/ottl/contexts/ottldatapoint/datapoint.go +++ b/pkg/ottl/contexts/ottldatapoint/datapoint.go @@ -20,7 +20,8 @@ import ( ) const ( - contextName = "DataPoint" + ContextName = "datapoint" + contextNameDescription = "DataPoint" ) var ( @@ -124,6 +125,20 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet return p, nil } +// EnablePathContextNames enables the support to path's context names on statements. +// When this option is configured, all statement's paths must have a valid context prefix, +// otherwise an error is reported. +func EnablePathContextNames() Option { + return func(p *ottl.Parser[TransformContext]) { + ottl.WithPathContextNames[TransformContext]([]string{ + ContextName, + internal.ResourceContextName, + internal.InstrumentationScopeContextName, + internal.MetricContextName, + })(p) + } +} + type StatementSequenceOption func(*ottl.StatementSequence[TransformContext]) func WithStatementSequenceErrorMode(errorMode ottl.ErrorMode) StatementSequenceOption { @@ -185,18 +200,23 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if path == nil { return nil, fmt.Errorf("path cannot be nil") } + // Higher contexts parsing + if path.Context() != "" && path.Context() != ContextName { + return pep.parseHigherContextPath(path.Context(), path) + } + // Backward compatibility with paths without context + if path.Context() == "" && (path.Name() == internal.ResourceContextName || + path.Name() == internal.InstrumentationScopeContextName || + path.Name() == internal.MetricContextName) { + return pep.parseHigherContextPath(path.Name(), path.Next()) + } + switch path.Name() { case "cache": if path.Keys() == nil { return accessCache(), nil } return accessCacheKey(path.Keys()), nil - case "resource": - return internal.ResourcePathGetSetter[TransformContext](path.Next()) - case "instrumentation_scope": - return internal.ScopePathGetSetter[TransformContext](path.Next()) - case "metric": - return internal.MetricPathGetSetter[TransformContext](path.Next()) case "attributes": if path.Keys() == nil { return accessAttributes(), nil @@ -239,7 +259,7 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot case "bucket_counts": return accessPositiveBucketCounts(), nil default: - return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), path.String(), contextName, internal.DataPointRef) + return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), path.String(), contextNameDescription, internal.DataPointRef) } } return accessPositive(), nil @@ -252,14 +272,31 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot case "bucket_counts": return accessNegativeBucketCounts(), nil default: - return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), path.String(), contextName, internal.DataPointRef) + return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), path.String(), contextNameDescription, internal.DataPointRef) } } return accessNegative(), nil case "quantile_values": return accessQuantileValues(), nil default: - return nil, internal.FormatDefaultErrorMessage(path.Name(), path.String(), contextName, internal.DataPointRef) + return nil, internal.FormatDefaultErrorMessage(path.Name(), path.String(), contextNameDescription, internal.DataPointRef) + } +} + +func (pep *pathExpressionParser) parseHigherContextPath(context string, path ottl.Path[TransformContext]) (ottl.GetSetter[TransformContext], error) { + switch context { + case internal.ResourceContextName: + return internal.ResourcePathGetSetter(path) + case internal.InstrumentationScopeContextName: + return internal.ScopePathGetSetter(path) + case internal.MetricContextName: + return internal.MetricPathGetSetter(path) + default: + var fullPath string + if path != nil { + fullPath = path.String() + } + return nil, internal.FormatDefaultErrorMessage(context, fullPath, contextNameDescription, internal.DataPointRef) } } diff --git a/pkg/ottl/contexts/ottldatapoint/datapoint_test.go b/pkg/ottl/contexts/ottldatapoint/datapoint_test.go index 1da5f4309e0a..eb4a7ed83d59 100644 --- a/pkg/ottl/contexts/ottldatapoint/datapoint_test.go +++ b/pkg/ottl/contexts/ottldatapoint/datapoint_test.go @@ -5,10 +5,12 @@ package ottldatapoint import ( "context" + "slices" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" @@ -57,6 +59,16 @@ func Test_newPathGetSetter_Cache(t *testing.T) { }, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -475,6 +487,16 @@ func Test_newPathGetSetter_NumberDataPoint(t *testing.T) { }, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -909,6 +931,16 @@ func Test_newPathGetSetter_HistogramDataPoint(t *testing.T) { }, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -1427,6 +1459,16 @@ func Test_newPathGetSetter_ExpoHistogramDataPoint(t *testing.T) { }, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -1846,6 +1888,16 @@ func Test_newPathGetSetter_SummaryDataPoint(t *testing.T) { }, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -1946,6 +1998,18 @@ func Test_newPathGetSetter_Metric(t *testing.T) { newMetric.CopyTo(metric) }, }, + { + name: "metric context", + path: &internal.TestPath[TransformContext]{ + C: "metric", + N: "", + }, + orig: refMetric, + newVal: newMetric, + modified: func(metric pmetric.Metric) { + newMetric.CopyTo(metric) + }, + }, { name: "metric name", path: &internal.TestPath[TransformContext]{ @@ -2029,6 +2093,17 @@ func Test_newPathGetSetter_Metric(t *testing.T) { metric.Sum().SetIsMonotonic(false) }, }, + { + name: "metric field with context", + path: &internal.TestPath[TransformContext]{ + C: "metric", + N: "type", + }, + orig: int64(pmetric.MetricTypeSum), + newVal: int64(pmetric.MetricTypeSum), + modified: func(_ pmetric.Metric) { + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2156,3 +2231,113 @@ func Test_ParseEnum_False(t *testing.T) { }) } } + +func Test_newPathGetSetter_higherContextPath(t *testing.T) { + resource := pcommon.NewResource() + resource.Attributes().PutStr("foo", "bar") + + metric := pmetric.NewMetric() + metric.SetName("metric") + + instrumentationScope := pcommon.NewInstrumentationScope() + instrumentationScope.SetName("instrumentation_scope") + + ctx := NewTransformContext( + pmetric.NewNumberDataPoint(), + metric, + pmetric.NewMetricSlice(), + instrumentationScope, + resource, + pmetric.NewScopeMetrics(), + pmetric.NewResourceMetrics()) + + tests := []struct { + name string + path ottl.Path[TransformContext] + expected any + }{ + { + name: "resource", + path: &internal.TestPath[TransformContext]{N: "resource"}, + expected: resource, + }, + { + name: "resource field", + path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ + N: "attributes", + KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }, + }}, + expected: "bar", + }, + { + name: "resource context", + path: &internal.TestPath[TransformContext]{C: "resource"}, + expected: resource, + }, + { + name: "resource field with context", + path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }}, + expected: "bar", + }, + { + name: "metric", + path: &internal.TestPath[TransformContext]{N: "metric"}, + expected: metric, + }, + { + name: "metric field", + path: &internal.TestPath[TransformContext]{N: "metric", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, + expected: metric.Name(), + }, + { + name: "metric context", + path: &internal.TestPath[TransformContext]{C: "metric"}, + expected: metric, + }, + { + name: "metric field with context", + path: &internal.TestPath[TransformContext]{C: "metric", N: "name"}, + expected: metric.Name(), + }, + { + name: "instrumentation_scope", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, + expected: instrumentationScope.Name(), + }, + { + name: "instrumentation_scope context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field with context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, + expected: instrumentationScope.Name(), + }, + } + + pep := pathExpressionParser{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + accessor, err := pep.parsePath(tt.path) + require.NoError(t, err) + + got, err := accessor.Get(context.Background(), ctx) + assert.NoError(t, err) + assert.Equal(t, tt.expected, got) + }) + } +} diff --git a/pkg/ottl/contexts/ottllog/log.go b/pkg/ottl/contexts/ottllog/log.go index 7ca056730cc7..4382f57520a3 100644 --- a/pkg/ottl/contexts/ottllog/log.go +++ b/pkg/ottl/contexts/ottllog/log.go @@ -22,7 +22,8 @@ import ( ) const ( - contextName = "Log" + ContextName = "log" + contextNameDescription = "Log" ) var ( @@ -121,6 +122,19 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet return p, nil } +// EnablePathContextNames enables the support to path's context names on statements. +// When this option is configured, all statement's paths must have a valid context prefix, +// otherwise an error is reported. +func EnablePathContextNames() Option { + return func(p *ottl.Parser[TransformContext]) { + ottl.WithPathContextNames[TransformContext]([]string{ + ContextName, + internal.InstrumentationScopeContextName, + internal.ResourceContextName, + })(p) + } +} + type StatementSequenceOption func(*ottl.StatementSequence[TransformContext]) func WithStatementSequenceErrorMode(errorMode ottl.ErrorMode) StatementSequenceOption { @@ -199,16 +213,21 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if path == nil { return nil, fmt.Errorf("path cannot be nil") } + // Higher contexts parsing + if path.Context() != "" && path.Context() != ContextName { + return pep.parseHigherContextPath(path.Context(), path) + } + // Backward compatibility with paths without context + if path.Context() == "" && (path.Name() == internal.ResourceContextName || path.Name() == internal.InstrumentationScopeContextName) { + return pep.parseHigherContextPath(path.Name(), path.Next()) + } + switch path.Name() { case "cache": if path.Keys() == nil { return accessCache(), nil } return accessCacheKey(path.Keys()), nil - case "resource": - return internal.ResourcePathGetSetter[TransformContext](path.Next()) - case "instrumentation_scope": - return internal.ScopePathGetSetter[TransformContext](path.Next()) case "time_unix_nano": return accessTimeUnixNano(), nil case "observed_time_unix_nano": @@ -227,7 +246,7 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if nextPath.Name() == "string" { return accessStringBody(), nil } - return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), contextName, internal.LogRef) + return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), contextNameDescription, internal.LogRef) } if path.Keys() == nil { return accessBody(), nil @@ -248,7 +267,7 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if nextPath.Name() == "string" { return accessStringTraceID(), nil } - return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), contextName, internal.LogRef) + return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), nextPath.String(), contextNameDescription, internal.LogRef) } return accessTraceID(), nil case "span_id": @@ -257,11 +276,26 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if nextPath.Name() == "string" { return accessStringSpanID(), nil } - return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), path.String(), contextName, internal.LogRef) + return nil, internal.FormatDefaultErrorMessage(nextPath.Name(), path.String(), contextNameDescription, internal.LogRef) } return accessSpanID(), nil default: - return nil, internal.FormatDefaultErrorMessage(path.Name(), path.String(), contextName, internal.LogRef) + return nil, internal.FormatDefaultErrorMessage(path.Name(), path.String(), contextNameDescription, internal.LogRef) + } +} + +func (pep *pathExpressionParser) parseHigherContextPath(context string, path ottl.Path[TransformContext]) (ottl.GetSetter[TransformContext], error) { + switch context { + case internal.ResourceContextName: + return internal.ResourcePathGetSetter(path) + case internal.InstrumentationScopeContextName: + return internal.ScopePathGetSetter(path) + default: + var fullPath string + if path != nil { + fullPath = path.String() + } + return nil, internal.FormatDefaultErrorMessage(context, fullPath, contextNameDescription, internal.LogRef) } } diff --git a/pkg/ottl/contexts/ottllog/log_test.go b/pkg/ottl/contexts/ottllog/log_test.go index a4efc98091be..9ab17c42c3e4 100644 --- a/pkg/ottl/contexts/ottllog/log_test.go +++ b/pkg/ottl/contexts/ottllog/log_test.go @@ -7,10 +7,12 @@ import ( "context" "encoding/hex" "fmt" + "slices" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/plog" @@ -51,12 +53,13 @@ func Test_newPathGetSetter(t *testing.T) { newMap["k2"] = newMap2 tests := []struct { - name string - path ottl.Path[TransformContext] - orig any - newVal any - modified func(log plog.LogRecord, il pcommon.InstrumentationScope, resource pcommon.Resource, cache pcommon.Map) - bodyType string + name string + path ottl.Path[TransformContext] + orig any + newVal any + modified func(log plog.LogRecord, il pcommon.InstrumentationScope, resource pcommon.Resource, cache pcommon.Map) + bodyType string + skipWithPathContextTest bool }{ { name: "time", @@ -606,6 +609,7 @@ func Test_newPathGetSetter(t *testing.T) { modified: func(_ plog.LogRecord, il pcommon.InstrumentationScope, _ pcommon.Resource, _ pcommon.Map) { pcommon.NewInstrumentationScope().CopyTo(il) }, + skipWithPathContextTest: true, }, { name: "resource", @@ -617,8 +621,23 @@ func Test_newPathGetSetter(t *testing.T) { modified: func(_ plog.LogRecord, _ pcommon.InstrumentationScope, resource pcommon.Resource, _ pcommon.Map) { pcommon.NewResource().CopyTo(resource) }, + skipWithPathContextTest: true, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + if tt.skipWithPathContextTest { + continue + } + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = &pathWithContext + tests = append(tests, testWithContext) + } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -648,6 +667,81 @@ func Test_newPathGetSetter(t *testing.T) { } } +func Test_newPathGetSetter_higherContextPath(t *testing.T) { + logRec, instrumentationScope, resource := createTelemetry("string") + ctx := NewTransformContext(logRec, instrumentationScope, resource, plog.NewScopeLogs(), plog.NewResourceLogs()) + + tests := []struct { + name string + path ottl.Path[TransformContext] + expected any + }{ + { + name: "resource", + path: &internal.TestPath[TransformContext]{N: "resource"}, + expected: resource, + }, + { + name: "resource field", + path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ + N: "attributes", + KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("str"), + }, + }, + }}, + expected: "val", + }, + { + name: "resource context", + path: &internal.TestPath[TransformContext]{C: "resource"}, + expected: resource, + }, + { + name: "resource field with context", + path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("str"), + }, + }}, + expected: "val", + }, + { + name: "instrumentation_scope", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, + expected: instrumentationScope.Name(), + }, + { + name: "instrumentation_scope context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field with context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, + expected: instrumentationScope.Name(), + }, + } + + pep := pathExpressionParser{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + accessor, err := pep.parsePath(tt.path) + require.NoError(t, err) + + got, err := accessor.Get(context.Background(), ctx) + assert.NoError(t, err) + assert.Equal(t, tt.expected, got) + }) + } +} + func createTelemetry(bodyType string) (plog.LogRecord, pcommon.InstrumentationScope, pcommon.Resource) { log := plog.NewLogRecord() log.SetTimestamp(pcommon.NewTimestampFromTime(time.UnixMilli(100))) diff --git a/pkg/ottl/contexts/ottlmetric/metrics.go b/pkg/ottl/contexts/ottlmetric/metrics.go index eba931c74404..4267b3976a1d 100644 --- a/pkg/ottl/contexts/ottlmetric/metrics.go +++ b/pkg/ottl/contexts/ottlmetric/metrics.go @@ -15,6 +15,10 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal" ) +const ( + ContextName = internal.MetricContextName +) + var ( _ internal.ResourceContext = TransformContext{} _ internal.InstrumentationScopeContext = TransformContext{} @@ -90,6 +94,19 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet return p, err } +// EnablePathContextNames enables the support to path's context names on statements. +// When this option is configured, all statement's paths must have a valid context prefix, +// otherwise an error is reported. +func EnablePathContextNames() Option { + return func(p *ottl.Parser[TransformContext]) { + ottl.WithPathContextNames[TransformContext]([]string{ + ContextName, + internal.InstrumentationScopeContextName, + internal.ResourceContextName, + })(p) + } +} + type StatementSequenceOption func(*ottl.StatementSequence[TransformContext]) func WithStatementSequenceErrorMode(errorMode ottl.ErrorMode) StatementSequenceOption { @@ -142,21 +159,41 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if path == nil { return nil, fmt.Errorf("path cannot be nil") } + // Higher contexts parsing + if path.Context() != "" && path.Context() != ContextName { + return pep.parseHigherContextPath(path.Context(), path) + } + // Backward compatibility with paths without context + if path.Context() == "" && (path.Name() == internal.ResourceContextName || path.Name() == internal.InstrumentationScopeContextName) { + return pep.parseHigherContextPath(path.Name(), path.Next()) + } + switch path.Name() { case "cache": if path.Keys() == nil { return accessCache(), nil } return accessCacheKey(path.Keys()), nil - case "resource": - return internal.ResourcePathGetSetter[TransformContext](path.Next()) - case "instrumentation_scope": - return internal.ScopePathGetSetter[TransformContext](path.Next()) default: return internal.MetricPathGetSetter[TransformContext](path) } } +func (pep *pathExpressionParser) parseHigherContextPath(context string, path ottl.Path[TransformContext]) (ottl.GetSetter[TransformContext], error) { + switch context { + case internal.ResourceContextName: + return internal.ResourcePathGetSetter(path) + case internal.InstrumentationScopeContextName: + return internal.ScopePathGetSetter(path) + default: + var fullPath string + if path != nil { + fullPath = path.String() + } + return nil, internal.FormatDefaultErrorMessage(context, fullPath, internal.MetricContextName, internal.MetricRef) + } +} + func accessCache() ottl.StandardGetSetter[TransformContext] { return ottl.StandardGetSetter[TransformContext]{ Getter: func(_ context.Context, tCtx TransformContext) (any, error) { diff --git a/pkg/ottl/contexts/ottlmetric/metrics_test.go b/pkg/ottl/contexts/ottlmetric/metrics_test.go index f83ead9e3a4b..0c60a9ca7b2e 100644 --- a/pkg/ottl/contexts/ottlmetric/metrics_test.go +++ b/pkg/ottl/contexts/ottlmetric/metrics_test.go @@ -5,9 +5,11 @@ package ottlmetric import ( "context" + "slices" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pmetric" @@ -140,6 +142,16 @@ func Test_newPathGetSetter(t *testing.T) { }, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -167,6 +179,86 @@ func Test_newPathGetSetter(t *testing.T) { } } +func Test_newPathGetSetter_higherContextPath(t *testing.T) { + resource := pcommon.NewResource() + resource.Attributes().PutStr("foo", "bar") + + instrumentationScope := pcommon.NewInstrumentationScope() + instrumentationScope.SetName("instrumentation_scope") + + ctx := NewTransformContext(pmetric.NewMetric(), pmetric.NewMetricSlice(), instrumentationScope, resource, pmetric.NewScopeMetrics(), pmetric.NewResourceMetrics()) + + tests := []struct { + name string + path ottl.Path[TransformContext] + expected any + }{ + { + name: "resource", + path: &internal.TestPath[TransformContext]{N: "resource"}, + expected: resource, + }, + { + name: "resource field", + path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ + N: "attributes", + KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }, + }}, + expected: "bar", + }, + { + name: "resource context", + path: &internal.TestPath[TransformContext]{C: "resource"}, + expected: resource, + }, + { + name: "resource field with context", + path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }}, + expected: "bar", + }, + { + name: "instrumentation_scope", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, + expected: instrumentationScope.Name(), + }, + { + name: "instrumentation_scope context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field with context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, + expected: instrumentationScope.Name(), + }, + } + + pep := pathExpressionParser{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + accessor, err := pep.parsePath(tt.path) + require.NoError(t, err) + + got, err := accessor.Get(context.Background(), ctx) + assert.NoError(t, err) + assert.Equal(t, tt.expected, got) + }) + } +} + func createMetricTelemetry() pmetric.Metric { metric := pmetric.NewMetric() metric.SetName("name") diff --git a/pkg/ottl/contexts/ottlresource/resource.go b/pkg/ottl/contexts/ottlresource/resource.go index da3a8ceea1b6..db120b774e26 100644 --- a/pkg/ottl/contexts/ottlresource/resource.go +++ b/pkg/ottl/contexts/ottlresource/resource.go @@ -17,6 +17,10 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/logging" ) +const ( + ContextName = internal.ResourceContextName +) + var ( _ internal.ResourceContext = (*TransformContext)(nil) _ zapcore.ObjectMarshaler = (*TransformContext)(nil) @@ -73,6 +77,15 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet return p, nil } +// EnablePathContextNames enables the support to path's context names on statements. +// When this option is configured, all statement's paths must have a valid context prefix, +// otherwise an error is reported. +func EnablePathContextNames() Option { + return func(p *ottl.Parser[TransformContext]) { + ottl.WithPathContextNames[TransformContext]([]string{ContextName})(p) + } +} + type StatementSequenceOption func(*ottl.StatementSequence[TransformContext]) func WithStatementSequenceErrorMode(errorMode ottl.ErrorMode) StatementSequenceOption { @@ -117,6 +130,10 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if path == nil { return nil, fmt.Errorf("path cannot be nil") } + if path.Context() != "" && path.Context() != ContextName { + return nil, internal.FormatDefaultErrorMessage(path.Context(), path.String(), "Resource", internal.ResourceContextRef) + } + switch path.Name() { case "cache": if path.Keys() == nil { diff --git a/pkg/ottl/contexts/ottlresource/resource_test.go b/pkg/ottl/contexts/ottlresource/resource_test.go index 9d490a362c3d..f37bcdf90c80 100644 --- a/pkg/ottl/contexts/ottlresource/resource_test.go +++ b/pkg/ottl/contexts/ottlresource/resource_test.go @@ -5,6 +5,7 @@ package ottlresource import ( "context" + "slices" "testing" "github.com/stretchr/testify/assert" @@ -361,6 +362,16 @@ func Test_newPathGetSetter(t *testing.T) { }, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} diff --git a/pkg/ottl/contexts/ottlscope/scope.go b/pkg/ottl/contexts/ottlscope/scope.go index 3ae5e0976446..c0729c558726 100644 --- a/pkg/ottl/contexts/ottlscope/scope.go +++ b/pkg/ottl/contexts/ottlscope/scope.go @@ -17,6 +17,10 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/logging" ) +const ( + ContextName = internal.ScopeContextName +) + var ( _ internal.ResourceContext = (*TransformContext)(nil) _ internal.InstrumentationScopeContext = (*TransformContext)(nil) @@ -85,6 +89,18 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet return p, nil } +// EnablePathContextNames enables the support to path's context names on statements. +// When this option is configured, all statement's paths must have a valid context prefix, +// otherwise an error is reported. +func EnablePathContextNames() Option { + return func(p *ottl.Parser[TransformContext]) { + ottl.WithPathContextNames[TransformContext]([]string{ + ContextName, + internal.ResourceContextName, + })(p) + } +} + type StatementSequenceOption func(*ottl.StatementSequence[TransformContext]) func WithStatementSequenceErrorMode(errorMode ottl.ErrorMode) StatementSequenceOption { @@ -129,19 +145,39 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if path == nil { return nil, fmt.Errorf("path cannot be nil") } + // Higher contexts parsing + if path.Context() != "" && path.Context() != ContextName { + return pep.parseHigherContextPath(path.Context(), path) + } + // Backward compatibility with paths without context + if path.Context() == "" && path.Name() == internal.ResourceContextName { + return pep.parseHigherContextPath(path.Name(), path.Next()) + } + switch path.Name() { case "cache": if path.Keys() == nil { return accessCache(), nil } return accessCacheKey(path.Keys()), nil - case "resource": - return internal.ResourcePathGetSetter[TransformContext](path.Next()) default: return internal.ScopePathGetSetter[TransformContext](path) } } +func (pep *pathExpressionParser) parseHigherContextPath(context string, path ottl.Path[TransformContext]) (ottl.GetSetter[TransformContext], error) { + switch context { + case internal.ResourceContextName: + return internal.ResourcePathGetSetter[TransformContext](path) + default: + var fullPath string + if path != nil { + fullPath = path.String() + } + return nil, internal.FormatDefaultErrorMessage(context, fullPath, "Instrumentation Scope", internal.InstrumentationScopeRef) + } +} + func accessCache() ottl.StandardGetSetter[TransformContext] { return ottl.StandardGetSetter[TransformContext]{ Getter: func(_ context.Context, tCtx TransformContext) (any, error) { diff --git a/pkg/ottl/contexts/ottlscope/scope_test.go b/pkg/ottl/contexts/ottlscope/scope_test.go index bd2e899f96fb..9049cd54bb92 100644 --- a/pkg/ottl/contexts/ottlscope/scope_test.go +++ b/pkg/ottl/contexts/ottlscope/scope_test.go @@ -5,9 +5,11 @@ package ottlscope import ( "context" + "slices" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/plog" @@ -35,11 +37,12 @@ func Test_newPathGetSetter(t *testing.T) { newMap["k2"] = newMap2 tests := []struct { - name string - path ottl.Path[TransformContext] - orig any - newVal any - modified func(is pcommon.InstrumentationScope, resource pcommon.Resource, cache pcommon.Map) + name string + path ottl.Path[TransformContext] + orig any + newVal any + modified func(is pcommon.InstrumentationScope, resource pcommon.Resource, cache pcommon.Map) + skipWithPathContextTest bool }{ { name: "cache", @@ -392,8 +395,34 @@ func Test_newPathGetSetter(t *testing.T) { modified: func(_ pcommon.InstrumentationScope, resource pcommon.Resource, _ pcommon.Map) { pcommon.NewResource().CopyTo(resource) }, + skipWithPathContextTest: true, + }, + { + name: "resource with context", + path: &internal.TestPath[TransformContext]{ + C: "resource", + }, + orig: refResource, + newVal: pcommon.NewResource(), + modified: func(_ pcommon.InstrumentationScope, resource pcommon.Resource, _ pcommon.Map) { + pcommon.NewResource().CopyTo(resource) + }, + skipWithPathContextTest: true, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + if tt.skipWithPathContextTest { + continue + } + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -421,6 +450,62 @@ func Test_newPathGetSetter(t *testing.T) { } } +func Test_newPathGetSetter_higherContextPath(t *testing.T) { + resource := pcommon.NewResource() + resource.Attributes().PutStr("foo", "bar") + ctx := NewTransformContext(pcommon.NewInstrumentationScope(), resource, plog.NewScopeLogs()) + + tests := []struct { + name string + path ottl.Path[TransformContext] + expected any + }{ + { + name: "resource", + path: &internal.TestPath[TransformContext]{N: "resource"}, + expected: resource, + }, + { + name: "resource field", + path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ + N: "attributes", + KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }, + }}, + expected: "bar", + }, + { + name: "resource context", + path: &internal.TestPath[TransformContext]{C: "resource"}, + expected: resource, + }, + { + name: "resource field with context", + path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }}, + expected: "bar", + }, + } + + pep := pathExpressionParser{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + accessor, err := pep.parsePath(tt.path) + require.NoError(t, err) + + got, err := accessor.Get(context.Background(), ctx) + assert.NoError(t, err) + assert.Equal(t, tt.expected, got) + }) + } +} + func createTelemetry() (pcommon.InstrumentationScope, pcommon.Resource) { is := pcommon.NewInstrumentationScope() is.SetName("library") diff --git a/pkg/ottl/contexts/ottlspan/span.go b/pkg/ottl/contexts/ottlspan/span.go index aa3283124bbd..02eb9ca7b913 100644 --- a/pkg/ottl/contexts/ottlspan/span.go +++ b/pkg/ottl/contexts/ottlspan/span.go @@ -18,6 +18,10 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/logging" ) +const ( + ContextName = internal.SpanContextName +) + var ( _ internal.ResourceContext = (*TransformContext)(nil) _ internal.InstrumentationScopeContext = (*TransformContext)(nil) @@ -95,6 +99,19 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet return p, nil } +// EnablePathContextNames enables the support to path's context names on statements. +// When this option is configured, all statement's paths must have a valid context prefix, +// otherwise an error is reported. +func EnablePathContextNames() Option { + return func(p *ottl.Parser[TransformContext]) { + ottl.WithPathContextNames[TransformContext]([]string{ + ContextName, + internal.ResourceContextName, + internal.InstrumentationScopeContextName, + })(p) + } +} + type StatementSequenceOption func(*ottl.StatementSequence[TransformContext]) func WithStatementSequenceErrorMode(errorMode ottl.ErrorMode) StatementSequenceOption { @@ -145,21 +162,41 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if path == nil { return nil, fmt.Errorf("path cannot be nil") } + // Higher contexts parsing + if path.Context() != "" && path.Context() != ContextName { + return pep.parseHigherContextPath(path.Context(), path) + } + // Backward compatibility with paths without context + if path.Context() == "" && (path.Name() == internal.ResourceContextName || path.Name() == internal.InstrumentationScopeContextName) { + return pep.parseHigherContextPath(path.Name(), path.Next()) + } + switch path.Name() { case "cache": if path.Keys() == nil { return accessCache(), nil } return accessCacheKey(path.Keys()), nil - case "resource": - return internal.ResourcePathGetSetter[TransformContext](path.Next()) - case "instrumentation_scope": - return internal.ScopePathGetSetter[TransformContext](path.Next()) default: return internal.SpanPathGetSetter[TransformContext](path) } } +func (pep *pathExpressionParser) parseHigherContextPath(context string, path ottl.Path[TransformContext]) (ottl.GetSetter[TransformContext], error) { + switch context { + case internal.ResourceContextName: + return internal.ResourcePathGetSetter[TransformContext](path) + case internal.InstrumentationScopeContextName: + return internal.ScopePathGetSetter[TransformContext](path) + default: + var fullPath string + if path != nil { + fullPath = path.String() + } + return nil, internal.FormatDefaultErrorMessage(context, fullPath, internal.SpanContextName, internal.SpanRef) + } +} + func accessCache() ottl.StandardGetSetter[TransformContext] { return ottl.StandardGetSetter[TransformContext]{ Getter: func(_ context.Context, tCtx TransformContext) (any, error) { diff --git a/pkg/ottl/contexts/ottlspan/span_test.go b/pkg/ottl/contexts/ottlspan/span_test.go index 967feac6e27a..c5348784b169 100644 --- a/pkg/ottl/contexts/ottlspan/span_test.go +++ b/pkg/ottl/contexts/ottlspan/span_test.go @@ -6,10 +6,12 @@ package ottlspan import ( "context" "encoding/hex" + "slices" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" @@ -53,11 +55,12 @@ func Test_newPathGetSetter(t *testing.T) { newMap["k2"] = newMap2 tests := []struct { - name string - path ottl.Path[TransformContext] - orig any - newVal any - modified func(span ptrace.Span, il pcommon.InstrumentationScope, resource pcommon.Resource, cache pcommon.Map) + name string + path ottl.Path[TransformContext] + orig any + newVal any + modified func(span ptrace.Span, il pcommon.InstrumentationScope, resource pcommon.Resource, cache pcommon.Map) + skipWithPathContextTest bool }{ { name: "cache", @@ -659,6 +662,19 @@ func Test_newPathGetSetter(t *testing.T) { modified: func(_ ptrace.Span, il pcommon.InstrumentationScope, _ pcommon.Resource, _ pcommon.Map) { pcommon.NewInstrumentationScope().CopyTo(il) }, + skipWithPathContextTest: true, + }, + { + name: "instrumentation_scope context", + path: &internal.TestPath[TransformContext]{ + C: "instrumentation_scope", + }, + orig: refIS, + newVal: pcommon.NewInstrumentationScope(), + modified: func(_ ptrace.Span, il pcommon.InstrumentationScope, _ pcommon.Resource, _ pcommon.Map) { + pcommon.NewInstrumentationScope().CopyTo(il) + }, + skipWithPathContextTest: true, }, { name: "resource", @@ -670,8 +686,34 @@ func Test_newPathGetSetter(t *testing.T) { modified: func(_ ptrace.Span, _ pcommon.InstrumentationScope, resource pcommon.Resource, _ pcommon.Map) { pcommon.NewResource().CopyTo(resource) }, + skipWithPathContextTest: true, + }, + { + name: "resource context", + path: &internal.TestPath[TransformContext]{ + C: "resource", + }, + orig: refResource, + newVal: pcommon.NewResource(), + modified: func(_ ptrace.Span, _ pcommon.InstrumentationScope, resource pcommon.Resource, _ pcommon.Map) { + pcommon.NewResource().CopyTo(resource) + }, + skipWithPathContextTest: true, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + if tt.skipWithPathContextTest { + continue + } + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -701,6 +743,86 @@ func Test_newPathGetSetter(t *testing.T) { } } +func Test_newPathGetSetter_higherContextPath(t *testing.T) { + resource := pcommon.NewResource() + resource.Attributes().PutStr("foo", "bar") + + instrumentationScope := pcommon.NewInstrumentationScope() + instrumentationScope.SetName("instrumentation_scope") + + ctx := NewTransformContext(ptrace.NewSpan(), instrumentationScope, resource, ptrace.NewScopeSpans(), ptrace.NewResourceSpans()) + + tests := []struct { + name string + path ottl.Path[TransformContext] + expected any + }{ + { + name: "resource", + path: &internal.TestPath[TransformContext]{N: "resource"}, + expected: resource, + }, + { + name: "resource field", + path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ + N: "attributes", + KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }, + }}, + expected: "bar", + }, + { + name: "resource context", + path: &internal.TestPath[TransformContext]{C: "resource"}, + expected: resource, + }, + { + name: "resource field with context", + path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }}, + expected: "bar", + }, + { + name: "instrumentation_scope", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, + expected: instrumentationScope.Name(), + }, + { + name: "instrumentation_scope context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field with context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, + expected: instrumentationScope.Name(), + }, + } + + pep := pathExpressionParser{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + accessor, err := pep.parsePath(tt.path) + require.NoError(t, err) + + got, err := accessor.Get(context.Background(), ctx) + assert.NoError(t, err) + assert.Equal(t, tt.expected, got) + }) + } +} + func createTelemetry() (ptrace.Span, pcommon.InstrumentationScope, pcommon.Resource) { span := ptrace.NewSpan() span.SetTraceID(traceID) diff --git a/pkg/ottl/contexts/ottlspanevent/span_events.go b/pkg/ottl/contexts/ottlspanevent/span_events.go index b3826f690d2d..a5d31a6e5bc5 100644 --- a/pkg/ottl/contexts/ottlspanevent/span_events.go +++ b/pkg/ottl/contexts/ottlspanevent/span_events.go @@ -19,6 +19,11 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/logging" ) +const ( + ContextName = "spanevent" + contextNameDescription = "Span Event" +) + var ( _ internal.ResourceContext = (*TransformContext)(nil) _ internal.InstrumentationScopeContext = (*TransformContext)(nil) @@ -103,6 +108,20 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet return p, nil } +// EnablePathContextNames enables the support to path's context names on statements. +// When this option is configured, all statement's paths must have a valid context prefix, +// otherwise an error is reported. +func EnablePathContextNames() Option { + return func(p *ottl.Parser[TransformContext]) { + ottl.WithPathContextNames[TransformContext]([]string{ + ContextName, + internal.SpanContextName, + internal.ResourceContextName, + internal.InstrumentationScopeContextName, + })(p) + } +} + type StatementSequenceOption func(*ottl.StatementSequence[TransformContext]) func WithStatementSequenceErrorMode(errorMode ottl.ErrorMode) StatementSequenceOption { @@ -153,18 +172,24 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot if path == nil { return nil, fmt.Errorf("path cannot be nil") } + // Higher contexts parsing + if path.Context() != "" && path.Context() != ContextName { + return pep.parseHigherContextPath(path.Context(), path) + } + // Backward compatibility with paths without context + if path.Context() == "" && + (path.Name() == internal.ResourceContextName || + path.Name() == internal.InstrumentationScopeContextName || + path.Name() == internal.SpanContextName) { + return pep.parseHigherContextPath(path.Name(), path.Next()) + } + switch path.Name() { case "cache": if path.Keys() == nil { return accessCache(), nil } return accessCacheKey(path.Keys()), nil - case "resource": - return internal.ResourcePathGetSetter[TransformContext](path.Next()) - case "instrumentation_scope": - return internal.ScopePathGetSetter[TransformContext](path.Next()) - case "span": - return internal.SpanPathGetSetter[TransformContext](path.Next()) case "time_unix_nano": return accessSpanEventTimeUnixNano(), nil case "time": @@ -179,7 +204,24 @@ func (pep *pathExpressionParser) parsePath(path ottl.Path[TransformContext]) (ot case "dropped_attributes_count": return accessSpanEventDroppedAttributeCount(), nil default: - return nil, internal.FormatDefaultErrorMessage(path.Name(), path.String(), "Span Event", internal.SpanEventRef) + return nil, internal.FormatDefaultErrorMessage(path.Name(), path.String(), contextNameDescription, internal.SpanEventRef) + } +} + +func (pep *pathExpressionParser) parseHigherContextPath(context string, path ottl.Path[TransformContext]) (ottl.GetSetter[TransformContext], error) { + switch context { + case internal.ResourceContextName: + return internal.ResourcePathGetSetter(path) + case internal.InstrumentationScopeContextName: + return internal.ScopePathGetSetter(path) + case internal.SpanContextName: + return internal.SpanPathGetSetter(path) + default: + var fullPath string + if path != nil { + fullPath = path.String() + } + return nil, internal.FormatDefaultErrorMessage(context, fullPath, contextNameDescription, internal.SpanEventRef) } } diff --git a/pkg/ottl/contexts/ottlspanevent/span_events_test.go b/pkg/ottl/contexts/ottlspanevent/span_events_test.go index 90b0c9781704..abf0aaa3602c 100644 --- a/pkg/ottl/contexts/ottlspanevent/span_events_test.go +++ b/pkg/ottl/contexts/ottlspanevent/span_events_test.go @@ -5,10 +5,12 @@ package ottlspanevent import ( "context" + "slices" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" @@ -47,11 +49,12 @@ func Test_newPathGetSetter(t *testing.T) { newMap["k2"] = newMap2 tests := []struct { - name string - path ottl.Path[TransformContext] - orig any - newVal any - modified func(spanEvent ptrace.SpanEvent, span ptrace.Span, il pcommon.InstrumentationScope, resource pcommon.Resource, cache pcommon.Map) + name string + path ottl.Path[TransformContext] + orig any + newVal any + modified func(spanEvent ptrace.SpanEvent, span ptrace.Span, il pcommon.InstrumentationScope, resource pcommon.Resource, cache pcommon.Map) + skipWithPathContextTest bool }{ { name: "span event time", @@ -415,6 +418,19 @@ func Test_newPathGetSetter(t *testing.T) { modified: func(_ ptrace.SpanEvent, _ ptrace.Span, il pcommon.InstrumentationScope, _ pcommon.Resource, _ pcommon.Map) { pcommon.NewInstrumentationScope().CopyTo(il) }, + skipWithPathContextTest: true, + }, + { + name: "instrumentation_scope context", + path: &internal.TestPath[TransformContext]{ + C: "instrumentation_scope", + }, + orig: refIS, + newVal: pcommon.NewInstrumentationScope(), + modified: func(_ ptrace.SpanEvent, _ ptrace.Span, il pcommon.InstrumentationScope, _ pcommon.Resource, _ pcommon.Map) { + pcommon.NewInstrumentationScope().CopyTo(il) + }, + skipWithPathContextTest: true, }, { name: "resource", @@ -426,6 +442,19 @@ func Test_newPathGetSetter(t *testing.T) { modified: func(_ ptrace.SpanEvent, _ ptrace.Span, _ pcommon.InstrumentationScope, resource pcommon.Resource, _ pcommon.Map) { pcommon.NewResource().CopyTo(resource) }, + skipWithPathContextTest: true, + }, + { + name: "resource context", + path: &internal.TestPath[TransformContext]{ + C: "resource", + }, + orig: refResource, + newVal: pcommon.NewResource(), + modified: func(_ ptrace.SpanEvent, _ ptrace.Span, _ pcommon.InstrumentationScope, resource pcommon.Resource, _ pcommon.Map) { + pcommon.NewResource().CopyTo(resource) + }, + skipWithPathContextTest: true, }, { name: "span", @@ -437,8 +466,34 @@ func Test_newPathGetSetter(t *testing.T) { modified: func(_ ptrace.SpanEvent, span ptrace.Span, _ pcommon.InstrumentationScope, _ pcommon.Resource, _ pcommon.Map) { ptrace.NewSpan().CopyTo(span) }, + skipWithPathContextTest: true, + }, + { + name: "span context", + path: &internal.TestPath[TransformContext]{ + C: "span", + }, + orig: refSpan, + newVal: ptrace.NewSpan(), + modified: func(_ ptrace.SpanEvent, span ptrace.Span, _ pcommon.InstrumentationScope, _ pcommon.Resource, _ pcommon.Map) { + ptrace.NewSpan().CopyTo(span) + }, + skipWithPathContextTest: true, }, } + // Copy all tests cases and sets the path.Context value to the generated ones. + // It ensures all exiting field access also work when the path context is set. + for _, tt := range slices.Clone(tests) { + if tt.skipWithPathContextTest { + continue + } + testWithContext := tt + testWithContext.name = "with_path_context:" + tt.name + pathWithContext := *tt.path.(*internal.TestPath[TransformContext]) + pathWithContext.C = ContextName + testWithContext.path = ottl.Path[TransformContext](&pathWithContext) + tests = append(tests, testWithContext) + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { pep := pathExpressionParser{} @@ -468,6 +523,109 @@ func Test_newPathGetSetter(t *testing.T) { } } +func Test_newPathGetSetter_higherContextPath(t *testing.T) { + resource := pcommon.NewResource() + resource.Attributes().PutStr("foo", "bar") + + span := ptrace.NewSpan() + span.SetName("span") + + instrumentationScope := pcommon.NewInstrumentationScope() + instrumentationScope.SetName("instrumentation_scope") + + ctx := NewTransformContext(ptrace.NewSpanEvent(), span, instrumentationScope, resource, ptrace.NewScopeSpans(), ptrace.NewResourceSpans()) + + tests := []struct { + name string + path ottl.Path[TransformContext] + expected any + }{ + { + name: "resource", + path: &internal.TestPath[TransformContext]{N: "resource"}, + expected: resource, + }, + { + name: "resource field", + path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ + N: "attributes", + KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }, + }}, + expected: "bar", + }, + { + name: "resource context", + path: &internal.TestPath[TransformContext]{C: "resource"}, + expected: resource, + }, + { + name: "resource field with context", + path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ + &internal.TestKey[TransformContext]{ + S: ottltest.Strp("foo"), + }, + }}, + expected: "bar", + }, + { + name: "instrumentation_scope", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field", + path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, + expected: instrumentationScope.Name(), + }, + { + name: "instrumentation_scope context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, + expected: instrumentationScope, + }, + { + name: "instrumentation_scope field with context", + path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, + expected: instrumentationScope.Name(), + }, + { + name: "span", + path: &internal.TestPath[TransformContext]{N: "span"}, + expected: span, + }, + { + name: "span field", + path: &internal.TestPath[TransformContext]{N: "span", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, + expected: span.Name(), + }, + { + name: "span context", + path: &internal.TestPath[TransformContext]{C: "span"}, + expected: span, + }, + { + name: "span field with context", + path: &internal.TestPath[TransformContext]{C: "span", N: "name"}, + expected: span.Name(), + }, + } + + pep := pathExpressionParser{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + accessor, err := pep.parsePath(tt.path) + require.NoError(t, err) + + got, err := accessor.Get(context.Background(), ctx) + assert.NoError(t, err) + assert.Equal(t, tt.expected, got) + }) + } +} + func createTelemetry() (ptrace.SpanEvent, ptrace.Span, pcommon.InstrumentationScope, pcommon.Resource) { spanEvent := ptrace.NewSpanEvent() diff --git a/pkg/ottl/functions.go b/pkg/ottl/functions.go index 4ff92123c7e6..3b11b8fd7fd3 100644 --- a/pkg/ottl/functions.go +++ b/pkg/ottl/functions.go @@ -103,6 +103,9 @@ func (p *Parser[K]) parsePathContext(path *path) (string, []field, error) { } if hasPathContextNames { + if ok, contextName := p.isPathToContextRootData(path); ok { + return contextName, []field{{}}, nil + } originalText := buildOriginalText(path) return "", nil, fmt.Errorf(`missing context name for path "%s", possibly valid options are: %s`, originalText, p.buildPathContextNamesText(originalText)) } @@ -110,6 +113,20 @@ func (p *Parser[K]) parsePathContext(path *path) (string, []field, error) { return "", path.Fields, nil } +// When a path has no dots separators (e.g.: resource), the grammar extracts it into the +// path.Fields slice, letting the path.Context empty. This function verifies if a given +// path is accessing any configured (ottl.WithPathContextNames) object root, returning +// true and the resolved context name. +func (p *Parser[K]) isPathToContextRootData(pat *path) (bool, string) { + // If the context value is filled, or it has multiple fields, it means the path + // has at least one dot on it. + if pat.Context != "" || len(pat.Fields) != 1 || len(pat.Fields[0].Keys) > 0 { + return false, "" + } + _, ok := p.pathContextNames[pat.Fields[0].Name] + return ok, pat.Fields[0].Name +} + func (p *Parser[K]) buildPathContextNamesText(path string) string { var builder strings.Builder var suffix string diff --git a/pkg/ottl/functions_test.go b/pkg/ottl/functions_test.go index a7bd4aef87c7..358f61e94f0b 100644 --- a/pkg/ottl/functions_test.go +++ b/pkg/ottl/functions_test.go @@ -2504,6 +2504,21 @@ func Test_newPath_WithPathContextNames(t *testing.T) { } } +func Test_newPath_withPathNameEqualsToContextName(t *testing.T) { + ps, _ := NewParser[any]( + defaultFunctionsForTests(), + testParsePath[any], + componenttest.NewNopTelemetrySettings(), + WithEnumParser[any](testParseEnum), + WithPathContextNames[any]([]string{"resource"}), + ) + + rs, err := ps.newPath(&path{Context: "", Fields: []field{{Name: "resource"}}}) + assert.NoError(t, err) + assert.Equal(t, "resource", rs.Context()) + assert.Empty(t, rs.Name()) +} + func Test_baseKey_String(t *testing.T) { bp := baseKey[any]{ s: ottltest.Strp("test"), diff --git a/pkg/ottl/parser.go b/pkg/ottl/parser.go index fade87d2982d..e162c8b57888 100644 --- a/pkg/ottl/parser.go +++ b/pkg/ottl/parser.go @@ -224,7 +224,9 @@ func (p *Parser[K]) prependContextToStatementPaths(context string, statement str var missingContextOffsets []int for _, it := range paths { if _, ok := p.pathContextNames[it.Context]; !ok { - missingContextOffsets = append(missingContextOffsets, it.Pos.Offset) + if skip, _ := p.isPathToContextRootData(&it); !skip { + missingContextOffsets = append(missingContextOffsets, it.Pos.Offset) + } } } diff --git a/pkg/ottl/parser_test.go b/pkg/ottl/parser_test.go index 726f531bfb81..b416d8239cd1 100644 --- a/pkg/ottl/parser_test.go +++ b/pkg/ottl/parser_test.go @@ -2832,6 +2832,13 @@ func Test_prependContextToStatementPaths_Success(t *testing.T) { pathContextNames: []string{"log", "resource"}, expected: `set(log.attributes["test"], "pass") where IsMatch(resource.name, "operation[AC]")`, }, + { + name: "path to context root object", + statement: `set(attributes["test"], resource)`, + context: "log", + pathContextNames: []string{"log", "resource"}, + expected: `set(log.attributes["test"], resource)`, + }, } for _, tt := range tests { From adfb986411242b560752ae12ccefc9abeea8b954 Mon Sep 17 00:00:00 2001 From: edmocosta <11836452+edmocosta@users.noreply.github.com> Date: Fri, 13 Dec 2024 13:57:29 +0100 Subject: [PATCH 2/7] Add changelog file --- ...contexts-to-support-path-with-context.yaml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .chloggen/change-contexts-to-support-path-with-context.yaml diff --git a/.chloggen/change-contexts-to-support-path-with-context.yaml b/.chloggen/change-contexts-to-support-path-with-context.yaml new file mode 100644 index 000000000000..a3754699d306 --- /dev/null +++ b/.chloggen/change-contexts-to-support-path-with-context.yaml @@ -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: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: pkg/ottl + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Change OTTL contexts to properly handle `ottl.Path` with context" + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [29017] + +# (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: "The OTTL contexts have a new option `EnablePathContextNames` to enabled support to expressing statements context via path names" + +# 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: [api] From a6413b27fe29efeda150119b15f31736311a26e3 Mon Sep 17 00:00:00 2001 From: edmocosta <11836452+edmocosta@users.noreply.github.com> Date: Fri, 13 Dec 2024 18:43:29 +0100 Subject: [PATCH 3/7] Fix default context inferrer order --- pkg/ottl/context_inferrer.go | 9 ++++----- pkg/ottl/context_inferrer_test.go | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/ottl/context_inferrer.go b/pkg/ottl/context_inferrer.go index 9dcb7afb1edd..493bf7866548 100644 --- a/pkg/ottl/context_inferrer.go +++ b/pkg/ottl/context_inferrer.go @@ -7,8 +7,8 @@ import "math" var defaultContextInferPriority = []string{ "log", - "metric", "datapoint", + "metric", "spanevent", "span", "resource", @@ -82,11 +82,10 @@ func defaultPriorityContextInferrer() contextInferrer { // newPriorityContextInferrer creates a new priority-based context inferrer. // To infer the context, it compares all [ottl.Path.Context] values, prioritizing them based -// on the provide contextsPriority argument, the lower the context position is in the array, +// on the provided contextsPriority argument, the lower the context position is in the array, // the more priority it will have over other items. -// If unknown/non-prioritized contexts are found on the statements, they can be either ignored -// or considered when no other prioritized context is found. To skip unknown contexts, the -// ignoreUnknownContext argument must be set to false. +// If unknown/non-prioritized contexts are found on the statements, they get assigned the lowest +// possible priority, and are only selected if no other prioritized context is found. func newPriorityContextInferrer(contextsPriority []string) contextInferrer { contextPriority := make(map[string]int, len(contextsPriority)) for i, ctx := range contextsPriority { diff --git a/pkg/ottl/context_inferrer_test.go b/pkg/ottl/context_inferrer_test.go index af8937df86c1..224eeca1c160 100644 --- a/pkg/ottl/context_inferrer_test.go +++ b/pkg/ottl/context_inferrer_test.go @@ -89,8 +89,8 @@ func Test_NewPriorityContextInferrer_InvalidStatement(t *testing.T) { func Test_DefaultPriorityContextInferrer(t *testing.T) { expectedPriority := []string{ "log", - "metric", "datapoint", + "metric", "spanevent", "span", "resource", From 53242ec8b92253c730839d68bac7e7754e834adf Mon Sep 17 00:00:00 2001 From: edmocosta <11836452+edmocosta@users.noreply.github.com> Date: Wed, 18 Dec 2024 11:12:47 +0100 Subject: [PATCH 4/7] Remove top-level context access tests --- pkg/ottl/contexts/internal/scope_test.go | 13 +------ .../contexts/ottldatapoint/datapoint_test.go | 38 ++----------------- pkg/ottl/contexts/ottllog/log_test.go | 26 ++----------- pkg/ottl/contexts/ottlmetric/metrics_test.go | 26 ++----------- pkg/ottl/contexts/ottlscope/scope_test.go | 14 +------ pkg/ottl/contexts/ottlspan/span_test.go | 26 ++----------- .../ottlspanevent/span_events_test.go | 38 ++----------------- 7 files changed, 20 insertions(+), 161 deletions(-) diff --git a/pkg/ottl/contexts/internal/scope_test.go b/pkg/ottl/contexts/internal/scope_test.go index b0fd6dd8e40f..788924115bd5 100644 --- a/pkg/ottl/contexts/internal/scope_test.go +++ b/pkg/ottl/contexts/internal/scope_test.go @@ -335,18 +335,7 @@ func TestScopePathGetSetter(t *testing.T) { }, }, { - name: "scope", - path: &TestPath[*instrumentationScopeContext]{ - C: "scope", - }, - orig: refIS, - newVal: pcommon.NewInstrumentationScope(), - modified: func(is pcommon.InstrumentationScope) { - pcommon.NewInstrumentationScope().CopyTo(is) - }, - }, - { - name: "scope field", + name: "scope with context", path: &TestPath[*instrumentationScopeContext]{ C: "scope", N: "name", diff --git a/pkg/ottl/contexts/ottldatapoint/datapoint_test.go b/pkg/ottl/contexts/ottldatapoint/datapoint_test.go index 6f01925cc544..604f008865ec 100644 --- a/pkg/ottl/contexts/ottldatapoint/datapoint_test.go +++ b/pkg/ottl/contexts/ottldatapoint/datapoint_test.go @@ -2232,12 +2232,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected any }{ { - name: "resource", - path: &internal.TestPath[TransformContext]{N: "resource"}, - expected: resource, - }, - { - name: "resource field", + name: "resource", path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ N: "attributes", KeySlice: []ottl.Key[TransformContext]{ @@ -2249,12 +2244,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected: "bar", }, { - name: "resource context", - path: &internal.TestPath[TransformContext]{C: "resource"}, - expected: resource, - }, - { - name: "resource field with context", + name: "resource with context", path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ &internal.TestKey[TransformContext]{ S: ottltest.Strp("foo"), @@ -2264,41 +2254,21 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { }, { name: "metric", - path: &internal.TestPath[TransformContext]{N: "metric"}, - expected: metric, - }, - { - name: "metric field", path: &internal.TestPath[TransformContext]{N: "metric", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, expected: metric.Name(), }, { - name: "metric context", - path: &internal.TestPath[TransformContext]{C: "metric"}, - expected: metric, - }, - { - name: "metric field with context", + name: "metric with context", path: &internal.TestPath[TransformContext]{C: "metric", N: "name"}, expected: metric.Name(), }, { name: "instrumentation_scope", - path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field", path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, expected: instrumentationScope.Name(), }, { - name: "instrumentation_scope context", - path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field with context", + name: "instrumentation_scope with context", path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, expected: instrumentationScope.Name(), }, diff --git a/pkg/ottl/contexts/ottllog/log_test.go b/pkg/ottl/contexts/ottllog/log_test.go index 1e8c747ba297..0e440052f1bd 100644 --- a/pkg/ottl/contexts/ottllog/log_test.go +++ b/pkg/ottl/contexts/ottllog/log_test.go @@ -649,12 +649,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected any }{ { - name: "resource", - path: &internal.TestPath[TransformContext]{N: "resource"}, - expected: resource, - }, - { - name: "resource field", + name: "resource", path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ N: "attributes", KeySlice: []ottl.Key[TransformContext]{ @@ -666,12 +661,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected: "val", }, { - name: "resource context", - path: &internal.TestPath[TransformContext]{C: "resource"}, - expected: resource, - }, - { - name: "resource field with context", + name: "resource with context", path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ &internal.TestKey[TransformContext]{ S: ottltest.Strp("str"), @@ -681,21 +671,11 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { }, { name: "instrumentation_scope", - path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field", path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, expected: instrumentationScope.Name(), }, { - name: "instrumentation_scope context", - path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field with context", + name: "instrumentation_scope with context", path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, expected: instrumentationScope.Name(), }, diff --git a/pkg/ottl/contexts/ottlmetric/metrics_test.go b/pkg/ottl/contexts/ottlmetric/metrics_test.go index 0c60a9ca7b2e..c434ab5735e9 100644 --- a/pkg/ottl/contexts/ottlmetric/metrics_test.go +++ b/pkg/ottl/contexts/ottlmetric/metrics_test.go @@ -194,12 +194,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected any }{ { - name: "resource", - path: &internal.TestPath[TransformContext]{N: "resource"}, - expected: resource, - }, - { - name: "resource field", + name: "resource", path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ N: "attributes", KeySlice: []ottl.Key[TransformContext]{ @@ -211,12 +206,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected: "bar", }, { - name: "resource context", - path: &internal.TestPath[TransformContext]{C: "resource"}, - expected: resource, - }, - { - name: "resource field with context", + name: "resource with context", path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ &internal.TestKey[TransformContext]{ S: ottltest.Strp("foo"), @@ -226,21 +216,11 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { }, { name: "instrumentation_scope", - path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field", path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, expected: instrumentationScope.Name(), }, { - name: "instrumentation_scope context", - path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field with context", + name: "instrumentation_scope with context", path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, expected: instrumentationScope.Name(), }, diff --git a/pkg/ottl/contexts/ottlscope/scope_test.go b/pkg/ottl/contexts/ottlscope/scope_test.go index 2d10353ae022..4f4d8cf7c3b5 100644 --- a/pkg/ottl/contexts/ottlscope/scope_test.go +++ b/pkg/ottl/contexts/ottlscope/scope_test.go @@ -433,12 +433,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected any }{ { - name: "resource", - path: &internal.TestPath[TransformContext]{N: "resource"}, - expected: resource, - }, - { - name: "resource field", + name: "resource", path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ N: "attributes", KeySlice: []ottl.Key[TransformContext]{ @@ -450,12 +445,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected: "bar", }, { - name: "resource context", - path: &internal.TestPath[TransformContext]{C: "resource"}, - expected: resource, - }, - { - name: "resource field with context", + name: "resource with context", path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ &internal.TestKey[TransformContext]{ S: ottltest.Strp("foo"), diff --git a/pkg/ottl/contexts/ottlspan/span_test.go b/pkg/ottl/contexts/ottlspan/span_test.go index 3aee05a5c341..24609de32b06 100644 --- a/pkg/ottl/contexts/ottlspan/span_test.go +++ b/pkg/ottl/contexts/ottlspan/span_test.go @@ -706,12 +706,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected any }{ { - name: "resource", - path: &internal.TestPath[TransformContext]{N: "resource"}, - expected: resource, - }, - { - name: "resource field", + name: "resource", path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ N: "attributes", KeySlice: []ottl.Key[TransformContext]{ @@ -723,12 +718,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected: "bar", }, { - name: "resource context", - path: &internal.TestPath[TransformContext]{C: "resource"}, - expected: resource, - }, - { - name: "resource field with context", + name: "resource with context", path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ &internal.TestKey[TransformContext]{ S: ottltest.Strp("foo"), @@ -738,21 +728,11 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { }, { name: "instrumentation_scope", - path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field", path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, expected: instrumentationScope.Name(), }, { - name: "instrumentation_scope context", - path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field with context", + name: "instrumentation_scope with context", path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, expected: instrumentationScope.Name(), }, diff --git a/pkg/ottl/contexts/ottlspanevent/span_events_test.go b/pkg/ottl/contexts/ottlspanevent/span_events_test.go index fe0386c102db..29abc6abce1d 100644 --- a/pkg/ottl/contexts/ottlspanevent/span_events_test.go +++ b/pkg/ottl/contexts/ottlspanevent/span_events_test.go @@ -465,12 +465,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected any }{ { - name: "resource", - path: &internal.TestPath[TransformContext]{N: "resource"}, - expected: resource, - }, - { - name: "resource field", + name: "resource", path: &internal.TestPath[TransformContext]{C: "", N: "resource", NextPath: &internal.TestPath[TransformContext]{ N: "attributes", KeySlice: []ottl.Key[TransformContext]{ @@ -482,12 +477,7 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { expected: "bar", }, { - name: "resource context", - path: &internal.TestPath[TransformContext]{C: "resource"}, - expected: resource, - }, - { - name: "resource field with context", + name: "resource with context", path: &internal.TestPath[TransformContext]{C: "resource", N: "attributes", KeySlice: []ottl.Key[TransformContext]{ &internal.TestKey[TransformContext]{ S: ottltest.Strp("foo"), @@ -497,41 +487,21 @@ func Test_newPathGetSetter_higherContextPath(t *testing.T) { }, { name: "instrumentation_scope", - path: &internal.TestPath[TransformContext]{N: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field", path: &internal.TestPath[TransformContext]{N: "instrumentation_scope", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, expected: instrumentationScope.Name(), }, { - name: "instrumentation_scope context", - path: &internal.TestPath[TransformContext]{C: "instrumentation_scope"}, - expected: instrumentationScope, - }, - { - name: "instrumentation_scope field with context", + name: "instrumentation_scope with context", path: &internal.TestPath[TransformContext]{C: "instrumentation_scope", N: "name"}, expected: instrumentationScope.Name(), }, { name: "span", - path: &internal.TestPath[TransformContext]{N: "span"}, - expected: span, - }, - { - name: "span field", path: &internal.TestPath[TransformContext]{N: "span", NextPath: &internal.TestPath[TransformContext]{N: "name"}}, expected: span.Name(), }, { - name: "span context", - path: &internal.TestPath[TransformContext]{C: "span"}, - expected: span, - }, - { - name: "span field with context", + name: "span with context", path: &internal.TestPath[TransformContext]{C: "span", N: "name"}, expected: span.Name(), }, From e056e1ca3e531a76ff0a057f35e6baad15da1546 Mon Sep 17 00:00:00 2001 From: edmocosta <11836452+edmocosta@users.noreply.github.com> Date: Wed, 18 Dec 2024 11:16:56 +0100 Subject: [PATCH 5/7] Revert parser and context inferrer changes --- pkg/ottl/context_inferrer.go | 34 +++-------- pkg/ottl/context_inferrer_test.go | 20 +------ pkg/ottl/contexts/internal/path.go | 13 ----- pkg/ottl/contexts/internal/path_test.go | 76 ------------------------- pkg/ottl/functions.go | 17 ------ pkg/ottl/functions_test.go | 15 ----- pkg/ottl/parser.go | 4 +- pkg/ottl/parser_test.go | 7 --- 8 files changed, 9 insertions(+), 177 deletions(-) delete mode 100644 pkg/ottl/contexts/internal/path_test.go diff --git a/pkg/ottl/context_inferrer.go b/pkg/ottl/context_inferrer.go index 493bf7866548..da4ade783278 100644 --- a/pkg/ottl/context_inferrer.go +++ b/pkg/ottl/context_inferrer.go @@ -7,8 +7,8 @@ import "math" var defaultContextInferPriority = []string{ "log", - "datapoint", "metric", + "datapoint", "spanevent", "span", "resource", @@ -37,15 +37,14 @@ func (s *priorityContextInferrer) infer(statements []string) (string, error) { } for _, p := range getParsedStatementPaths(parsed) { - pathContext := s.getContextCandidate(p) - pathContextPriority, ok := s.contextPriority[pathContext] + pathContextPriority, ok := s.contextPriority[p.Context] if !ok { // Lowest priority pathContextPriority = math.MaxInt } if inferredContext == "" || pathContextPriority < inferredContextPriority { - inferredContext = pathContext + inferredContext = p.Context inferredContextPriority = pathContextPriority } } @@ -54,26 +53,6 @@ func (s *priorityContextInferrer) infer(statements []string) (string, error) { return inferredContext, nil } -// When a path has no dots separators (e.g.: resource), the grammar extracts it into the -// path.Fields slice, letting the path.Context empty. This function returns either the -// path.Context string or, if it's eligible and meets certain conditions, the first -// path.Fields name. -func (s *priorityContextInferrer) getContextCandidate(p path) string { - if p.Context != "" { - return p.Context - } - // If it has multiple fields or keys, it means the path has at least one dot on it, - // and isn't a context access. - if len(p.Fields) != 1 || len(p.Fields[0].Keys) > 0 { - return "" - } - _, ok := s.contextPriority[p.Fields[0].Name] - if ok { - return p.Fields[0].Name - } - return "" -} - // defaultPriorityContextInferrer is like newPriorityContextInferrer, but using the default // context priorities and ignoring unknown/non-prioritized contexts. func defaultPriorityContextInferrer() contextInferrer { @@ -82,10 +61,11 @@ func defaultPriorityContextInferrer() contextInferrer { // newPriorityContextInferrer creates a new priority-based context inferrer. // To infer the context, it compares all [ottl.Path.Context] values, prioritizing them based -// on the provided contextsPriority argument, the lower the context position is in the array, +// on the provide contextsPriority argument, the lower the context position is in the array, // the more priority it will have over other items. -// If unknown/non-prioritized contexts are found on the statements, they get assigned the lowest -// possible priority, and are only selected if no other prioritized context is found. +// If unknown/non-prioritized contexts are found on the statements, they can be either ignored +// or considered when no other prioritized context is found. To skip unknown contexts, the +// ignoreUnknownContext argument must be set to false. func newPriorityContextInferrer(contextsPriority []string) contextInferrer { contextPriority := make(map[string]int, len(contextsPriority)) for i, ctx := range contextsPriority { diff --git a/pkg/ottl/context_inferrer_test.go b/pkg/ottl/context_inferrer_test.go index 224eeca1c160..4d4455dd0dcf 100644 --- a/pkg/ottl/context_inferrer_test.go +++ b/pkg/ottl/context_inferrer_test.go @@ -49,24 +49,6 @@ func Test_NewPriorityContextInferrer_Infer(t *testing.T) { statements: []string{"set(span.foo, true) where span.bar == true"}, expected: "span", }, - { - name: "with context root access", - priority: []string{"resource", "foo"}, - statements: []string{"set(foo.attributes[\"body\"], resource)"}, - expected: "resource", - }, - { - name: "with non-eligible context root access", - priority: []string{"resource", "foo"}, - statements: []string{"set(foo.attributes[\"body\"], resource[\"foo\"])"}, - expected: "foo", - }, - { - name: "with non-prioritized context root access", - priority: []string{"foo"}, - statements: []string{"set(resource, bar.attributes[\"body\"])"}, - expected: "bar", - }, } for _, tt := range tests { @@ -89,8 +71,8 @@ func Test_NewPriorityContextInferrer_InvalidStatement(t *testing.T) { func Test_DefaultPriorityContextInferrer(t *testing.T) { expectedPriority := []string{ "log", - "datapoint", "metric", + "datapoint", "spanevent", "span", "resource", diff --git a/pkg/ottl/contexts/internal/path.go b/pkg/ottl/contexts/internal/path.go index 99b4ebcc9c35..954d14329646 100644 --- a/pkg/ottl/contexts/internal/path.go +++ b/pkg/ottl/contexts/internal/path.go @@ -55,16 +55,3 @@ func (k *TestKey[K]) String(_ context.Context, _ K) (*string, error) { func (k *TestKey[K]) Int(_ context.Context, _ K) (*int64, error) { return k.I, nil } - -// isPathToContextRoot return true if the given path is accessing the context's root object -// instead of specific fields. -func isPathToContextRoot[T any](path ottl.Path[T], ctx string) bool { - if path == nil { - return true - } - // path with matching context and empty name/keys/next - return path.Context() == ctx && - path.Name() == "" && - len(path.Keys()) == 0 && - path.Next() == nil -} diff --git a/pkg/ottl/contexts/internal/path_test.go b/pkg/ottl/contexts/internal/path_test.go deleted file mode 100644 index f169fbce6098..000000000000 --- a/pkg/ottl/contexts/internal/path_test.go +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -package internal - -import ( - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" -) - -func Test_isPathToContextRoot(t *testing.T) { - tests := []struct { - name string - context string - path ottl.Path[any] - expected bool - }{ - { - name: "with context and empty path", - context: "resource", - expected: true, - path: &TestPath[any]{ - C: "resource", - N: "", - }, - }, - { - name: "with context and next path", - context: "resource", - expected: false, - path: &TestPath[any]{ - C: "resource", - N: "", - NextPath: &TestPath[any]{ - N: "foo", - }, - }, - }, - { - name: "with context and path keys", - context: "resource", - expected: false, - path: &TestPath[any]{ - C: "resource", - N: "", - KeySlice: []ottl.Key[any]{ - &TestKey[any]{}, - }, - }, - }, - { - name: "with both context and path name", - context: "resource", - expected: false, - path: &TestPath[any]{ - C: "resource", - N: "resource", - }, - }, - { - name: "with nil path", - context: "resource", - expected: true, - path: nil, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, isPathToContextRoot(tt.path, tt.context)) - }) - } -} diff --git a/pkg/ottl/functions.go b/pkg/ottl/functions.go index 3b11b8fd7fd3..4ff92123c7e6 100644 --- a/pkg/ottl/functions.go +++ b/pkg/ottl/functions.go @@ -103,9 +103,6 @@ func (p *Parser[K]) parsePathContext(path *path) (string, []field, error) { } if hasPathContextNames { - if ok, contextName := p.isPathToContextRootData(path); ok { - return contextName, []field{{}}, nil - } originalText := buildOriginalText(path) return "", nil, fmt.Errorf(`missing context name for path "%s", possibly valid options are: %s`, originalText, p.buildPathContextNamesText(originalText)) } @@ -113,20 +110,6 @@ func (p *Parser[K]) parsePathContext(path *path) (string, []field, error) { return "", path.Fields, nil } -// When a path has no dots separators (e.g.: resource), the grammar extracts it into the -// path.Fields slice, letting the path.Context empty. This function verifies if a given -// path is accessing any configured (ottl.WithPathContextNames) object root, returning -// true and the resolved context name. -func (p *Parser[K]) isPathToContextRootData(pat *path) (bool, string) { - // If the context value is filled, or it has multiple fields, it means the path - // has at least one dot on it. - if pat.Context != "" || len(pat.Fields) != 1 || len(pat.Fields[0].Keys) > 0 { - return false, "" - } - _, ok := p.pathContextNames[pat.Fields[0].Name] - return ok, pat.Fields[0].Name -} - func (p *Parser[K]) buildPathContextNamesText(path string) string { var builder strings.Builder var suffix string diff --git a/pkg/ottl/functions_test.go b/pkg/ottl/functions_test.go index 358f61e94f0b..a7bd4aef87c7 100644 --- a/pkg/ottl/functions_test.go +++ b/pkg/ottl/functions_test.go @@ -2504,21 +2504,6 @@ func Test_newPath_WithPathContextNames(t *testing.T) { } } -func Test_newPath_withPathNameEqualsToContextName(t *testing.T) { - ps, _ := NewParser[any]( - defaultFunctionsForTests(), - testParsePath[any], - componenttest.NewNopTelemetrySettings(), - WithEnumParser[any](testParseEnum), - WithPathContextNames[any]([]string{"resource"}), - ) - - rs, err := ps.newPath(&path{Context: "", Fields: []field{{Name: "resource"}}}) - assert.NoError(t, err) - assert.Equal(t, "resource", rs.Context()) - assert.Empty(t, rs.Name()) -} - func Test_baseKey_String(t *testing.T) { bp := baseKey[any]{ s: ottltest.Strp("test"), diff --git a/pkg/ottl/parser.go b/pkg/ottl/parser.go index e162c8b57888..fade87d2982d 100644 --- a/pkg/ottl/parser.go +++ b/pkg/ottl/parser.go @@ -224,9 +224,7 @@ func (p *Parser[K]) prependContextToStatementPaths(context string, statement str var missingContextOffsets []int for _, it := range paths { if _, ok := p.pathContextNames[it.Context]; !ok { - if skip, _ := p.isPathToContextRootData(&it); !skip { - missingContextOffsets = append(missingContextOffsets, it.Pos.Offset) - } + missingContextOffsets = append(missingContextOffsets, it.Pos.Offset) } } diff --git a/pkg/ottl/parser_test.go b/pkg/ottl/parser_test.go index b416d8239cd1..726f531bfb81 100644 --- a/pkg/ottl/parser_test.go +++ b/pkg/ottl/parser_test.go @@ -2832,13 +2832,6 @@ func Test_prependContextToStatementPaths_Success(t *testing.T) { pathContextNames: []string{"log", "resource"}, expected: `set(log.attributes["test"], "pass") where IsMatch(resource.name, "operation[AC]")`, }, - { - name: "path to context root object", - statement: `set(attributes["test"], resource)`, - context: "log", - pathContextNames: []string{"log", "resource"}, - expected: `set(log.attributes["test"], resource)`, - }, } for _, tt := range tests { From 41d9bfcccdbaa906c018712e9574b8a296d985e5 Mon Sep 17 00:00:00 2001 From: edmocosta <11836452+edmocosta@users.noreply.github.com> Date: Thu, 19 Dec 2024 19:09:46 +0100 Subject: [PATCH 6/7] Add an experimental warning to all new exported objects --- pkg/ottl/contexts/ottldatapoint/datapoint.go | 3 +++ pkg/ottl/contexts/ottllog/log.go | 3 +++ pkg/ottl/contexts/ottlmetric/metrics.go | 3 +++ pkg/ottl/contexts/ottlresource/resource.go | 3 +++ pkg/ottl/contexts/ottlscope/scope.go | 3 +++ pkg/ottl/contexts/ottlspan/span.go | 3 +++ pkg/ottl/contexts/ottlspanevent/span_events.go | 3 +++ 7 files changed, 21 insertions(+) diff --git a/pkg/ottl/contexts/ottldatapoint/datapoint.go b/pkg/ottl/contexts/ottldatapoint/datapoint.go index e0fa9bfeda86..d231c4210813 100644 --- a/pkg/ottl/contexts/ottldatapoint/datapoint.go +++ b/pkg/ottl/contexts/ottldatapoint/datapoint.go @@ -20,6 +20,7 @@ import ( ) const ( + // Experimental: *NOTE* this constant is subject to change or removal in the future. ContextName = "datapoint" contextNameDescription = "DataPoint" ) @@ -128,6 +129,8 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet // EnablePathContextNames enables the support to path's context names on statements. // When this option is configured, all statement's paths must have a valid context prefix, // otherwise an error is reported. +// +// Experimental: *NOTE* this option is subject to change or removal in the future. func EnablePathContextNames() Option { return func(p *ottl.Parser[TransformContext]) { ottl.WithPathContextNames[TransformContext]([]string{ diff --git a/pkg/ottl/contexts/ottllog/log.go b/pkg/ottl/contexts/ottllog/log.go index 4382f57520a3..4983e60ddc6c 100644 --- a/pkg/ottl/contexts/ottllog/log.go +++ b/pkg/ottl/contexts/ottllog/log.go @@ -22,6 +22,7 @@ import ( ) const ( + // Experimental: *NOTE* this constant is subject to change or removal in the future. ContextName = "log" contextNameDescription = "Log" ) @@ -125,6 +126,8 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet // EnablePathContextNames enables the support to path's context names on statements. // When this option is configured, all statement's paths must have a valid context prefix, // otherwise an error is reported. +// +// Experimental: *NOTE* this option is subject to change or removal in the future. func EnablePathContextNames() Option { return func(p *ottl.Parser[TransformContext]) { ottl.WithPathContextNames[TransformContext]([]string{ diff --git a/pkg/ottl/contexts/ottlmetric/metrics.go b/pkg/ottl/contexts/ottlmetric/metrics.go index 4267b3976a1d..b8c26bac2e9d 100644 --- a/pkg/ottl/contexts/ottlmetric/metrics.go +++ b/pkg/ottl/contexts/ottlmetric/metrics.go @@ -16,6 +16,7 @@ import ( ) const ( + // Experimental: *NOTE* this constant is subject to change or removal in the future. ContextName = internal.MetricContextName ) @@ -97,6 +98,8 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet // EnablePathContextNames enables the support to path's context names on statements. // When this option is configured, all statement's paths must have a valid context prefix, // otherwise an error is reported. +// +// Experimental: *NOTE* this option is subject to change or removal in the future. func EnablePathContextNames() Option { return func(p *ottl.Parser[TransformContext]) { ottl.WithPathContextNames[TransformContext]([]string{ diff --git a/pkg/ottl/contexts/ottlresource/resource.go b/pkg/ottl/contexts/ottlresource/resource.go index db120b774e26..fef87f37b54e 100644 --- a/pkg/ottl/contexts/ottlresource/resource.go +++ b/pkg/ottl/contexts/ottlresource/resource.go @@ -18,6 +18,7 @@ import ( ) const ( + // Experimental: *NOTE* this constant is subject to change or removal in the future. ContextName = internal.ResourceContextName ) @@ -80,6 +81,8 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet // EnablePathContextNames enables the support to path's context names on statements. // When this option is configured, all statement's paths must have a valid context prefix, // otherwise an error is reported. +// +// Experimental: *NOTE* this option is subject to change or removal in the future. func EnablePathContextNames() Option { return func(p *ottl.Parser[TransformContext]) { ottl.WithPathContextNames[TransformContext]([]string{ContextName})(p) diff --git a/pkg/ottl/contexts/ottlscope/scope.go b/pkg/ottl/contexts/ottlscope/scope.go index c0729c558726..13f2c7a83e83 100644 --- a/pkg/ottl/contexts/ottlscope/scope.go +++ b/pkg/ottl/contexts/ottlscope/scope.go @@ -18,6 +18,7 @@ import ( ) const ( + // Experimental: *NOTE* this constant is subject to change or removal in the future. ContextName = internal.ScopeContextName ) @@ -92,6 +93,8 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet // EnablePathContextNames enables the support to path's context names on statements. // When this option is configured, all statement's paths must have a valid context prefix, // otherwise an error is reported. +// +// Experimental: *NOTE* this option is subject to change or removal in the future. func EnablePathContextNames() Option { return func(p *ottl.Parser[TransformContext]) { ottl.WithPathContextNames[TransformContext]([]string{ diff --git a/pkg/ottl/contexts/ottlspan/span.go b/pkg/ottl/contexts/ottlspan/span.go index 02eb9ca7b913..be11495e8dde 100644 --- a/pkg/ottl/contexts/ottlspan/span.go +++ b/pkg/ottl/contexts/ottlspan/span.go @@ -19,6 +19,7 @@ import ( ) const ( + // Experimental: *NOTE* this constant is subject to change or removal in the future. ContextName = internal.SpanContextName ) @@ -102,6 +103,8 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet // EnablePathContextNames enables the support to path's context names on statements. // When this option is configured, all statement's paths must have a valid context prefix, // otherwise an error is reported. +// +// Experimental: *NOTE* this option is subject to change or removal in the future. func EnablePathContextNames() Option { return func(p *ottl.Parser[TransformContext]) { ottl.WithPathContextNames[TransformContext]([]string{ diff --git a/pkg/ottl/contexts/ottlspanevent/span_events.go b/pkg/ottl/contexts/ottlspanevent/span_events.go index a5d31a6e5bc5..6d654b058004 100644 --- a/pkg/ottl/contexts/ottlspanevent/span_events.go +++ b/pkg/ottl/contexts/ottlspanevent/span_events.go @@ -20,6 +20,7 @@ import ( ) const ( + // Experimental: *NOTE* this constant is subject to change or removal in the future. ContextName = "spanevent" contextNameDescription = "Span Event" ) @@ -111,6 +112,8 @@ func NewParser(functions map[string]ottl.Factory[TransformContext], telemetrySet // EnablePathContextNames enables the support to path's context names on statements. // When this option is configured, all statement's paths must have a valid context prefix, // otherwise an error is reported. +// +// Experimental: *NOTE* this option is subject to change or removal in the future. func EnablePathContextNames() Option { return func(p *ottl.Parser[TransformContext]) { ottl.WithPathContextNames[TransformContext]([]string{ From 9f0a0e8be65f8900925655ef9ff559fd6a56924b Mon Sep 17 00:00:00 2001 From: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Date: Fri, 20 Dec 2024 15:38:17 -0500 Subject: [PATCH 7/7] Update .chloggen/change-contexts-to-support-path-with-context.yaml --- .chloggen/change-contexts-to-support-path-with-context.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/change-contexts-to-support-path-with-context.yaml b/.chloggen/change-contexts-to-support-path-with-context.yaml index a3754699d306..451da0538236 100644 --- a/.chloggen/change-contexts-to-support-path-with-context.yaml +++ b/.chloggen/change-contexts-to-support-path-with-context.yaml @@ -15,7 +15,7 @@ issues: [29017] # (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: "The OTTL contexts have a new option `EnablePathContextNames` to enabled support to expressing statements context via path names" +subtext: "The OTTL contexts have a new option `EnablePathContextNames` to enable support for expressing a statement's context via path names in the statement" # 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.