Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Filterx handle unknown function arguments #153

Merged
merged 21 commits into from
Jun 10, 2024
Merged

Filterx handle unknown function arguments #153

merged 21 commits into from
Jun 10, 2024

Conversation

bazsi
Copy link
Member

@bazsi bazsi commented Jun 7, 2024 •
edited
Loading

This branch implements the handling of incorrect filterx function arguments, and also improves error handling aesthetics in multiple ways.

Here's a list of user-visible changes:

  • in some cases, the documentation/contact links were not printed on syntax errors, this was fixed
  • the primary error upon syntax error was easily lost in the log/error output, make it stand out
  • errors in function arguments are reported nicely, both when detected at compile time and at runtime

Internal changes:

  • it is now possible to set a filterx error condition where the extra info is a string and not a filterx object. Sometimes the filterx object is not available and creating one just for reporting an error would be an overhead. This should make it easier to report errors via the filterx error mechanism, instead of msg_error()
  • improve argument handling performance, use a GPtrArray to represent arg expressions, instead of a GList, which is iterated by index

@bazsi bazsi force-pushed the filterx-handle-unknown-function-arguments branch 2 times, most recently from 54479d2 to 5adc196 Compare June 8, 2024 09:59
@bazsi bazsi changed the title WIP: Filterx handle unknown function arguments Filterx handle unknown function arguments Jun 8, 2024
@bazsi bazsi force-pushed the filterx-handle-unknown-function-arguments branch 2 times, most recently from 0ca56f0 to fe4afd1 Compare June 9, 2024 08:17
alltilla
alltilla reviewed Jun 10, 2024
View reviewed changes
Copy link
Member

@alltilla alltilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only have one rebase related comment, otherwise LGTM.

Thanks for this!

modules/examples/filterx/example-filterx-func/example-filterx-func-plugin.c Show resolved Hide resolved
bazsi added 21 commits June 10, 2024 10:11
@bazsi @alltilla
cfg-parser: move documentation and contact report to cfg_parser_parse()
4771d75 
This fixes a problem where the documentation link is not printed in
case CHECK_ERROR_WITHOUT_MESSAGE() is used in the main grammar.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
cfg-parser: improve error display
d0d6a9e 
Improve error reporting by highlighting the actual error message a bit more.
The error message was printed at the end of the startup log which may not be
visible enough. Leave an empty line to make sure it is easy to notice for the user.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx: add filterx_eval_push_error_info() function
e8e8dbf 
This is an alternative to the normal push_error() function which takes
a string as an extra detail to the error instead of an object.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/eval: handle cases where we push an error state without eval …
3a4e9f2 
…context

This only happens in unit test programs.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr: handle expr == NULL in filterx_expr_format_location_tag
a4d639e 
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/grammar: improve formatting for function call related errors
b45092a 
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
otel/filterx: use "self" instead of "s" to match conventions
043fe21 
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx: move FilterXExpr typedef to filterx-object
9ac5b62 
object related functions (such as type casts) are tied to FilterXExpr anyway,
so let's declare the type at least, as there's an inherent interdependency
between them.


Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr-function: store the FilterXFunctionArg instance instead …
784b46d 
…of just the expr

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr-function: use a GPtrArray for positional arguments
133d123 
Instead of a list use a pointer array.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr-function: add filterx_function_args_empty()
017e7d7 
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr-function: handle invalid arguments for functions
1b1dcab 
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr-function: prepare self->function_name to be displayed
50c3d65 
So far function_name was only saved and not displayed, but whenever we
display it, adding () at the end helps users recognize this is a function.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr-function: argument error reporting for simple functions
e825cb4 
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr-function: add filterx_simple_function_argument_error()
8a0b8fb 
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr-function: don't double-check if a simple function argume…
d0837c9 
…nt is null

The simple function framework already ensures that all arguments are
non-null, so don't double-check them, as it adds another error path and
increases cognitive load.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx/expr-function: add type-safe simple function framework
7e341fa 
Simple functions are just callbacks which were simply cast to an untyped
pointer, making it difficult to change their prototype.

By introducing a type-safe framework, we will at least get a warning from
the compiler when there's a function prototype mismatch.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
otel/filterx: add simple initializers for otel filterx objects
2aedde1 
Instead of calling the filterx-exposed simple function, add a wrapper that
removes this complexity.  btw, we should probably swap these two functions,
provide a low-level constructor and a filterx simple function on top of it.

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
filterx: add filterx error handling to simple functions argument vali…
8019487 
…dation

This patch does two things:
1) adds FilterXExpr * argument to all simple functions
2) changes msg_error() style error handling to filterx style

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
light/test_filterx: fix strip_whitespace parameter name to parse_csv
b77783c 
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@bazsi @alltilla
all: fix style issues
4d2bcfb 
Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>
@alltilla alltilla force-pushed the filterx-handle-unknown-function-arguments branch from 1bd9dda to 4d2bcfb Compare June 10, 2024 08:14
@alltilla alltilla merged commit 8df5d39 into axoflow:main Jun 10, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alltilla alltilla alltilla approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bazsi @alltilla