Feat/logging sampling statistics #74
Merged
Merged
Conversation
- 实现 SamplingFilter 类用于限制高频日志输出，支持线程安全的采样控制 - 添加 StatisticsAppender 类用于收集日志指标，包括总数量、错误率、级别分布等统计信息 - 提供时间窗口内的采样机制，可配置采样率和时间窗口参数 - 实现完整的统计报告生成功能，支持按级别和日志记录器分类展示 - 添加线程安全的数据结构确保并发环境下的数据一致性 - 提供统计重置和数据查询接口便于监控和调试
- 为SamplingFilter添加完整的单元测试覆盖采样率、时间窗口、线程安全等功能 - 为StatisticsAppender添加全面的单元测试验证统计计算、错误率跟踪、报告生成功能 - 测试各个日志级别和记录器的独立状态维护 - 验证多线程环境下的数据一致性 - 包含边界条件和异常情况的测试用例
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# |  |
Mar 5, 2026 8:03a.m. | Review ↗ |
| Secrets | |
Mar 5, 2026 8:03a.m. | Review ↗ |
Reviewer's Guide引入了一个线程安全的日志统计追加器，用于跟踪日志数量与错误率，并新增按 logger 维度的采样过滤器以限制高频日志，同时为这两个组件提供了完备的单元测试。 使用 SamplingFilter 和 StatisticsAppender 进行日志处理的时序图sequenceDiagram
participant Logger
participant SamplingFilter
participant StatisticsAppender
Logger->>SamplingFilter: ShouldLog(logEntry)
SamplingFilter->>SamplingFilter: GetOrAdd SamplingState for logger
SamplingFilter->>SamplingFilter: SamplingState.ShouldLog(sampleRate, timeWindow)
alt should log
SamplingFilter-->>Logger: true
Logger->>StatisticsAppender: Append(logEntry)
StatisticsAppender->>StatisticsAppender: Increment _totalCount
StatisticsAppender->>StatisticsAppender: Update _levelCounts
StatisticsAppender->>StatisticsAppender: Update _loggerCounts
alt level >= Error
StatisticsAppender->>StatisticsAppender: Increment _errorCount
end
else filtered out
SamplingFilter-->>Logger: false
end
新的日志采样与统计组件的类图classDiagram
direction LR
class LogEntry {
}
class LogLevel {
}
class ILogAppender {
<<interface>>
+void Append(LogEntry entry)
+void Flush()
}
class ILogFilter {
<<interface>>
+bool ShouldLog(LogEntry entry)
}
class StatisticsAppender {
-ConcurrentDictionary~LogLevel,long~ _levelCounts
-ConcurrentDictionary~string,long~ _loggerCounts
-long _errorCount
-long _totalCount
-DateTime _startTime
+long TotalCount
+long ErrorCount
+DateTime StartTime
+TimeSpan Uptime
+double ErrorRate
+void Append(LogEntry entry)
+void Flush()
+long GetCountByLevel(LogLevel level)
+long GetCountByLogger(string loggerName)
+IReadOnlyDictionary~LogLevel,long~ GetLevelCounts()
+IReadOnlyDictionary~string,long~ GetLoggerCounts()
+void Reset()
+string GenerateReport()
}
class SamplingFilter {
-int _sampleRate
-TimeSpan _timeWindow
-ConcurrentDictionary~string,SamplingState~ _samplingStates
+SamplingFilter(int sampleRate, TimeSpan timeWindow)
+bool ShouldLog(LogEntry entry)
}
class SamplingState {
-object _lock
-long _count
-DateTime _windowStart
+bool ShouldLog(int sampleRate, TimeSpan timeWindow)
}
ILogAppender <|.. StatisticsAppender
ILogFilter <|.. SamplingFilter
SamplingFilter o-- SamplingState
StatisticsAppender ..> LogEntry
StatisticsAppender ..> LogLevel
SamplingFilter ..> LogEntry
SamplingState ..> TimeSpan
SamplingState ..> DateTime
SamplingState.ShouldLog 判定逻辑的流程图flowchart TD
A_Start[Start ShouldLog] --> B_CheckWindow{now - _windowStart >= timeWindow?}
B_CheckWindow -- Yes --> C_ResetWindow[_windowStart = now\n_count = 0]
B_CheckWindow -- No --> D_KeepWindow[Keep current window]
C_ResetWindow --> E_Increment[_count++]
D_KeepWindow --> E_Increment
E_Increment --> F_Mod{_count % sampleRate == 1?}
F_Mod -- Yes --> G_ReturnTrue[Return true]
F_Mod -- No --> H_ReturnFalse[Return false]
G_ReturnTrue --> I_End[End]
H_ReturnFalse --> I_End[End]
文件级改动
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideIntroduces a thread-safe logging statistics appender that tracks counts and error rates, plus a per-logger sampling filter to limit high-frequency logs, along with comprehensive unit tests for both components. Sequence diagram for log processing with SamplingFilter and StatisticsAppendersequenceDiagram
participant Logger
participant SamplingFilter
participant StatisticsAppender
Logger->>SamplingFilter: ShouldLog(logEntry)
SamplingFilter->>SamplingFilter: GetOrAdd SamplingState for logger
SamplingFilter->>SamplingFilter: SamplingState.ShouldLog(sampleRate, timeWindow)
alt should log
SamplingFilter-->>Logger: true
Logger->>StatisticsAppender: Append(logEntry)
StatisticsAppender->>StatisticsAppender: Increment _totalCount
StatisticsAppender->>StatisticsAppender: Update _levelCounts
StatisticsAppender->>StatisticsAppender: Update _loggerCounts
alt level >= Error
StatisticsAppender->>StatisticsAppender: Increment _errorCount
end
else filtered out
SamplingFilter-->>Logger: false
end
Class diagram for new logging sampling and statistics componentsclassDiagram
direction LR
class LogEntry {
}
class LogLevel {
}
class ILogAppender {
<<interface>>
+void Append(LogEntry entry)
+void Flush()
}
class ILogFilter {
<<interface>>
+bool ShouldLog(LogEntry entry)
}
class StatisticsAppender {
-ConcurrentDictionary~LogLevel,long~ _levelCounts
-ConcurrentDictionary~string,long~ _loggerCounts
-long _errorCount
-long _totalCount
-DateTime _startTime
+long TotalCount
+long ErrorCount
+DateTime StartTime
+TimeSpan Uptime
+double ErrorRate
+void Append(LogEntry entry)
+void Flush()
+long GetCountByLevel(LogLevel level)
+long GetCountByLogger(string loggerName)
+IReadOnlyDictionary~LogLevel,long~ GetLevelCounts()
+IReadOnlyDictionary~string,long~ GetLoggerCounts()
+void Reset()
+string GenerateReport()
}
class SamplingFilter {
-int _sampleRate
-TimeSpan _timeWindow
-ConcurrentDictionary~string,SamplingState~ _samplingStates
+SamplingFilter(int sampleRate, TimeSpan timeWindow)
+bool ShouldLog(LogEntry entry)
}
class SamplingState {
-object _lock
-long _count
-DateTime _windowStart
+bool ShouldLog(int sampleRate, TimeSpan timeWindow)
}
ILogAppender <|.. StatisticsAppender
ILogFilter <|.. SamplingFilter
SamplingFilter o-- SamplingState
StatisticsAppender ..> LogEntry
StatisticsAppender ..> LogLevel
SamplingFilter ..> LogEntry
SamplingState ..> TimeSpan
SamplingState ..> DateTime
Flow diagram for SamplingState.ShouldLog decision logicflowchart TD
A_Start[Start ShouldLog] --> B_CheckWindow{now - _windowStart >= timeWindow?}
B_CheckWindow -- Yes --> C_ResetWindow[_windowStart = now\n_count = 0]
B_CheckWindow -- No --> D_KeepWindow[Keep current window]
C_ResetWindow --> E_Increment[_count++]
D_KeepWindow --> E_Increment
E_Increment --> F_Mod{_count % sampleRate == 1?}
F_Mod -- Yes --> G_ReturnTrue[Return true]
F_Mod -- No --> H_ReturnFalse[Return false]
G_ReturnTrue --> I_End[End]
H_ReturnFalse --> I_End[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些高层次的反馈：
StatisticsAppender和SamplingFilter的测试都依赖Thread.Sleep和DateTime.UtcNow来做对时间敏感的断言，这在环境较慢或资源竞争激烈时容易变得不稳定；建议将时间抽象到一个接口或可注入的时钟后面，这样在测试中可以以确定性的方式推进时间，而不是依赖睡眠。- 在
StatisticsAppender.Reset中，_startTime字段在没有任何同步的情况下被更新，而此时StartTime/Uptime可能在并发读取，这在 32 位运行时上可能导致“撕裂读”；建议将 ticks 存储在一个long中，并使用Interlocked或其他同步机制来实现原子更新。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- Both `StatisticsAppender` and `SamplingFilter` tests rely on `Thread.Sleep` and `DateTime.UtcNow` for timing-sensitive assertions, which can be flaky under slow or contended environments; consider abstracting time behind an interface or injectable clock so you can deterministically advance time in tests instead of sleeping.
- In `StatisticsAppender.Reset` the `_startTime` field is updated without any synchronization while being concurrently read by `StartTime`/`Uptime`, which can lead to torn reads on 32‑bit runtimes; consider storing ticks in a `long` and using `Interlocked` or another synchronization mechanism for atomic updates.
## Individual Comments
### Comment 1
<location path="GFramework.Core/logging/filters/SamplingFilter.cs" line_range="13-22" />
<code_context>
+ private readonly ConcurrentDictionary<string, SamplingState> _samplingStates = new();
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider how to prevent unbounded growth of per-logger sampling state.
Keying `_samplingStates` by `LoggerName` means each distinct name creates a `SamplingState` that is never reclaimed. In environments with dynamically generated logger names (per-tenant/request or with arbitrary tags), this can cause unbounded memory growth. If that’s a possibility, consider adding eviction or bounding logic (e.g., track last-used time and periodically prune, use an LRU policy, or fall back to a shared state once a maximum number of loggers is reached).
</issue_to_address>请帮我变得更有用！请对每条评论点击 👍 或 👎，我会根据这些反馈改进之后的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Both
StatisticsAppenderandSamplingFiltertests rely onThread.SleepandDateTime.UtcNowfor timing-sensitive assertions, which can be flaky under slow or contended environments; consider abstracting time behind an interface or injectable clock so you can deterministically advance time in tests instead of sleeping. - In
StatisticsAppender.Resetthe_startTimefield is updated without any synchronization while being concurrently read byStartTime/Uptime, which can lead to torn reads on 32‑bit runtimes; consider storing ticks in alongand usingInterlockedor another synchronization mechanism for atomic updates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Both `StatisticsAppender` and `SamplingFilter` tests rely on `Thread.Sleep` and `DateTime.UtcNow` for timing-sensitive assertions, which can be flaky under slow or contended environments; consider abstracting time behind an interface or injectable clock so you can deterministically advance time in tests instead of sleeping.
- In `StatisticsAppender.Reset` the `_startTime` field is updated without any synchronization while being concurrently read by `StartTime`/`Uptime`, which can lead to torn reads on 32‑bit runtimes; consider storing ticks in a `long` and using `Interlocked` or another synchronization mechanism for atomic updates.
## Individual Comments
### Comment 1
<location path="GFramework.Core/logging/filters/SamplingFilter.cs" line_range="13-22" />
<code_context>
+ private readonly ConcurrentDictionary<string, SamplingState> _samplingStates = new();
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider how to prevent unbounded growth of per-logger sampling state.
Keying `_samplingStates` by `LoggerName` means each distinct name creates a `SamplingState` that is never reclaimed. In environments with dynamically generated logger names (per-tenant/request or with arbitrary tags), this can cause unbounded memory growth. If that’s a possibility, consider adding eviction or bounding logic (e.g., track last-used time and periodically prune, use an LRU policy, or fall back to a shared state once a maximum number of loggers is reached).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 将ILogAppender接口继承IDisposable以支持资源释放 - 添加ITimeProvider和SystemTimeProvider接口及实现类 - 创建FakeTimeProvider用于测试时间控制 - 修改SamplingFilter支持时间提供者注入和最大日志记录器数量限制 - 为SamplingFilter添加过期状态清理功能 - 修改StatisticsAppender使用时间提供者并实现IDisposable - 更新相关单元测试以使用FakeTimeProvider进行精确时间控制 - 在测试类中为模拟追加器添加Dispose方法实现
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
添加日志统计附加器和采样过滤器，以支持日志指标采集和限频日志记录。
新特性：
StatisticsAppender，用于跟踪日志计数、错误指标、运行时间，并生成文本报告。SamplingFilter，根据可配置的采样率和时间窗口，以每个 logger 为粒度对日志进行采样。测试：
StatisticsAppender的计数、错误率、运行时间、重置行为、报告生成、空状态、并发性以及刷新行为。SamplingFilter的采样行为、时间窗口重置、按 logger 区分状态、参数校验以及并发性。Original summary in English
Summary by Sourcery
Add logging statistics appender and sampling filter to support log metric collection and rate-limited logging.
New Features:
Tests: