[pkg/ottl] Add Contains Converter #35482
Conversation
lkwronski
requested review from
TylerHelmuth,
bogdandrutu,
evan-bradley and
a team
as code owners
pkg/ottl/e2e/e2e_test.go
Outdated
| { | ||
| name: `set attribute when tag "staging" is in tags attributes slice using Contains`, | ||
| statement: []string{ | ||
| `set(attributes["tags"], ["staging", "hello", "world", "work"])`, |
There was a problem hiding this comment.
It's not clear to me why we need to support a slice of statements in this test. Isn't this first statement just a form of setup? Could it be moved into constructLogTransformContext in a way that doesn't interfere with the other tests?
There was a problem hiding this comment.
Yes, instead of adding new attributes in the constructLogTransformContext function, I simply reuse the already created slice, so the effect is the same, but I don't have to add new attributes to the current code.
| val := slice.At(i).AsRaw() | ||
| if val == item { | ||
| return true, nil | ||
| } |
There was a problem hiding this comment.
Can this return incomparable types? (e.g. slices or maps)
Can we handle the various types explicitly?
There was a problem hiding this comment.
I think it can return incomparable types, let me think how to do it explicitly. Do you think about such solution?
for i := 0; i < slice.Len(); i++ {
val := slice.At(i).AsRaw()
switch val.(type) {
case string:
if s, ok := item.(string); ok && s == val {
return true, nil
}
}
}
There was a problem hiding this comment.
I think we can check if the value is a complex type like a slice or map and perform some kind of deep equality check where necessary. For any primitive type, we can just compare them directly; I don't think we have to go as far as casting like this since two different primitive types will trivially fail the equality check.
lkwronski
force-pushed
the
lkwronski.issue-30420-contains
branch
from
417027d to
e398730
Compare
lkwronski
force-pushed
the
lkwronski.issue-30420-contains
branch
from
e398730 to
f928122
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Description: Adds a new contains function to check if slice contains item.
Link to tracking Issue: Fixes #30420
Testing: Added unit test
Documentation: Added documentation to Readme
I reopened the PR because my commissions were closed, sorry for the long response time. #33078