lib/filterx: add startswith / endswith functions #277

OverOrion · 2024-09-06T18:43:47Z

@version: 4.8

log {
  source {stdin(flags(no-parse));};
  filterx {
    declare needle = "bar";
    declare path = "";

    if (startswith($MSG, "foo", ignorecase=true)) {
      path = "starts with foo";
    }
    elif (startswith($MSG, needle)) {
      path = "starts with bar";
    }
    elif (endswith($MSG, "foo", ignorecase=true)) {
      path = "ends with foo";
    }
    elif (endswith($MSG, needle, ignorecase=true)) {
      path = "ends with bar";
    }
    else {path = "NOPE";};
  };
  filterx {vars();};
  destination { file("/dev/stdout"); };
};

Signed-off-by: Szilard Parrag <[email protected]>

alltilla · 2024-09-09T07:45:50Z

lib/filterx/func-str.c

@@ -33,6 +33,7 @@


 #define FILTERX_FUNC_STARTSWITH_USAGE "Usage: startswith(my_string, my_prefix)"
+#define FILTERX_FUNC_ENDSWITH_USAGE "Usage: endswith(my_string, my_prefix)"


typo: "my_suffix"

alltilla · 2024-09-09T07:51:35Z

lib/filterx/func-str.c

+  FilterXExpr *needle_expr = filterx_function_args_get_expr(args, 1);
+  if (!filterx_expr_is_literal(needle_expr))
+    {
+      needle->expr = filterx_expr_ref(needle_expr);


I think this ref is unnecessary.

alltilla · 2024-09-09T07:51:49Z

lib/filterx/func-str.c

+  gsize needle_str_len;
+  const gchar *needle_str = filterx_function_args_get_literal_string(args, 1, &needle_str_len);
+  if (!needle_str)
+    return FALSE;


Please set an error message here.

alltilla · 2024-09-09T08:56:54Z

lib/filterx/func-str.c

+      return FALSE;
+    }
+  if (!ignorecase)
+    *needle_str = g_strdup(needle_str_res);


I guess that this strdup() is here because this way the function always returns a string that needs to be freed, but if ignorecase is not set, this is basically an unnecessary copy. Can we solve this somehow? Maybe we can return whether the returned string needs freeing or not. Or we could check for self->ignorecase on the call site to decide whether we need to free or not (the check could even be extracted to a function, but it might be overkill).

Same for haystack.

alltilla · 2024-09-09T09:01:59Z

lib/filterx/func-str.c

+  filterx_expr_unref(self->haystack);
+  if (self->needle.expr)
+    filterx_expr_unref(self->needle.expr);
+


Who frees self->needle.literal.str?

alltilla · 2024-09-09T09:05:27Z

lib/filterx/func-str.h

+typedef struct _FilterXExprOrLiteral
+{
+  FilterXExpr *expr;
+  struct
+  {
+    gchar *str;
+    gssize str_len;
+  } literal;
+} FilterXExprOrLiteral;
+
+typedef struct _FilterXFuncStartsWith
+{
+  FilterXFunction super;
+  FilterXExpr *haystack;
+  FilterXExprOrLiteral needle;
+  gsize needle_len;
+  gboolean ignore_case;
+} FilterXFuncStartsWith;


These could be "private" == defined in the .c file.

alltilla · 2024-09-09T10:09:32Z

tests/light/functional_tests/filterx/test_filterx.py

+def test_startswith_literal(config, syslog_ng):
+    (file_true, file_false) = create_config(
+        config, r"""
+            if (startswith($MSG, "foo"))
+              {
+                $MSG = json();
+                $MSG.startswith_foo = true;
+              };
+        """, msg="foo",
+    )
+    syslog_ng.start(config)
+
+    assert file_true.get_stats()["processed"] == 1
+    assert "processed" not in file_false.get_stats()
+    assert file_true.read_log() == '{"startswith_foo":true}\n'
+
+
+def test_startswith_expr_ignorecase(config, syslog_ng):
+    (file_true, file_false) = create_config(
+        config, r"""
+            prefix = "FoO";
+            if (startswith($MSG, prefix, ignorecase=true))
+              {
+                $MSG = json();
+                $MSG.startswith_foo_ignorecase = true;
+              };
+        """, msg="foo",
+    )
+    syslog_ng.start(config)
+
+    assert file_true.get_stats()["processed"] == 1
+    assert "processed" not in file_false.get_stats()
+    assert file_true.read_log() == '{"startswith_foo_ignorecase":true}\n'


[optional]

Having 2 (4 with endswith) testcases is cleaner, but takes longer time to execute, as we start syslog-ng multiple times, and it has an overhead.
Could you merge the 4 into one testcase, please?

OverOrion force-pushed the filterx-startswith-endswith branch from 0b96fb7 to e3bf29f Compare September 6, 2024 18:49

OverOrion added 5 commits September 6, 2024 20:52

lib/filterx: add startswith

3262dbd

Signed-off-by: Szilard Parrag <[email protected]>

lib/filterx: add endswith

306ecea

Signed-off-by: Szilard Parrag <[email protected]>

lib/filterx: register startswith/endswith

b5cd4c6

Signed-off-by: Szilard Parrag <[email protected]>

lib/filterx: add E2E test for startswith

700d467

Signed-off-by: Szilard Parrag <[email protected]>

lib/filterx: add E2E test for endswith

2b0315b

Signed-off-by: Szilard Parrag <[email protected]>

OverOrion force-pushed the filterx-startswith-endswith branch from e3bf29f to 2b0315b Compare September 6, 2024 18:52

OverOrion requested a review from alltilla September 9, 2024 06:01

alltilla reviewed Sep 9, 2024

View reviewed changes

OverOrion closed this Sep 19, 2024

OverOrion mentioned this pull request Sep 19, 2024

lib/filterx: add startswith / endswith / includes functions #297

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib/filterx: add startswith / endswith functions #277

lib/filterx: add startswith / endswith functions #277

OverOrion commented Sep 6, 2024

alltilla Sep 9, 2024

alltilla Sep 9, 2024

alltilla Sep 9, 2024

alltilla Sep 9, 2024

alltilla Sep 9, 2024

alltilla Sep 9, 2024

alltilla Sep 9, 2024

		@@ -33,6 +33,7 @@


		#define FILTERX_FUNC_STARTSWITH_USAGE "Usage: startswith(my_string, my_prefix)"
		#define FILTERX_FUNC_ENDSWITH_USAGE "Usage: endswith(my_string, my_prefix)"

lib/filterx: add startswith / endswith functions #277

lib/filterx: add startswith / endswith functions #277

Conversation

OverOrion commented Sep 6, 2024

alltilla Sep 9, 2024

Choose a reason for hiding this comment

alltilla Sep 9, 2024

Choose a reason for hiding this comment

alltilla Sep 9, 2024

Choose a reason for hiding this comment

alltilla Sep 9, 2024

Choose a reason for hiding this comment

alltilla Sep 9, 2024

Choose a reason for hiding this comment

alltilla Sep 9, 2024

Choose a reason for hiding this comment

alltilla Sep 9, 2024

Choose a reason for hiding this comment