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

[ISSUEhttps://github.com/alibaba/nacos/issues/6999]Managed log framework #228

Merged
merged 3 commits into from
Mar 14, 2022

Conversation

hujun-w-2
Copy link
Collaborator

@hujun-w-2
Copy link
Collaborator Author

nacos托管日志框架方案.pdf
采用方案2开发,目前测试可以正常托管logback和log4j2 xml配置文件,并且当logback和log4j2配置文件配置了自动刷新属性,日志配置能正常实现自动刷新

@hujun-w-2
Copy link
Collaborator Author

hujun-w-2 commented Feb 9, 2022

测试前提:
springboot默认使用logback,测试log4j2需要正常引入相关依赖
测试流程:
1.在控制台添加logback.xml或者log4j2.xml
2.启动demo工程
3.来回修改logback日志级别
4.观察日志demo工程日志级别是否变化
测试使用配置:
config.zip

@hujun-w-2
Copy link
Collaborator Author

@realJackSun @KomachiSion @chuntaojun 能否帮忙review一下

@realJackSun
Copy link
Member

Looks good.


}

private void writeLogFile(String content, String dataId) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nacos client will cache config in cache dir, why cache agqin?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nacos client will cache config in cache dir, why cache agqin?
The purpose of writing the configuration content into the file here is to facilitate the log framework to read from the directory, because the snapshot file path of the Nacos client is not exposed. If the dead path written on the springboot side points to the Nacos client, problems will occur later when the Nacos client is changed.

}
}

public Properties buildGlobalNacosProperties() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

These properties whether has built in other Configuration class?

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'll optimize it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The modification has been completed

@KomachiSion KomachiSion merged commit a8d6a80 into nacos-group:master Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants