Add new rewrite function: set-severity() #3115

smortex · 2020-02-04T03:28:59Z

This is a follow-up to #2602 by @Kokan

I rebased the code on top of master, fixed the build, and renamed the function set-severity as requested in #2602 (comment)

A few more points where under discussion, I will add in-line notes to discuss the topics further.

This PR also fixes cisco-parser $PRIORITY macro parsing.

Usage: set-severtity(TEMPLATE)

Accepted values are numeric strings [0-7] or named values (emerg, emergency, panic, alert, crit, critical, err, error, warning, warn, notice, info, informational, debug).

Examples:

rewrite {
  set-severity("info");
  set-severity("6");
  set-severity("${.json.severity}");
};

kira-syslogng · 2020-02-04T03:29:02Z

This user does not have permission to start the build. Can one of the admins verify this patch and start the build?
(admin: you have the next options (make sure you checked the code):
"ok to test" to accept this pull request (and further changes) for testing
"test this please" for a one time test run
"add to whitelist" add author of a Pull Request to whitelist (globally, be careful, it means this user can trigger kira for any PR)
do nothing -> CI won't start)

kira-syslogng · 2020-02-04T03:29:02Z

This user does not have permission to start the build. Can one of the admins verify this patch and start the build?
(admin: you have the next options (make sure you checked the code):
"ok to test" to accept this pull request (and further changes) for testing
"test this please" for a one time test run
"add to whitelist" add author of a Pull Request to whitelist (globally, be careful, it means this user can trigger kira for any PR)
do nothing -> CI won't start)

kira-syslogng · 2020-02-04T03:29:03Z

This user does not have permission to start the build. Can one of the admins verify this patch and start the build?
(admin: you have the next options (make sure you checked the code):
"ok to test" to accept this pull request (and further changes) for testing
"test this please" for a one time test run
"add to whitelist" add author of a Pull Request to whitelist (globally, be careful, it means this user can trigger kira for any PR)
do nothing -> CI won't start)

lib/rewrite/rewrite-set-severity.c

Kokan · 2020-02-04T04:28:22Z

@kira-syslogng ok to test

kira-syslogng · 2020-02-04T04:34:43Z

Build FAILURE

kira-syslogng · 2020-02-04T05:28:52Z

Build FAILURE

Signed-off-by: Kokan <[email protected]>

Changes the message level based on its numeric value: ``` rewrite { set-level("${user_macro}"); }; ``` The option is template that expects to be resolved into a numeric representation of a valid level. Signed-off-by: Kokan <[email protected]>

Signed-off-by: Kokan <[email protected]>

``` rewrite { set-level("critical"); }; ``` Now the macro accepts in text format the levels as well. Signed-off-by: Kokan <[email protected]>

Signed-off-by: Kokan <[email protected]>

Signed-off-by: Romain Tartière <[email protected]>

kira-syslogng · 2020-02-04T09:46:53Z

Build FAILURE

lib/rewrite/rewrite-expr-parser.c

smortex · 2020-02-04T19:06:53Z

I fixed a bunch of refactoring mistakes yesterday, but kira is still failing. May someone with access to kira logs help by providing some insights about what is going wrong? Thanks!

Kokan · 2020-02-04T19:31:59Z

@smortex Only one test failed, and that failure is actually test case issue. (Cisco parser tests actually asserts on wrong priority value.) I'll push the fix tomorrow, and rerun kira.

Also I plan to do a review, regarding the rename part. I'll probably have some not one backward compat.

Kokan · 2020-02-05T07:03:26Z

@kira-syslogng retest this please; branch=kokan-github-pr-3115

kira-syslogng · 2020-02-05T07:27:03Z

Build FAILURE

The "emergency" severity has numerical code 0, so this value should be accepted as a valid severity for proper messages classification. Signed-off-by: Romain Tartière <[email protected]>

kira-syslogng · 2020-02-19T19:36:54Z

Build FAILURE

gaborznagy · 2020-02-21T09:35:12Z

@kira-syslogng retest this please branch=kokan-github-pr-3115;

kira-syslogng · 2020-02-21T09:58:28Z

Build SUCCESS

MrAnno

@smortex Thank you for opening, fixing, and finishing this PR! 👍

I've found 2 bugs, only one of them seems important (set-severity() sets emerg when its template evaluates to an empty string).

lib/rewrite/tests/test_set_severity.c

lib/rewrite/rewrite-set-severity.c

kira-syslogng · 2020-02-23T23:46:19Z

Build FAILURE

MrAnno · 2020-02-24T07:40:29Z

@kira-syslogng retest this please branch=kokan-github-pr-3115;

kira-syslogng · 2020-02-24T08:03:25Z

Build SUCCESS

Signed-off-by: Romain Tartière <[email protected]>

When a template evaulates as an empty string, strtol() returns 0 and endptr[0] is NULL. This result in a misclassification of the message with a severity of 0, aka "emergency". Fix this by ensuring that there is nothing left to be read AND the end position is different from the start position. Signed-off-by: Romain Tartière <[email protected]>

kira-syslogng · 2020-02-24T09:24:50Z

Build FAILURE

gaborznagy · 2020-02-24T11:31:35Z

@kira-syslogng retest this please branch=kokan-github-pr-3115;

kira-syslogng · 2020-02-24T11:54:31Z

Build SUCCESS

Unfortunately, 20 years ago, syslog-ng choose the term "LEVEL" to refer to the severity of a message, as that was the language used by syslog(3) manual page. Quoting the manpage: The priority argument is formed by ORing together a facility value and a level value (described below). If no facility value is ORed into priority, then the default value set by openlog() is used, or, if there was no preceding openlog() call, a default of LOG_USER is employed. However RFC3164 adopted the term "severity", quoting RFC3164: The number contained within these angle brackets is known as the Priority value and represents both the Facility and Severity as described below. syslog-ng predates the choice in RFC3164, however I think it is highly time to adopt the language of the RFC, especially as it is better than the original. See discussions in github PR syslog-ng#3115 for further details. Signed-off-by: Balazs Scheidler <[email protected]>

Signed-off-by: László Várady <[email protected]>

smortex force-pushed the rewrite-pri branch from bd8db77 to cddc73b Compare February 4, 2020 03:34

smortex commented Feb 4, 2020

View reviewed changes

lib/rewrite/rewrite-set-severity.c Outdated Show resolved Hide resolved

smortex force-pushed the rewrite-pri branch from cddc73b to 606c925 Compare February 4, 2020 03:47

smortex force-pushed the rewrite-pri branch from 606c925 to b9ea325 Compare February 4, 2020 05:08

Kokan self-requested a review February 4, 2020 08:24

smortex force-pushed the rewrite-pri branch 2 times, most recently from 57aa8ea to 7e0efd4 Compare February 4, 2020 09:11

Kokan and others added 6 commits February 3, 2020 23:11

cfg-grammar: use LOG_PRIMASK

ff45e15

Signed-off-by: Kokan <[email protected]>

rewrite: set-level()

9386a6a

Changes the message level based on its numeric value: ``` rewrite { set-level("${user_macro}"); }; ``` The option is template that expects to be resolved into a numeric representation of a valid level. Signed-off-by: Kokan <[email protected]>

cisco-parser: update LEVEL hard macro

c534b51

Signed-off-by: Kokan <[email protected]>

set-level: accept text format

5526fca

``` rewrite { set-level("critical"); }; ``` Now the macro accepts in text format the levels as well. Signed-off-by: Kokan <[email protected]>

set-level: use scratch buffer

b919194

Signed-off-by: Kokan <[email protected]>

set-level: Fix build after rebase

9f875e2

Signed-off-by: Romain Tartière <[email protected]>

smortex force-pushed the rewrite-pri branch from 7e0efd4 to 06dcd96 Compare February 4, 2020 09:12

set-level: Rename to set-severity

bdc619e

Signed-off-by: Romain Tartière <[email protected]>

smortex force-pushed the rewrite-pri branch from 06dcd96 to bdc619e Compare February 4, 2020 09:26

smortex commented Feb 4, 2020

View reviewed changes

lib/rewrite/rewrite-expr-parser.c Show resolved Hide resolved

smortex changed the title ~~Rewrite pri~~ New rewrite function: set-severity() Feb 4, 2020

smortex changed the title ~~New rewrite function: set-severity()~~ Add new rewrite function: set-severity() Feb 4, 2020

set-severity: Fix setting severity to emergency

a48c6d1

The "emergency" severity has numerical code 0, so this value should be accepted as a valid severity for proper messages classification. Signed-off-by: Romain Tartière <[email protected]>

smortex force-pushed the rewrite-pri branch from 7869920 to a48c6d1 Compare February 19, 2020 19:12

gaborznagy previously approved these changes Feb 21, 2020

View reviewed changes

MrAnno suggested changes Feb 22, 2020

View reviewed changes

lib/rewrite/tests/test_set_severity.c Show resolved Hide resolved

lib/rewrite/rewrite-set-severity.c Show resolved Hide resolved

lib/rewrite/rewrite-set-severity.c Show resolved Hide resolved

smortex dismissed gaborznagy’s stale review via 1e2a8dd February 23, 2020 23:22

smortex requested review from MrAnno and gaborznagy February 24, 2020 05:24

MrAnno previously approved these changes Feb 24, 2020

View reviewed changes

smortex added 2 commits February 23, 2020 23:00

set-severity: Avoid reading an unallocated message

353ea3c

Signed-off-by: Romain Tartière <[email protected]>

smortex dismissed MrAnno’s stale review via a552d62 February 24, 2020 09:00

smortex force-pushed the rewrite-pri branch from 1e2a8dd to a552d62 Compare February 24, 2020 09:00

MrAnno approved these changes Feb 24, 2020

View reviewed changes

gaborznagy approved these changes Feb 24, 2020

View reviewed changes

gaborznagy merged commit dc212b5 into syslog-ng:master Feb 24, 2020

MrAnno added a commit to MrAnno/syslog-ng that referenced this pull request Feb 24, 2020

news: add syslog-ng#3115

7dcbfab

Signed-off-by: László Várady <[email protected]>

alltilla pushed a commit to alltilla/syslog-ng that referenced this pull request Feb 25, 2020

news: add syslog-ng#3115

3e73fa5

Signed-off-by: László Várady <[email protected]>

smortex deleted the rewrite-pri branch March 2, 2020 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new rewrite function: set-severity() #3115

Add new rewrite function: set-severity() #3115

smortex commented Feb 4, 2020 •

edited by MrAnno

Loading

kira-syslogng commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

Kokan commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

smortex commented Feb 4, 2020

Kokan commented Feb 4, 2020

Kokan commented Feb 5, 2020

kira-syslogng commented Feb 5, 2020

kira-syslogng commented Feb 19, 2020

gaborznagy commented Feb 21, 2020

kira-syslogng commented Feb 21, 2020

MrAnno left a comment

kira-syslogng commented Feb 23, 2020

MrAnno commented Feb 24, 2020

kira-syslogng commented Feb 24, 2020

kira-syslogng commented Feb 24, 2020

gaborznagy commented Feb 24, 2020

kira-syslogng commented Feb 24, 2020

Add new rewrite function: set-severity() #3115

Add new rewrite function: set-severity() #3115

Conversation

smortex commented Feb 4, 2020 • edited by MrAnno Loading

kira-syslogng commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

Kokan commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

kira-syslogng commented Feb 4, 2020

smortex commented Feb 4, 2020

Kokan commented Feb 4, 2020

Kokan commented Feb 5, 2020

kira-syslogng commented Feb 5, 2020

kira-syslogng commented Feb 19, 2020

gaborznagy commented Feb 21, 2020

kira-syslogng commented Feb 21, 2020

MrAnno left a comment

Choose a reason for hiding this comment

kira-syslogng commented Feb 23, 2020

MrAnno commented Feb 24, 2020

kira-syslogng commented Feb 24, 2020

kira-syslogng commented Feb 24, 2020

gaborznagy commented Feb 24, 2020

kira-syslogng commented Feb 24, 2020

smortex commented Feb 4, 2020 •

edited by MrAnno

Loading