opentelemetry-collector-contrib
opentelemetry-collector-contrib copied to clipboard
Support IsMatch function take pattern parameter from attributes
Component(s)
No response
Is your feature request related to a problem? Please describe.
While collecting stdout logs, we would like to filter some containers based on pod annotations. With current filterprocessor and static string can be used as pattern, I request to IsMatch function to accept attributes as second parameter
Describe the solution you'd like
I suggested my solution with this PR
Describe alternatives you've considered
No response
Additional context
No response
Pinging code owners for pkg/ottl: @TylerHelmuth @kentquirk @bogdandrutu @evan-bradley. See Adding Labels via Comments if you do not have permissions to add labels yourself.
@evan-bradley we've had a "standard" (not documented anywhere) that patterns in OTTL functions be static. So far this has provided a couple things:
- regex validation during startup
- building the regex once and reusing it, saving some performance (theoretically).
If we update the pattern to allow dynamic value we'll loose those features even when using static strings (unless we get fancy, which maybe we can).
I've always imagined we'd have a use case where the pattern needs to come from an attribute and it has now come up. I am ok allowing dynamic values, and if we do, I'd like to allow the pattern for all functions that need them.
I'd also like to allow providing dynamic values in any function arguments that hold arbitrary strings. We definitely can find a way to keep our current functionality for static-value arguments, but I think we will need to add to the language to enable that.
I wonder if we could add to the Getter interface some sort of IsStatic function that helps identify this Getter as returning a static value.
Is there any update about this issue?
If you can come up with a way to perform regex validation of static strings when the pattern parameter is a StringGetter that would be the preferred option, but I'm willing to move to a StringGetter even if that isn't possible.
Please add some benchmarks to the function so we can compare the performance before and after.
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.
Pinging code owners:
- pkg/ottl: @TylerHelmuth @kentquirk @bogdandrutu @evan-bradley
See Adding Labels via Comments if you do not have permissions to add labels yourself.
Hi @TylerHelmuth I wrote some benchmark test in my fork
In proposed solution, I cached compiled patterns and wrote 2 benchmark test, 1 for same pattern with same target and the other one for changing pattern in each iteration and keeping target same.
Results for proposed change:
goos: darwin goarch: arm64 pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottlfuncs Benchmark_performance_same_pattern-8 2390307 503.1 ns/op Benchmark_performance_different_pattern-8 466681 3142 ns/op PASS ok github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottlfuncs 3.950s
Results for existing:
goos: darwin goarch: arm64 pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottlfuncs Benchmark_performance_same_pattern-8 705002 1683 ns/op Benchmark_performance_different_pattern-8 610514 2195 ns/op PASS ok github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottlfuncs 3.325s
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.
Pinging code owners:
- pkg/ottl: @TylerHelmuth @kentquirk @bogdandrutu @evan-bradley
See Adding Labels via Comments if you do not have permissions to add labels yourself.
@rtekdogan thank you for those benchmarks. I have not had time to work on this issue yet, but I think there is still a way for a Getter to announce that it is static and can be called before hot-path execution.
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.
Pinging code owners:
- pkg/ottl: @TylerHelmuth @kentquirk @bogdandrutu @evan-bradley
See Adding Labels via Comments if you do not have permissions to add labels yourself.