Add expr language for logs and use it in filter processor #5680
Conversation
Signed-off-by: Dominik Rosiek <[email protected]>
sumo-drosiek
force-pushed
the
drosiek-log-matcher
branch
from
9a150f7 to
6c7b474
Compare
Signed-off-by: Dominik Rosiek <[email protected]>
Signed-off-by: Dominik Rosiek <[email protected]>
sumo-drosiek
force-pushed
the
drosiek-log-matcher
branch
from
6a1e31a to
0d2617c
Compare
Signed-off-by: Dominik Rosiek <[email protected]>
Signed-off-by: Dominik Rosiek <[email protected]>
Signed-off-by: Dominik Rosiek <[email protected]>
Signed-off-by: Dominik Rosiek <[email protected]>
Signed-off-by: Dominik Rosiek <[email protected]>
Signed-off-by: Dominik Rosiek <[email protected]>
|
Sorry for being slow in reviewing this. I plan on reviewing this tomorrow. Ping me again if I don't get it ready by then. |
|
@jpkrohling pinging :) |
There was a problem hiding this comment.
This looks very cool, I didn't know we had it for metrics already! I have only one question about the security considerations, and they might be documented already as part of the metrics part. There are also a couple of minor things to adjust, but overall, this LGTM!
Co-authored-by: Juraci Paixão Kröhling <[email protected]>
Co-authored-by: Juraci Paixão Kröhling <[email protected]>
Co-authored-by: Juraci Paixão Kröhling <[email protected]>
Signed-off-by: Dominik Rosiek <[email protected]>
Signed-off-by: Dominik Rosiek <[email protected]>
| To ensure that everything is safe and secure: | ||
|
|
||
| - only basic types (like `string` instead of more complex log body) are being exposed | ||
| - expr has only access to a copy of object values |
There was a problem hiding this comment.
Is it possible for an outside attacker to send a payload that would cause the collector to crash, perhaps as it would allocate much more memory than the payload itself? I guess not, but if there are any protections by the underlying library on this area, it would be good to mention here.
There was a problem hiding this comment.
I didn't find answer in documentation. I believe @djaglowski should know more about expr language
There was a problem hiding this comment.
I'm not very familiar with the internals of the dependency, but it appears there is some limit to the memory.
jpkrohling
added
the
ready to merge
There was a problem hiding this comment.
In general I like this approach, but @punya owes us the configuration for filtering that we should use for filter processors and for "transformation" processors.
bogdandrutu
removed
the
ready to merge
|
@bogdandrutu I would like to take this as a starting point and commit to using it for filters and transforms as well. antonmedv/expr is general-purpose and can be applied there too. |
|
@sumo-drosiek and @djaglowski, I created #5887 for the detailed evaluation. I'd love to work together on this - I'll reach out on Slack to find a time to kick off. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Description:
Link to tracking Issue:
#5679
Testing:
Documentation:
README