add black list to aviod take too much efforts to translate in file log watcher #961

smileusd · 2024-09-29T10:03:58Z

This pr is to add black list in file log watcher to skip the translat which take too much overhead in cpu when log flush outburst #960

k8s-ci-robot · 2024-09-29T10:04:08Z

Hi @smileusd. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

wangzhen127 · 2024-10-02T06:03:27Z

Do you always know what pattern to skip in the log? Can you provide a sample config file with the blacklist for your target use case?

/ok-to-test

hakman · 2024-10-02T09:45:40Z

pkg/systemlogmonitor/logwatchers/filelog/log_watcher.go

+	if s.cfg.BlackList == nil || len(s.cfg.BlackList) == 0 {
+		return false
+	}


I don't think this adds much value. Range should be able to handle nil or empty slices.

Suggested change

if s.cfg.BlackList == nil || len(s.cfg.BlackList) == 0 {

return false

}

hakman · 2024-10-02T09:48:59Z

pkg/systemlogmonitor/logwatchers/types/log_watcher.go

+	// Skip the black list by simple filter to avoid running more efforts in regex
+	BlackList []string `json:"blackList,omitempty"`


Suggested change

// Skip the black list by simple filter to avoid running more efforts in regex

BlackList []string `json:"blackList,omitempty"`

// Skip the log lines containing any of the strings in the list to avoid running unnecessary regex.

SkipList []string `json:"skipList,omitempty"`

smileusd · 2024-10-09T11:45:04Z

Do you always know what pattern to skip in the log? Can you provide a sample config file with the blacklist for your target use case?

/ok-to-test

In our cluster, the users deploy the audit beat pod and record the log with all files changed in /var/log/messages. So we skip log simply contains the "audit:" and "audit[" .

root@~# tail  -n 10000 /var/log/messages |grep audit |wc -l 
9554

There is 95% useless logs in NPD impact the performance

smileusd · 2024-10-10T01:49:30Z

/test pull-npd-e2e-test

hakman · 2024-10-11T09:04:45Z

/lgtm
/assign @wangzhen127

wangzhen127 · 2024-10-14T15:40:40Z

pkg/systemlogmonitor/logwatchers/filelog/log_watcher.go

@@ -116,6 +116,9 @@ func (s *filelogWatcher) watchLoop() {
 		}
 		line = buffer.String()
 		buffer.Reset()
+		if s.filterSkipList(line) {
+			continue


Can you add some test for this?

wangzhen127 · 2024-10-14T15:41:46Z

pkg/systemlogmonitor/logwatchers/types/log_watcher.go

@@ -36,6 +36,8 @@ type WatcherConfig struct {
 	// PluginConfig is a key/value configuration of a plugin. Valid configurations
 	// are defined in different log watcher plugin.
 	PluginConfig map[string]string `json:"pluginConfig,omitempty"`
+	// Skip the log lines containing any of the strings in the list to avoid running unnecessary regex.
+	SkipList []string `json:"skipList,omitempty"`


Can you add a sample config file in https://github.com/kubernetes/node-problem-detector/tree/master/config to illustrate how to use this?

wangzhen127

Can you also squash the commits?

wangzhen127 · 2024-10-15T21:52:25Z

config/docker-monitor-filelog.json

@@ -9,6 +9,7 @@
 	"lookback": "5m",
 	"bufferSize": 10,
 	"source": "docker-monitor",
+	"skipList": [],


No need to add empty list in existing files. We need to make sure when it is missing, those files still work for backward compatibility.

wangzhen127 · 2024-10-15T21:54:57Z

config/kernel-monitor-filelog.json

@@ -9,6 +9,7 @@
 	"lookback": "5m",
 	"bufferSize": 10,
 	"source": "kernel-monitor",
+	"skipList": [ " audit:", " audit[" ],


Can you remove from this file and add a new json file with this?

wangzhen127 · 2024-10-15T22:55:13Z

BTW, we plan to make a new release this week. If you can get the PR ready in a day or two, we can include it in the upcoming release. Otherwise, it can wait for the next release.

/retest

… watcher

wangzhen127 · 2024-10-16T15:07:18Z

/lgtm

k8s-ci-robot · 2024-10-16T15:07:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman, smileusd, wangzhen127

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [wangzhen127]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 29, 2024

k8s-ci-robot requested review from derekwaynecarr and hakman September 29, 2024 10:04

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 29, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 2, 2024

hakman suggested changes Oct 2, 2024

View reviewed changes

smileusd force-pushed the upstream_add_black_list_in_log_watcher branch from c7b9f37 to 2b5c836 Compare October 9, 2024 11:55

wangzhen127 mentioned this pull request Oct 9, 2024

pull-npd-e2e-test failing ssh handshake #970

Open

k8s-ci-robot assigned wangzhen127 and hakman Oct 11, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 11, 2024

wangzhen127 reviewed Oct 14, 2024

View reviewed changes

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 15, 2024

wangzhen127 reviewed Oct 15, 2024

View reviewed changes

add skip list to aviod take too much efforts to translate in file log…

3a386a6

… watcher

smileusd force-pushed the upstream_add_black_list_in_log_watcher branch from 81d34d2 to 3a386a6 Compare October 16, 2024 02:57

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 16, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 16, 2024

k8s-ci-robot merged commit 0f4d8b9 into kubernetes:master Oct 16, 2024
11 checks passed

smileusd mentioned this pull request Nov 26, 2024

REQUEST: New membership for smileusd kubernetes/org#5274

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add black list to aviod take too much efforts to translate in file log watcher #961

add black list to aviod take too much efforts to translate in file log watcher #961

smileusd commented Sep 29, 2024

k8s-ci-robot commented Sep 29, 2024

wangzhen127 commented Oct 2, 2024

hakman Oct 2, 2024

smileusd Oct 9, 2024

hakman Oct 2, 2024

smileusd Oct 9, 2024

smileusd commented Oct 9, 2024 •

edited

Loading

smileusd commented Oct 10, 2024

hakman commented Oct 11, 2024

wangzhen127 Oct 14, 2024

smileusd Oct 15, 2024

wangzhen127 Oct 14, 2024

smileusd Oct 15, 2024

wangzhen127 left a comment

wangzhen127 Oct 15, 2024

smileusd Oct 16, 2024

wangzhen127 Oct 15, 2024

smileusd Oct 16, 2024

wangzhen127 commented Oct 15, 2024

wangzhen127 commented Oct 16, 2024

k8s-ci-robot commented Oct 16, 2024

	if s.cfg.BlackList == nil \|\| len(s.cfg.BlackList) == 0 {
	return false
	}

		// Skip the black list by simple filter to avoid running more efforts in regex
		BlackList []string `json:"blackList,omitempty"`

add black list to aviod take too much efforts to translate in file log watcher #961

add black list to aviod take too much efforts to translate in file log watcher #961

Conversation

smileusd commented Sep 29, 2024

k8s-ci-robot commented Sep 29, 2024

wangzhen127 commented Oct 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smileusd commented Oct 9, 2024 • edited Loading

smileusd commented Oct 10, 2024

hakman commented Oct 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangzhen127 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangzhen127 commented Oct 15, 2024

wangzhen127 commented Oct 16, 2024

k8s-ci-robot commented Oct 16, 2024

smileusd commented Oct 9, 2024 •

edited

Loading