Skip to content

Commit

Permalink
[pkg/ottl] Unexport the grammar's constants (open-telemetry#29925)
Browse files Browse the repository at this point in the history
**Description:**
On the continuing quest to unexport the grammar, this PR unexports the
grammar's constants.

Depends on
open-telemetry#29924

Link to tracking Issue:
Related to
open-telemetry#22744
  • Loading branch information
TylerHelmuth authored Dec 18, 2023
1 parent 80c69e6 commit 7268d84
Show file tree
Hide file tree
Showing 12 changed files with 190 additions and 161 deletions.
27 changes: 27 additions & 0 deletions .chloggen/ottl-hide-consts.yaml
Original file line number Diff line number Diff line change
@@ -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: breaking

# 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: Unexport `ADD`, `SUB`, `MULT`, `DIV`, `EQ`, `NE`, `LT`, `LTE`, `GT`, and `GTE`

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [29925]

# (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:

# 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]
8 changes: 4 additions & 4 deletions pkg/ottl/boolean_value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func valueFor(x any) value {
}
case strings.Contains(v, "ENUM"):
// if the string contains ENUM construct an EnumSymbol from it.
val.Enum = (*EnumSymbol)(ottltest.Strp(v))
val.Enum = (*enumSymbol)(ottltest.Strp(v))
case v == "dur1" || v == "dur2":
val.Literal = &mathExprLiteral{
Path: &path{
Expand Down Expand Up @@ -198,9 +198,9 @@ func Test_newConditionEvaluator_invalid(t *testing.T) {
name: "unknown path",
comparison: &comparison{
Left: value{
Enum: (*EnumSymbol)(ottltest.Strp("SYMBOL_NOT_FOUND")),
Enum: (*enumSymbol)(ottltest.Strp("SYMBOL_NOT_FOUND")),
},
Op: EQ,
Op: eq,
Right: value{
String: ottltest.Strp("trash"),
},
Expand Down Expand Up @@ -488,7 +488,7 @@ func Test_newBooleanExpressionEvaluator(t *testing.T) {
Left: value{
String: ottltest.Strp("test"),
},
Op: EQ,
Op: eq,
Right: value{
String: ottltest.Strp("not test"),
},
Expand Down
62 changes: 31 additions & 31 deletions pkg/ottl/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,29 +15,29 @@ import (
// values of type any, which for the purposes of OTTL mean values that are one of
// int, float, string, bool, or pointers to those, or []byte, or nil.

// invalidComparison returns false for everything except NE (where it returns true to indicate that the
// invalidComparison returns false for everything except ne (where it returns true to indicate that the
// objects were definitely not equivalent).
// It also gives us an opportunity to log something.
func (p *Parser[K]) invalidComparison(msg string, op compareOp) bool {
p.telemetrySettings.Logger.Debug(msg, zap.Any("op", op))
return op == NE
return op == ne
}

// comparePrimitives implements a generic comparison helper for all Ordered types (derived from Float, Int, or string).
// According to benchmarks, it's faster than explicit comparison functions for these types.
func comparePrimitives[T constraints.Ordered](a T, b T, op compareOp) bool {
switch op {
case EQ:
case eq:
return a == b
case NE:
case ne:
return a != b
case LT:
case lt:
return a < b
case LTE:
case lte:
return a <= b
case GTE:
case gte:
return a >= b
case GT:
case gt:
return a > b
default:
return false
Expand All @@ -46,17 +46,17 @@ func comparePrimitives[T constraints.Ordered](a T, b T, op compareOp) bool {

func compareBools(a bool, b bool, op compareOp) bool {
switch op {
case EQ:
case eq:
return a == b
case NE:
case ne:
return a != b
case LT:
case lt:
return !a && b
case LTE:
case lte:
return !a || b
case GTE:
case gte:
return a || !b
case GT:
case gt:
return a && !b
default:
return false
Expand All @@ -65,17 +65,17 @@ func compareBools(a bool, b bool, op compareOp) bool {

func compareBytes(a []byte, b []byte, op compareOp) bool {
switch op {
case EQ:
case eq:
return bytes.Equal(a, b)
case NE:
case ne:
return !bytes.Equal(a, b)
case LT:
case lt:
return bytes.Compare(a, b) < 0
case LTE:
case lte:
return bytes.Compare(a, b) <= 0
case GTE:
case gte:
return bytes.Compare(a, b) >= 0
case GT:
case gt:
return bytes.Compare(a, b) > 0
default:
return false
Expand Down Expand Up @@ -103,10 +103,10 @@ func (p *Parser[K]) compareString(a string, b any, op compareOp) bool {
func (p *Parser[K]) compareByte(a []byte, b any, op compareOp) bool {
switch v := b.(type) {
case nil:
return op == NE
return op == ne
case []byte:
if v == nil {
return op == NE
return op == ne
}
return compareBytes(a, v, op)
default:
Expand Down Expand Up @@ -151,17 +151,17 @@ func (p *Parser[K]) compareTime(a time.Time, b any, op compareOp) bool {
switch v := b.(type) {
case time.Time:
switch op {
case EQ:
case eq:
return a.Equal(v)
case NE:
case ne:
return !a.Equal(v)
case LT:
case lt:
return a.Before(v)
case LTE:
case lte:
return a.Before(v) || a.Equal(v)
case GTE:
case gte:
return a.After(v) || a.Equal(v)
case GT:
case gt:
return a.After(v)
default:
return p.invalidComparison("invalid comparison operator", op)
Expand All @@ -177,7 +177,7 @@ func (p *Parser[K]) compare(a any, b any, op compareOp) bool {
// nils are equal to each other and never equal to anything else,
// so if they're both nil, report equality.
if a == nil && b == nil {
return op == EQ || op == LTE || op == GTE
return op == eq || op == lte || op == gte
}
// Anything else, we switch on the left side first.
switch v := a.(type) {
Expand Down Expand Up @@ -206,9 +206,9 @@ func (p *Parser[K]) compare(a any, b any, op compareOp) bool {
// If we don't know what type it is, we can't do inequalities yet. So we can fall back to the old behavior where we just
// use Go's standard equality.
switch op {
case EQ:
case eq:
return a == b
case NE:
case ne:
return a != b
default:
return p.invalidComparison("unsupported type for inequality on left", op)
Expand Down
36 changes: 18 additions & 18 deletions pkg/ottl/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func Test_compare(t *testing.T) {
name string
a any
b any
want []bool // in order of EQ, NE, LT, LTE, GTE, GT.
want []bool // in order of eq, ne, lt, lte, gte, gt.
}{
{"identity string", sa, sa, []bool{true, false, false, true, true, false}},
{"identity int64", i64a, i64a, []bool{true, false, false, true, true, false}},
Expand Down Expand Up @@ -101,7 +101,7 @@ func Test_compare(t *testing.T) {
{"non-prim, int type", testA{"hi"}, 5, []bool{false, true, false, false, false, false}},
{"int, non-prim", 5, testA{"hi"}, []bool{false, true, false, false, false, false}},
}
ops := []compareOp{EQ, NE, LT, LTE, GTE, GT}
ops := []compareOp{eq, ne, lt, lte, gte, gt}
for _, tt := range tests {
for _, op := range ops {
t.Run(fmt.Sprintf("%s %v", tt.name, op), func(t *testing.T) {
Expand All @@ -123,7 +123,7 @@ func BenchmarkCompareEQInt64(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(i64a, i64b, EQ)
testParser.compare(i64a, i64b, eq)
}
}

Expand All @@ -132,7 +132,7 @@ func BenchmarkCompareEQFloat(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(f64a, f64b, EQ)
testParser.compare(f64a, f64b, eq)
}
}

Expand All @@ -141,7 +141,7 @@ func BenchmarkCompareEQString(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(sa, sb, EQ)
testParser.compare(sa, sb, eq)
}
}

Expand All @@ -150,7 +150,7 @@ func BenchmarkCompareEQPString(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(&sa, &sb, EQ)
testParser.compare(&sa, &sb, eq)
}
}

Expand All @@ -159,7 +159,7 @@ func BenchmarkCompareEQBytes(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(ba, bb, EQ)
testParser.compare(ba, bb, eq)
}
}

Expand All @@ -168,7 +168,7 @@ func BenchmarkCompareEQNil(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(nil, nil, EQ)
testParser.compare(nil, nil, eq)
}
}

Expand All @@ -177,7 +177,7 @@ func BenchmarkCompareNEInt(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(i64a, i64b, NE)
testParser.compare(i64a, i64b, ne)
}
}

Expand All @@ -186,7 +186,7 @@ func BenchmarkCompareNEFloat(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(f64a, f64b, NE)
testParser.compare(f64a, f64b, ne)
}
}

Expand All @@ -195,7 +195,7 @@ func BenchmarkCompareNEString(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(sa, sb, NE)
testParser.compare(sa, sb, ne)
}
}

Expand All @@ -204,7 +204,7 @@ func BenchmarkCompareLTFloat(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(f64a, f64b, LT)
testParser.compare(f64a, f64b, lt)
}
}

Expand All @@ -213,7 +213,7 @@ func BenchmarkCompareLTString(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(sa, sb, LT)
testParser.compare(sa, sb, lt)
}
}

Expand All @@ -222,17 +222,17 @@ func BenchmarkCompareLTNil(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
testParser.compare(nil, nil, LT)
testParser.compare(nil, nil, lt)
}
}

// this is only used for benchmarking, and is a rough equivalent of the original compare function
// before adding LT, LTE, GTE, and GT.
// before adding lt, lte, gte, and gt.
func compareEq(a any, b any, op compareOp) bool {
switch op {
case EQ:
case eq:
return a == b
case NE:
case ne:
return a != b
default:
return false
Expand All @@ -241,6 +241,6 @@ func compareEq(a any, b any, op compareOp) bool {

func BenchmarkCompareEQFunction(b *testing.B) {
for i := 0; i < b.N; i++ {
compareEq(sa, sb, EQ)
compareEq(sa, sb, eq)
}
}
2 changes: 1 addition & 1 deletion pkg/ottl/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ func (p *Parser[K]) newGetter(val value) (Getter[K], error) {
}

if val.Enum != nil {
enum, err := p.enumParser(val.Enum)
enum, err := p.enumParser((*EnumSymbol)(val.Enum))
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/expression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func Test_newGetter(t *testing.T) {
{
name: "enum",
val: value{
Enum: (*EnumSymbol)(ottltest.Strp("TEST_ENUM_ONE")),
Enum: (*enumSymbol)(ottltest.Strp("TEST_ENUM_ONE")),
},
want: int64(1),
},
Expand Down
4 changes: 3 additions & 1 deletion pkg/ottl/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ type EnumParser func(*EnumSymbol) (*Enum, error)

type Enum int64

type EnumSymbol string

func newPath[K any](fields []field) Path[K] {
if len(fields) == 0 {
return nil
Expand Down Expand Up @@ -430,7 +432,7 @@ func (p *Parser[K]) buildArg(argVal value, argType reflect.Type) (any, error) {
}
return StandardTimeGetter[K]{Getter: arg.Get}, nil
case name == "Enum":
arg, err := p.enumParser(argVal.Enum)
arg, err := p.enumParser((*EnumSymbol)(argVal.Enum))
if err != nil {
return nil, fmt.Errorf("must be an Enum")
}
Expand Down
Loading

0 comments on commit 7268d84

Please sign in to comment.