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

新增 csp.sentinel.log.level 启动配置项,可设置为OFF,进行关闭日志 #2514

Merged
merged 3 commits into from
Jan 14, 2022
Merged

Conversation

zhaoxinhu
Copy link
Contributor

对应issues:Add record log level configuration support | 支持配置 record 日志的输出级别配置 #2505
通过添加启动配置项 csp.sentinel.log.level=OFF,可以选择性的关闭产生的日志

@CLAassistant
Copy link

CLAassistant commented Dec 28, 2021

CLA assistant check
All committers have signed the CLA.

@sczyh30 sczyh30 added the area/logging Issues or PRs related to logging of Sentinel label Dec 29, 2021
@zhaoxinhu
Copy link
Contributor Author

第一次贡献源代码,这个pr是有问题合并不了?还是需要审批流程?

@@ -80,7 +82,9 @@ private static void startMetricTimerListener() {
SentinelConfig.METRIC_FLUSH_INTERVAL);
return;
}
SCHEDULER.scheduleAtFixedRate(new MetricTimerListener(), 0, flushInterval, TimeUnit.SECONDS);
if (!LogBase.getLogLevel().equals(Level.OFF)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

metric的写日志逻辑实际已经通过csp.sentinel.metric.flush.interval参数进行控制了,这个日志打印我觉得更应该在日志层面调用去控制,而不是通过日志级别来控制metricTimerListener的启用。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,明白了,我调整一下

@sczyh30 sczyh30 self-assigned this Jan 10, 2022
@sczyh30 sczyh30 added this to the 1.8.4 milestone Jan 10, 2022
@@ -32,6 +32,7 @@
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
Copy link
Member

Choose a reason for hiding this comment

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

Could you please remove the unused import?

// load log level
String logLevelString = properties.getProperty(LOG_LEVEL);
if (logLevelString != null && (logLevelString = logLevelString.trim()).length() > 0) {
logLevel = Level.parse(logLevelString);
Copy link
Member

Choose a reason for hiding this comment

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

What if the logLevel is invalid?

@sczyh30 sczyh30 added the wait-for-feedback Issues that need more feedback label Jan 12, 2022
@sczyh30
Copy link
Member

sczyh30 commented Jan 13, 2022

Could you please sign the CLA here: https://cla-assistant.io/alibaba/Sentinel?pullRequest=2514

@sczyh30 sczyh30 removed the wait-for-feedback Issues that need more feedback label Jan 13, 2022
@sczyh30
Copy link
Member

sczyh30 commented Jan 13, 2022

cc @brotherlu-xcq Any other suggestions?

@brotherlu-xcq
Copy link
Collaborator

cc @brotherlu-xcq Any other suggestions?

I have no more suggestions. LGTM

Copy link
Member

@sczyh30 sczyh30 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@brotherlu-xcq brotherlu-xcq left a comment

Choose a reason for hiding this comment

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

LGTM

@sczyh30 sczyh30 merged commit 794530f into alibaba:master Jan 14, 2022
@sczyh30
Copy link
Member

sczyh30 commented Jan 14, 2022

Thanks for contributing!

Zhang-0952 pushed a commit to Zhang-0952/Sentinel that referenced this pull request Mar 4, 2022
* Add `csp.sentinel.log.level` property to support logging level for record logs (not for block log and metric log)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/logging Issues or PRs related to logging of Sentinel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants