Skip to content

Commit

Permalink
Revert parser and context inferrer changes
Browse files Browse the repository at this point in the history
  • Loading branch information
edmocosta committed Dec 18, 2024
1 parent 53242ec commit e056e1c
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 177 deletions.
34 changes: 7 additions & 27 deletions pkg/ottl/context_inferrer.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import "math"

var defaultContextInferPriority = []string{
"log",
"datapoint",
"metric",
"datapoint",
"spanevent",
"span",
"resource",
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
20 changes: 1 addition & 19 deletions pkg/ottl/context_inferrer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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",
Expand Down
13 changes: 0 additions & 13 deletions pkg/ottl/contexts/internal/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
76 changes: 0 additions & 76 deletions pkg/ottl/contexts/internal/path_test.go

This file was deleted.

17 changes: 0 additions & 17 deletions pkg/ottl/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,30 +103,13 @@ 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))
}

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
Expand Down
15 changes: 0 additions & 15 deletions pkg/ottl/functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
4 changes: 1 addition & 3 deletions pkg/ottl/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
7 changes: 0 additions & 7 deletions pkg/ottl/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit e056e1c

Please sign in to comment.