Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

add sofa-serverless-adapter-logback #305

Merged
merged 9 commits into from
Dec 11, 2023

Conversation

chenlei3641
Copy link
Contributor

[### Motivation

Explain the context, and why you're making that change.
To make others understand what is the problem you're trying to solve.

Modification

Describe the idea and modifications you've done.

Result

Resolved or fixed #.

If there is no issue then describe the changes introduced by this PR.
](#274)

Copy link

sofastack-cla bot commented Nov 16, 2023

Hi @chenlei3641, welcome to SOFAStack community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

chenlei3641 and others added 2 commits November 16, 2023 15:16
# Conflicts:
#	sofa-serverless-runtime/sofa-serverless-adapter-ext/pom.xml
@chenlei3641
Copy link
Contributor Author

@lvjing2 有空看下,后面怎么弄

@lvjing2
Copy link
Collaborator

lvjing2 commented Nov 22, 2023

@lvjing2 有空看下,后面怎么弄

@yuanyuancin 这几天会看下,另外方便加下开发者沟通群吗?这是个比较重要的 adapter,希望能再群里能更方便的沟通。

Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d60cb2d) 82.88% compared to head (c84423e) 82.35%.
Report is 22 commits behind head on master.

❗ Current head c84423e differs from pull request most recent head a0b51a2. Consider uploading reports for the commit a0b51a2 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #305      +/-   ##
============================================
- Coverage     82.88%   82.35%   -0.54%     
- Complexity       66      131      +65     
============================================
  Files            40       38       -2     
  Lines          1005      918      -87     
  Branches         91       77      -14     
============================================
- Hits            833      756      -77     
+ Misses          119      115       -4     
+ Partials         53       47       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yuanyuancin
Copy link
Collaborator

这个feature的核心是为了解决serverless合并部署场景下,多biz使用logback、支持biz独立的日志配置、上下文隔离;

需要增加sample验证一下方案可行性(可以参考log4j2 sample搭建在工程sample目录下)验证过程中可能需要考虑下:

  1. 方案中的 Adapter 使用 ThreadLocal 缓存不同 biz 不同 logger context可能不可靠,模块安装不一定会单独线程,模块卸载时的线程也不会复用安装时的线程。

  2. 如果如预期在启动期为不同 biz 创建了不同 logger context,并使用biz自己的日志配置configure context,需要机制来确保在getLogger时,根据不同biz获取不同logger context,否则启动期创建的logger context可能不会生效。

@sofastack-cla sofastack-cla bot added size/XXL and removed size/XL labels Nov 29, 2023
Copy link
Collaborator

@yuanyuancin yuanyuancin left a comment

Choose a reason for hiding this comment

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

  1. Adapter的能力合并到 ContextSelector就行啦,不需要两个类
  2. 不只是biz启动期会 getContext,运行期,需要考虑运行期如果tccl非biz classloader的情况,考虑通过 callerClass classloader?
  3. ContextSelector.detachLoggerContext 实现remove逻辑
  4. context name可以使用 String contextName = Integer.toHexString(System.identityHashCode(bizClassloader));

@yuanyuancin
Copy link
Collaborator

LGTM

@yuanyuancin yuanyuancin merged commit 5819eba into sofastack:master Dec 11, 2023
0 of 3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants