Skip to content

Commit a6e74fa

Browse files
edmocostasongy23
andauthored
[pkg/ottl] Fix OTTL grammar to properly handle nil string literals (#44393)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Fixed OTTL grammar to treat the string literal `"nil"` as ordinary text instead of a `nil` value. <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes #44374 <!--Describe what testing was performed and which tests were added.--> #### Testing Unit tests <!--Describe the documentation added.--> #### Documentation No changes <!--Please delete paragraphs that you did not use before submitting.--> Co-authored-by: Yang Song <[email protected]>
1 parent 3ddce5b commit a6e74fa

File tree

4 files changed

+159
-1
lines changed

4 files changed

+159
-1
lines changed

.chloggen/fix-ottl-nil-string.yaml

Lines changed: 27 additions & 0 deletions
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. receiver/filelog)
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: "Fixed OTTL grammar to treat the string literal \"nil\" as ordinary text instead of a nil value."
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: [44374]
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/e2e/e2e_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,12 @@ func Test_e2e_editors(t *testing.T) {
325325
statement: `set(attributes["test"], nil)`,
326326
want: func(_ ottllog.TransformContext) {},
327327
},
328+
{
329+
statement: `set(attributes["test"], "nil")`,
330+
want: func(tCtx ottllog.TransformContext) {
331+
tCtx.GetLogRecord().Attributes().PutStr("test", "nil")
332+
},
333+
},
328334
{
329335
statement: `set(attributes["test"], attributes["unknown"])`,
330336
want: func(_ ottllog.TransformContext) {},
@@ -1570,6 +1576,46 @@ func Test_e2e_ottl_features(t *testing.T) {
15701576
tCtx.GetLogRecord().Attributes().PutStr("my.environment.2", "ost")
15711577
},
15721578
},
1579+
{
1580+
name: "map value with nil",
1581+
statement: `set(body, {"value": nil})`,
1582+
want: func(tCtx ottllog.TransformContext) {
1583+
mapValue := tCtx.GetLogRecord().Body().SetEmptyMap()
1584+
mapValue.PutEmpty("value")
1585+
},
1586+
},
1587+
{
1588+
name: "map value with quoted nil",
1589+
statement: `set(body, {"value": "nil"})`,
1590+
want: func(tCtx ottllog.TransformContext) {
1591+
mapValue := tCtx.GetLogRecord().Body().SetEmptyMap()
1592+
mapValue.PutStr("value", "nil")
1593+
},
1594+
},
1595+
{
1596+
name: "slice with nil and quoted nil",
1597+
statement: `set(attributes["test"], [nil, "nil", nil])`,
1598+
want: func(tCtx ottllog.TransformContext) {
1599+
arr := tCtx.GetLogRecord().Attributes().PutEmptySlice("test")
1600+
arr.AppendEmpty() // nil
1601+
arr.AppendEmpty().SetStr("nil")
1602+
arr.AppendEmpty() // nil
1603+
},
1604+
},
1605+
{
1606+
name: "where clause with nil",
1607+
statement: `set(attributes["test"], "pass") where attributes["non_exiting_attrs"] == nil`,
1608+
want: func(tCtx ottllog.TransformContext) {
1609+
tCtx.GetLogRecord().Attributes().PutStr("test", "pass")
1610+
},
1611+
},
1612+
{
1613+
name: "where clause with quoted nil",
1614+
statement: `set(attributes["test"], "pass") where attributes["nil_string"] == "nil"`,
1615+
want: func(tCtx ottllog.TransformContext) {
1616+
tCtx.GetLogRecord().Attributes().PutStr("test", "pass")
1617+
},
1618+
},
15731619
}
15741620

15751621
for _, tt := range tests {
@@ -1940,6 +1986,7 @@ func constructLogTransformContext() ottllog.TransformContext {
19401986
logRecord.Attributes().PutStr("slice", "slice")
19411987
logRecord.Attributes().PutStr("val", "val2")
19421988
logRecord.Attributes().PutInt("int_value", 0)
1989+
logRecord.Attributes().PutStr("nil_string", "nil")
19431990
arr := logRecord.Attributes().PutEmptySlice("array")
19441991
arr0 := arr.AppendEmpty()
19451992
arr0.SetStr("looong")

pkg/ottl/grammar.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ func (a *argument) accept(v grammarVisitor) {
234234
// value represents a part of a parsed statement which is resolved to a value of some sort. This can be a telemetry path
235235
// mathExpression, function call, or literal.
236236
type value struct {
237-
IsNil *isNil `parser:"( @'nil'"`
237+
IsNil *isNil `parser:"( @Nil"`
238238
Literal *mathExprLiteral `parser:"| @@ (?! OpAddSub | OpMultDiv)"`
239239
MathExpression *mathExpression `parser:"| @@"`
240240
Bytes *byteSlice `parser:"| @Bytes"`
@@ -504,6 +504,7 @@ func buildLexer() *lexer.StatefulDefinition {
504504
{Name: `Float`, Pattern: `[-+]?\d*\.\d+([eE][-+]?\d+)?`},
505505
{Name: `Int`, Pattern: `[-+]?\d+`},
506506
{Name: `String`, Pattern: `"(\\.|[^\\"])*"`},
507+
{Name: `Nil`, Pattern: `\b(nil)\b`},
507508
{Name: `OpNot`, Pattern: `\b(not)\b`},
508509
{Name: `OpOr`, Pattern: `\b(or)\b`},
509510
{Name: `OpAnd`, Pattern: `\b(and)\b`},

pkg/ottl/parser_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,46 @@ func Test_parse(t *testing.T) {
944944
WhereClause: nil,
945945
},
946946
},
947+
{
948+
name: "editor with quoted nil",
949+
statement: `set(attributes["test"], "nil")`,
950+
expected: &parsedStatement{
951+
Editor: editor{
952+
Function: "set",
953+
Arguments: []argument{
954+
{
955+
Value: value{
956+
Literal: &mathExprLiteral{
957+
Path: &path{
958+
Pos: lexer.Position{
959+
Offset: 4,
960+
Line: 1,
961+
Column: 5,
962+
},
963+
Fields: []field{
964+
{
965+
Name: "attributes",
966+
Keys: []key{
967+
{
968+
String: ottltest.Strp("test"),
969+
},
970+
},
971+
},
972+
},
973+
},
974+
},
975+
},
976+
},
977+
{
978+
Value: value{
979+
String: ottltest.Strp("nil"),
980+
},
981+
},
982+
},
983+
},
984+
WhereClause: nil,
985+
},
986+
},
947987
{
948988
name: "editor with Enum",
949989
statement: `set(attributes["test"], TEST_ENUM)`,
@@ -2100,6 +2140,42 @@ func Test_parseWhere(t *testing.T) {
21002140
},
21012141
}),
21022142
},
2143+
{
2144+
statement: `nil == nil`,
2145+
expected: setNameTest(&booleanExpression{
2146+
Left: &term{
2147+
Left: &booleanValue{
2148+
Comparison: &comparison{
2149+
Left: value{
2150+
IsNil: (*isNil)(ottltest.Boolp(true)),
2151+
},
2152+
Op: eq,
2153+
Right: value{
2154+
IsNil: (*isNil)(ottltest.Boolp(true)),
2155+
},
2156+
},
2157+
},
2158+
},
2159+
}),
2160+
},
2161+
{
2162+
statement: `nil == "nil"`,
2163+
expected: setNameTest(&booleanExpression{
2164+
Left: &term{
2165+
Left: &booleanValue{
2166+
Comparison: &comparison{
2167+
Left: value{
2168+
IsNil: (*isNil)(ottltest.Boolp(true)),
2169+
},
2170+
Op: eq,
2171+
Right: value{
2172+
String: ottltest.Strp("nil"),
2173+
},
2174+
},
2175+
},
2176+
},
2177+
}),
2178+
},
21032179
}
21042180

21052181
// create a test name that doesn't confuse vscode so we can rerun tests with one click
@@ -2179,6 +2255,13 @@ func Test_ParseValueExpression_full(t *testing.T) {
21792255
return nil
21802256
},
21812257
},
2258+
{
2259+
name: "quoted nil",
2260+
valueExpression: `"nil"`,
2261+
expected: func() any {
2262+
return "nil"
2263+
},
2264+
},
21822265
{
21832266
name: "string",
21842267
valueExpression: `"string"`,

0 commit comments

Comments
 (0)