Skip to content

Commit 8247a3e

Browse files
edmocostaFiery-Fenix
edmocosta
authored and
Fiery-Fenix
committed
[pkg/ottl] Fix the context inference order to prioritize scope over resource (open-telemetry#39307)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Changed the context inferrer so the `scope` context has priority over the `resource` context, fixing the error described on the linked issue. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#39155 <!--Please delete paragraphs that you did not use before submitting.-->
1 parent 7517cae commit 8247a3e

File tree

3 files changed

+216
-3
lines changed

3 files changed

+216
-3
lines changed

‎.chloggen/fix-context-inferrer-priority.yaml

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: pkg/ottl
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix OTTL context inference order to prioritize the `scope` context over `resource`.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [39155]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

‎pkg/ottl/context_inferrer.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ var defaultContextInferPriority = []string{
2121
"metric",
2222
"spanevent",
2323
"span",
24-
"resource",
2524
"scope",
2625
"instrumentation_scope",
26+
"resource",
2727
}
2828

2929
// contextInferrer is an interface used to infer the OTTL context from statements.

‎pkg/ottl/context_inferrer_test.go

+188-2
Original file line numberDiff line numberDiff line change
@@ -282,16 +282,16 @@ func Test_NewPriorityContextInferrer_InvalidStatement(t *testing.T) {
282282
require.ErrorContains(t, err, "unexpected token")
283283
}
284284

285-
func Test_DefaultPriorityContextInferrer(t *testing.T) {
285+
func Test_NewPriorityContextInferrer_DefaultPriorityList(t *testing.T) {
286286
expectedPriority := []string{
287287
"log",
288288
"datapoint",
289289
"metric",
290290
"spanevent",
291291
"span",
292-
"resource",
293292
"scope",
294293
"instrumentation_scope",
294+
"resource",
295295
}
296296

297297
inferrer := newPriorityContextInferrer(componenttest.NewNopTelemetrySettings(), map[string]*priorityContextInferrerCandidate{}).(*priorityContextInferrer)
@@ -301,3 +301,189 @@ func Test_DefaultPriorityContextInferrer(t *testing.T) {
301301
require.Equal(t, pri, inferrer.contextPriority[ctx])
302302
}
303303
}
304+
305+
func Test_NewPriorityContextInferrer_InferStatements_DefaultContextsOrder(t *testing.T) {
306+
inferrer := newPriorityContextInferrer(componenttest.NewNopTelemetrySettings(), map[string]*priorityContextInferrerCandidate{
307+
"log": newDummyPriorityContextInferrerCandidate(true, true, []string{"scope", "instrumentation_scope", "resource"}),
308+
"metric": newDummyPriorityContextInferrerCandidate(true, true, []string{"datapoint", "scope", "instrumentation_scope", "resource"}),
309+
"datapoint": newDummyPriorityContextInferrerCandidate(true, true, []string{"scope", "instrumentation_scope", "resource"}),
310+
"span": newDummyPriorityContextInferrerCandidate(true, true, []string{"spanevent", "scope", "instrumentation_scope", "resource"}),
311+
"spanevent": newDummyPriorityContextInferrerCandidate(true, true, []string{"scope", "instrumentation_scope", "resource"}),
312+
"scope": newDummyPriorityContextInferrerCandidate(true, true, []string{"resource"}),
313+
"instrumentation_scope": newDummyPriorityContextInferrerCandidate(true, true, []string{"resource"}),
314+
"resource": newDummyPriorityContextInferrerCandidate(true, true, []string{}),
315+
})
316+
317+
tests := []struct {
318+
name string
319+
statement string
320+
expected string
321+
}{
322+
{
323+
name: "log,instrumentation_scope,resource",
324+
statement: `set(log.attributes["foo"], true) where instrumentation_scope.attributes["foo"] == resource.attributes["foo"]`,
325+
expected: "log",
326+
},
327+
{
328+
name: "log,scope,resource",
329+
statement: `set(log.attributes["foo"], true) where scope.attributes["foo"] == resource.attributes["foo"]`,
330+
expected: "log",
331+
},
332+
{
333+
name: "instrumentation_scope,resource",
334+
statement: `set(instrumentation_scope.attributes["foo"], true) where resource.attributes["foo"] != nil`,
335+
expected: "instrumentation_scope",
336+
},
337+
{
338+
name: "scope,resource",
339+
statement: `set(scope.attributes["foo"], true) where resource.attributes["foo"] != nil`,
340+
expected: "scope",
341+
},
342+
{
343+
name: "metric,instrumentation_scope,resource",
344+
statement: `set(metric.name, "foo") where instrumentation_scope.name != nil and resource.attributes["foo"] != nil`,
345+
expected: "metric",
346+
},
347+
{
348+
name: "metric,scope,resource",
349+
statement: `set(metric.name, "foo") where scope.name != nil and resource.attributes["foo"] != nil`,
350+
expected: "metric",
351+
},
352+
{
353+
name: "datapoint,metric,instrumentation_scope,resource",
354+
statement: `set(metric.name, "foo") where datapoint.double_value > 0 and instrumentation_scope.name != nil and resource.attributes["foo"] != nil`,
355+
expected: "datapoint",
356+
},
357+
{
358+
name: "datapoint,metric,scope,resource",
359+
statement: `set(metric.name, "foo") where datapoint.double_value > 0 and scope.name != nil and resource.attributes["foo"] != nil`,
360+
expected: "datapoint",
361+
},
362+
{
363+
name: "span,instrumentation_scope,resource",
364+
statement: `set(span.name, "foo") where instrumentation_scope.name != nil and resource.attributes["foo"] != nil`,
365+
expected: "span",
366+
},
367+
{
368+
name: "span,scope,resource",
369+
statement: `set(span.name, "foo") where scope.name != nil and resource.attributes["foo"] != nil`,
370+
expected: "span",
371+
},
372+
{
373+
name: "spanevent,span,instrumentation_scope,resource",
374+
statement: `set(span.name, "foo") where spanevent.name != nil and instrumentation_scope.name != nil and resource.attributes["foo"] != nil`,
375+
expected: "spanevent",
376+
},
377+
{
378+
name: "spanevent,span,scope,resource",
379+
statement: `set(span.name, "foo") where spanevent.name != nil and scope.name != nil and resource.attributes["foo"] != nil`,
380+
expected: "spanevent",
381+
},
382+
{
383+
name: "resource",
384+
statement: `set(resource.attributes["bar"], "foo") where dummy.attributes["foo"] != nil`,
385+
expected: "resource",
386+
},
387+
}
388+
389+
for _, tt := range tests {
390+
t.Run(tt.name, func(t *testing.T) {
391+
inferred, err := inferrer.inferFromStatements([]string{tt.statement})
392+
require.NoError(t, err)
393+
assert.Equal(t, tt.expected, inferred)
394+
})
395+
}
396+
}
397+
398+
func Test_NewPriorityContextInferrer_InferConditions_DefaultContextsOrder(t *testing.T) {
399+
inferrer := newPriorityContextInferrer(componenttest.NewNopTelemetrySettings(), map[string]*priorityContextInferrerCandidate{
400+
"log": newDummyPriorityContextInferrerCandidate(true, true, []string{"scope", "instrumentation_scope", "resource"}),
401+
"metric": newDummyPriorityContextInferrerCandidate(true, true, []string{"datapoint", "scope", "instrumentation_scope", "resource"}),
402+
"datapoint": newDummyPriorityContextInferrerCandidate(true, true, []string{"scope", "instrumentation_scope", "resource"}),
403+
"span": newDummyPriorityContextInferrerCandidate(true, true, []string{"spanevent", "scope", "instrumentation_scope", "resource"}),
404+
"spanevent": newDummyPriorityContextInferrerCandidate(true, true, []string{"scope", "instrumentation_scope", "resource"}),
405+
"scope": newDummyPriorityContextInferrerCandidate(true, true, []string{"resource"}),
406+
"instrumentation_scope": newDummyPriorityContextInferrerCandidate(true, true, []string{"resource"}),
407+
"resource": newDummyPriorityContextInferrerCandidate(true, true, []string{}),
408+
})
409+
410+
tests := []struct {
411+
name string
412+
condition string
413+
expected string
414+
}{
415+
{
416+
name: "log,instrumentation_scope,resource",
417+
condition: `log.attributes["foo"] !=nil and instrumentation_scope.attributes["foo"] == resource.attributes["foo"]`,
418+
expected: "log",
419+
},
420+
{
421+
name: "log,scope,resource",
422+
condition: `log.attributes["foo"] != nil and scope.attributes["foo"] == resource.attributes["foo"]`,
423+
expected: "log",
424+
},
425+
{
426+
name: "instrumentation_scope,resource",
427+
condition: `instrumentation_scope.attributes["foo"] != nil and resource.attributes["foo"] != nil`,
428+
expected: "instrumentation_scope",
429+
},
430+
{
431+
name: "scope,resource",
432+
condition: `scope.attributes["foo"] != nil and resource.attributes["foo"] != nil`,
433+
expected: "scope",
434+
},
435+
{
436+
name: "metric,instrumentation_scope,resource",
437+
condition: `metric.name != nil and instrumentation_scope.name != nil and resource.attributes["foo"] != nil`,
438+
expected: "metric",
439+
},
440+
{
441+
name: "metric,scope,resource",
442+
condition: `metric.name != nil and scope.name != nil and resource.attributes["foo"] != nil`,
443+
expected: "metric",
444+
},
445+
{
446+
name: "datapoint,metric,instrumentation_scope,resource",
447+
condition: `metric.name != nil and datapoint.double_value > 0 and instrumentation_scope.name != nil and resource.attributes["foo"] != nil`,
448+
expected: "datapoint",
449+
},
450+
{
451+
name: "datapoint,metric,scope,resource",
452+
condition: `metric.name != nil and datapoint.double_value > 0 and scope.name != nil and resource.attributes["foo"] != nil`,
453+
expected: "datapoint",
454+
},
455+
{
456+
name: "span,instrumentation_scope,resource",
457+
condition: `span.name != nil and instrumentation_scope.name != nil and resource.attributes["foo"] != nil`,
458+
expected: "span",
459+
},
460+
{
461+
name: "span,scope,resource",
462+
condition: `span.name != nil and scope.name != nil and resource.attributes["foo"] != nil`,
463+
expected: "span",
464+
},
465+
{
466+
name: "spanevent,span,instrumentation_scope,resource",
467+
condition: `span.name != nil and spanevent.name != nil and instrumentation_scope.name != nil and resource.attributes["foo"] != nil`,
468+
expected: "spanevent",
469+
},
470+
{
471+
name: "spanevent,span,scope,resource",
472+
condition: `span.name != nil and spanevent.name != nil and scope.name != nil and resource.attributes["foo"] != nil`,
473+
expected: "spanevent",
474+
},
475+
{
476+
name: "resource",
477+
condition: `resource.attributes["bar"] != nil and dummy.attributes["foo"] != nil`,
478+
expected: "resource",
479+
},
480+
}
481+
482+
for _, tt := range tests {
483+
t.Run(tt.name, func(t *testing.T) {
484+
inferred, err := inferrer.inferFromConditions([]string{tt.condition})
485+
require.NoError(t, err)
486+
assert.Equal(t, tt.expected, inferred)
487+
})
488+
}
489+
}

0 commit comments

Comments
 (0)