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

Execution logger #83

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open

Conversation

1579923417
Copy link

@1579923417 1579923417 commented Mar 1, 2025

这个 PR 尝试优化项目中的日志管理功能,提升日志的可读性和管理效率。主要修改如下:

  1. 新增log.ts文件,统一管理项目日志,基于loglevel日志库构建开发日志类LOG,提供debuginfowarnerror等日志方法。

  2. 替换项目中console.loglog.ts中的logger.infologger.warn等方法,便于集中管理日志。

验证此 PR 时,请检查日志是否正确输出到控制台和文件中,以及日志信息是否清晰、完整。

This PR attempts to optimize the project's logging management capabilities and improve log readability and management efficiency.The main changes are as follows:

  1. Added a newlog.tsfile to unify the project's logging management.It uses thetsloglibrary to build a development logging classLOG,providing methods likedebug,info,warn,anderror.

  2. Replaced allconsole.loginstances in the project with methods fromlogger.ts,such aslog.infoandlogger.warn,to centralize log management.

To verify this PR,please check whether logs are correctly output to the console and file,and ensure that the log information is clear and complete.

Copy link
Collaborator

@HairlessVillager HairlessVillager left a comment

Choose a reason for hiding this comment

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

另外别开新 PR 了,就在这个 PR 里改。我不熟悉你的业务和用到的技术(比如tslog),如果你觉得我的 comment 不对你可以反驳。

@HairlessVillager
Copy link
Collaborator

另外是谁教你这么写 PR description 的?假设你是新来的开发者,你看到这个 PR description 会作何感想?

@HairlessVillager
Copy link
Collaborator

另外是谁教你这么写 PR description 的?假设你是新来的开发者,你看到这个 PR description 会作何感想?

这里确实言过了,你在 PR ready for review 之前改一下 description 就行。

-The method of replacing all console logging instances with log.ts (log.info, log.warn....)
-Log messages will be truncated when they exceed 2048 characters
-Fix the issue of improper use of log levels in the project
-Introduce the 'rotating-file-stream' library, use the 'mask' method to preprocess log parameters (ARGs), and call the 'toReadableString' method to format the parameters
-Simplify timestamp generation code in 'log. ts' and optimize log file naming logic
Copy link
Collaborator

@HairlessVillager HairlessVillager left a comment

Choose a reason for hiding this comment

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

不错,不过“记录日志到 console 和临时文件”的功能看起来还没实现。

@1579923417
Copy link
Author

不错,不过“记录日志到 console 和临时文件”的功能看起来还没实现。

tslog 库会默认将日志记录到控制台,“临时文件”功能 没有实现

- update logger implementation to use 'loglevel' instead of 'tslog'
- implement the getAllLogs method to return stored logs in array format.
- enhance the logging method factory by storing truncated logs in logStorage.
bug: the toReadableString method does not handle nested objects well and cannot achieve the expected effect.
private logger: Logger<ILogObj>;
private logFilePath: string;
private logger: log.Logger;
private logStorage: string[] = [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

潜在的性能瓶颈?当大量日志涌入时,这里的性能会不会过慢而拖慢整个工作流的后腿?问一下 AI。

Copy link
Author

@1579923417 1579923417 Mar 4, 2025

Choose a reason for hiding this comment

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

在高并发或大量日志的情况下,这段代码确实会存在性能瓶颈。

1. 性能瓶颈分析

(1)日志存储(logStorage

logStorage 是一个数组,用于存储所有日志内容。随着日志数量的增加,logStorage 的大小会不断增长,这可能导致以下问题:

  • 内存占用:大量日志会导致内存占用快速上升,尤其是在长时间运行的应用中。
  • 数组操作性能:每次调用 logStorage.push() 时,数组会动态扩展,这在 JavaScript 中可能涉及内存重新分配。当数组非常大时,频繁的内存分配和扩展会显著降低性能。
  • 垃圾回收压力:大量的日志数据可能导致频繁的垃圾回收,进一步影响应用性能。

(2)日志内容截断和序列化

toReadableString() 方法中,对日志内容进行了截断和序列化:

  • JSON 序列化JSON.stringify() 是一个相对较慢的操作,尤其是对于复杂对象。在高并发场景下,大量调用 JSON.stringify() 可能会拖慢性能。
  • 并且这里toReadableString的方法不能很好地处理嵌套对象,无法达到预期的效果(没有进行截断),目前修改了几版代码还是无法解决这个问题。

2. 优化建议

限制日志存储大小

  • 固定大小的存储:可以将 logStorage 设计为一个固定大小的存储结构,例如使用环形缓冲区(FIFO 队列)。当存储满时,自动丢弃最早的日志。
  • 日志采样:在高并发场景下,可以对日志进行采样,只记录部分日志,而不是所有日志。

Copy link
Collaborator

Choose a reason for hiding this comment

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

我让你问 AI 不是因为我懒得问,是因为我想让你参考 AI 的回复来决定是否修改这里的实现……

@HairlessVillager
Copy link
Collaborator

这里toReadableString的方法不能很好地处理嵌套对象,无法达到预期的效果(没有进行截断),目前修改了几版代码还是无法解决这个问题。

这个问题先挂起,等你把其他功能实现之后我来测试一下

@1579923417 1579923417 marked this pull request as ready for review March 6, 2025 03:02
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.

2 participants