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.

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

cfg: add named logpaths with counters #4344

Merged
merged 15 commits into from Mar 6, 2023
Merged

cfg: add named logpaths with counters #4344

merged 15 commits into from Mar 6, 2023

Conversation

alltilla
Copy link
Collaborator

@alltilla alltilla commented Feb 24, 2023 •
edited

E.g.:

log top-level {
    source(s_local);

    log inner-1 {
        filter(f_inner_1);
        destination(d_local_1);
    };

    log inner-2 {
        filter(f_inner_2);
        destination(d_local_2);
    };
};

Each named log path counts its ingress and egress messages:

syslogng_log_path_ingress{id="top-level"} 114
syslogng_log_path_ingress{id="inner-1"} 114
syslogng_log_path_ingress{id="inner-2"} 114
syslogng_log_path_egress{id="top-level"} 103
syslogng_log_path_egress{id="inner-1"} 62
syslogng_log_path_egress{id="inner-2"} 41

Note that the egress statistics only count the messages which have been have not been filtered out from the related
log path, it does care about whether there are any destinations in it or that any destination delivers or drops the message.

Depends on #4325 and #4339.

Signed-off-by: Attila Szakacs szakacs.attila96@gmail.com

@github-actions
Copy link
Contributor

No news file has been detected. Please write one, if applicable.

@alltilla alltilla force-pushed the named-log-paths branch 2 times, most recently from 8a90734 to 3399d17 Compare February 27, 2023 08:04
@MrAnno MrAnno self-requested a review February 27, 2023 08:59
@MrAnno MrAnno added this to the syslog-ng 4.1 milestone Feb 27, 2023
@alltilla
config-graph-json-to-dot: use boxes for nodes
a9ae8db 
It makes a neater output IMO.

Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla alltilla force-pushed the named-log-paths branch 2 times, most recently from c017eeb to 4044b4e Compare March 1, 2023 09:26
@alltilla
Copy link
Collaborator Author

alltilla commented Mar 1, 2023

Updated the E2E tests according to what we agreed on.

@alltilla alltilla force-pushed the named-log-paths branch 4 times, most recently from 949234f to 2d374ea Compare March 2, 2023 15:42
@kira-syslogng
Copy link
Contributor

Build FAILURE

MrAnno
MrAnno reviewed Mar 3, 2023
View reviewed changes
Copy link
Collaborator

@MrAnno MrAnno left a comment

Choose a reason for hiding this comment

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

Nice!

I would like to look into the commit [cfg-tree: inject metrics-pipes to named log paths](https://github.com/syslog-ng/syslog-ng/pull/4344/commits/e572f54b4eaf06b82d2fe8a9a94c5cbac6bbb88d) more closely, I'll get back with an approve or another review note in a few hours.

lib/cfg-grammar.y Outdated Show resolved Hide resolved
lib/cfg-tree.c Show resolved Hide resolved
lib/metrics-pipe.c Outdated Show resolved Hide resolved
@MrAnno
Copy link
Collaborator

MrAnno commented Mar 3, 2023

@kira-syslogng do stresstest

@kira-syslogng
Copy link
Contributor

Kira-stress-test: Build FAILURE

@MrAnno
Copy link
Collaborator

MrAnno commented Mar 3, 2023

@HofiOne Kira is broken due to an internal Java change. Can you check it, please?

@kira-syslogng
Copy link
Contributor

Build FAILURE

@HofiOne
Copy link
Collaborator

HofiOne commented Mar 3, 2023

@HofiOne Kira is broken due to an internal Java change. Can you check it, please?

done, sorry for the inconvenience

@MrAnno
Copy link
Collaborator

MrAnno commented Mar 3, 2023

@kira-syslogng retest this please

@MrAnno
Copy link
Collaborator

MrAnno commented Mar 3, 2023

@kira-syslogng do stresstest

@HofiOne Thanks

@MrAnno
Copy link
Collaborator

MrAnno commented Mar 3, 2023

LGTM otherwise.

@kira-syslogng
Copy link
Contributor

Kira-stress-test: Build SUCCESS

bazsi
bazsi reviewed Mar 3, 2023
View reviewed changes
lib/cfg-grammar.y Outdated Show resolved Hide resolved
bazsi
bazsi reviewed Mar 3, 2023
View reviewed changes
lib/metrics-pipe.c
self->log_path_name = g_strdup(log_path_name);

log_pipe_add_info(&self->super, "metrics-pipe");
log_pipe_add_info(&self->super, self->log_path_name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should start specifying some kind of "type" qualifier to info, so that we could use the same mechanism for the debugger, but in a way that allows programmatic use.

Like use a key-value pair here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is more like a where should we go next kind of comment, not to be addressed here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree, we could add more detailed info for the LogPipes. Let's gather the use cases and make the LogPipe info functionality smarter.

@alltilla
cfg: make it possible to name a logpath
a3a1842 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
MrAnno
MrAnno reviewed Mar 6, 2023
View reviewed changes
lib/cfg-tree.c Outdated Show resolved Hide resolved
@MrAnno
Copy link
Collaborator

MrAnno commented Mar 6, 2023

This is my last comment, I promise. LGTM otherwise.

@alltilla
cfg: disallow setting logpaths with the same name
eb62334 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
metrics-pipe: add new LogPipe which counts ingress/egress messages
a869580 
The ingress counter is incremented once, when its queue() method is called,
the egress counter is increased once, when the message gets matched by at
least one of the following pipes.

Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
cfg-tree: inject metrics-pipes to named log paths
cd3a5da 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
light: add option to create named logpath
6113397 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
light: fix indentation of inner logpaths
9797a03 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
light: add support for syslog-ng-ctl stats prometheus
9735c33 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
light: add prometheus stats support for LogPath
d11efd4 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
light: add stats assert helper to logpath functional tests
9b33d33 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
light: add E2E tests for named logpaths without flags
56f17f2 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
light: add E2E tests for named logpaths with catchall flag
d7cf0b8 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
light: add E2E tests for named logpaths with fallback flag
e589306 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
light: add E2E tests for named logpaths with final flag
668b218 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@alltilla
news: add entry for syslog-ng#4342
e2c0fb1 
Signed-off-by: Attila Szakacs <szakacs.attila96@gmail.com>
@MrAnno MrAnno merged commit aafe466 into syslog-ng:master Mar 6, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bazsi bazsi bazsi left review comments

@MrAnno MrAnno MrAnno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
syslog-ng 4.1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@alltilla @kira-syslogng @MrAnno @HofiOne @bazsi