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

解决日志系统未初始化完成时,apollo 的加载日志没法输出问题 #3677

Merged
merged 29 commits into from
May 23, 2021

Conversation

klboke
Copy link
Contributor

@klboke klboke commented May 11, 2021

What's the purpose of this PR

解决问题:

解决 spring boot 方式集成时,配置 apollo.bootstrap.eagerLoad.enabled 情况下,apollo 的加载日志没法输出问题。

设计原理:

封装一个基于 guava-cache 的日志输出内容存储容器 DeferredLogUtil,将 apollo 初始化时的日志信息,添加到延迟输出的日志容器。在日志系统加载完成也就是 EnvironmentPostProcessor 事件立刻回放,最终实现的效果和不开启 apollo.bootstrap.eagerLoad.enabled 是一样的,设计过程参考了 spring-boot#DeferredLog

@codecov-commenter
Copy link

codecov-commenter commented May 11, 2021

Codecov Report

Merging #3677 (4eecb14) into master (f41a73b) will decrease coverage by 0.17%.
The diff coverage is 44.19%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3677      +/-   ##
============================================
- Coverage     50.99%   50.82%   -0.18%     
- Complexity     2333     2373      +40     
============================================
  Files           449      452       +3     
  Lines         14033    14279     +246     
  Branches       1426     1431       +5     
============================================
+ Hits           7156     7257     +101     
- Misses         6385     6517     +132     
- Partials        492      505      +13     
Impacted Files Coverage Δ Complexity Δ
...ip/framework/apollo/core/utils/DeferredLogger.java 24.53% <24.53%> (ø) 21.00 <21.00> (?)
...rk/apollo/internals/DefaultMetaServerProvider.java 57.89% <66.66%> (+2.33%) 5.00 <1.00> (ø)
.../ctrip/framework/apollo/core/MetaDomainConsts.java 80.95% <66.66%> (-0.53%) 18.00 <1.00> (ø)
...tion/internals/provider/DefaultServerProvider.java 65.51% <71.42%> (+0.40%) 20.00 <1.00> (ø)
.../framework/apollo/core/utils/DeferredLogCache.java 71.83% <71.83%> (ø) 14.00 <14.00> (?)
...framework/apollo/internals/AbstractConfigFile.java 85.96% <100.00%> (ø) 11.00 <1.00> (ø)
...trip/framework/apollo/internals/DefaultConfig.java 82.97% <100.00%> (ø) 29.00 <1.00> (ø)
...rk/apollo/internals/LocalFileConfigRepository.java 77.30% <100.00%> (ø) 27.00 <1.00> (ø)
...ework/apollo/internals/RemoteConfigRepository.java 89.37% <100.00%> (+0.62%) 25.00 <1.00> (+1.00)
...ring/boot/ApolloApplicationContextInitializer.java 93.87% <100.00%> (+0.26%) 15.00 <0.00> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f41a73b...4eecb14. Read the comment docs.

@github-actions
Copy link

github-actions bot commented May 15, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@nobodyiam
Copy link
Member

Here is my 2 cents, what do you think?

  1. we keep the original logger interface but with a different implementation, e.g. a composite logger
  2. we add a deferred flag to control the logger logic
  3. in normal cases, the deferred flag is always false, so the log behavior is the same as the old one
  4. in eager load case, we first set the deferred flag to true to cache the logs and after logging system is initialized, we set the flag to false and flush the cached logs

image

@nobodyiam
Copy link
Member

@klboke please fix the failed tests

image

Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

LGTM

@nobodyiam nobodyiam merged commit dc41c93 into apolloconfig:master May 23, 2021
@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2021
@nobodyiam nobodyiam added this to the 1.9.0 milestone May 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants