[pkg/telemetryquerylanguage] Add Join factory #13120
Conversation
evan-bradley
force-pushed
the
issue-12476
branch
from
ca77964
to
e31681e
Compare
|
/cc @kentquirk |
evan-bradley
force-pushed
the
issue-12476
branch
2 times, most recently
from
5083b7f
to
2f1e4b9
Compare
There was a problem hiding this comment.
@evan-bradley can you break this PR into 2 PRs, one where the function is added to pkg/telemetryquerylanguage and a second that adds the function to the transformprocessor? The PR that updates the transformprocessor should update one of the signal's processor_test.go with a new integration test for this function.
pkg/telemetryquerylanguage/functions/tqlcommon/func_concat_test.go
Outdated
Show resolved
Hide resolved
evan-bradley
force-pushed
the
issue-12476
branch
3 times, most recently
from
f09908a
to
eb9d296
Compare
evan-bradley
changed the title
| "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/telemetryquerylanguage/tql" | ||
| ) | ||
|
|
||
| func Join(delimiter string, vals []tql.Getter) (tql.ExprFunc, error) { |
There was a problem hiding this comment.
One thing I want to call out here is that this function could be either more efficient if we streamline the loop and Builder allocations to cut out some edge cases, or could be simpler if we are okay with allocating additional memory for a string list and letting the strings.Join function handle the joining. I took a middle-of-the-road approach since we're likely not going to be joining a substantial number of strings, but I also didn't want to be wasteful. I'm happy to consider alternate approaches if we want to optimize or simplify this.
There was a problem hiding this comment.
Your implementation seems good to me.
| "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/telemetryquerylanguage/tql" | ||
| ) | ||
|
|
||
| func Join(delimiter string, vals []tql.Getter) (tql.ExprFunc, error) { |
There was a problem hiding this comment.
Your implementation seems good to me.
evan-bradley
force-pushed
the
issue-12476
branch
from
eb9d296
to
aba0019
Compare
TylerHelmuth
added
the
ready to merge
Description:
This is a follow-up to #12706.
Add a
Concatfactory, which allows concatenating an arbitrary number of strings.This function also converts types to a string representation, if easily applicable. Converting types like numbers or byte slices should relieve the user of having to think about which type something like an attribute is when concatenating it.
The exceptions to stringification are slices and maps, which may have large payloads.
nilis stringified to allow the user flexibility and to make the resulting string more comprehensible; if they don't want to concatenate anything in these cases, a where-clause is possible.Link to tracking Issue:
Fixes #12476
Testing:
Unit tests were added, manual tests were performed.
Documentation:
The
tqlcommonpackage's README has been updated with documentation for the factory.