Release v1.3.7#85
Merged
Merged
Conversation
There was a problem hiding this comment.
Hey - 我在这里给出了一些高层面的反馈:
- 建议捕获并至少记录
viewKey和hllKey上Expire调用的错误,这样设置 TTL 失败的情况就不会被忽略。 - 由于
time.Now().Format("20060102")是在收集器循环内部计算的,如果某个逻辑事件跨越了午夜,同一个事件可能会被划分到不同的日期桶中;建议在事件入队时就计算日期,并通过 channel 传递该日期。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- Consider capturing and at least logging errors from the `Expire` calls on both `viewKey` and `hllKey` so that failures to set TTLs don’t go unnoticed.
- Because `time.Now().Format("20060102")` is evaluated inside the collector loop, the same logical event could be bucketed into different dates if it straddles midnight; consider computing the date when the event is enqueued and passing it through the channel instead.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈来改进评审质量。
Original comment in English
Hey - I've left some high level feedback:
- Consider capturing and at least logging errors from the
Expirecalls on bothviewKeyandhllKeyso that failures to set TTLs don’t go unnoticed. - Because
time.Now().Format("20060102")is evaluated inside the collector loop, the same logical event could be bucketed into different dates if it straddles midnight; consider computing the date when the event is enqueued and passing it through the channel instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider capturing and at least logging errors from the `Expire` calls on both `viewKey` and `hllKey` so that failures to set TTLs don’t go unnoticed.
- Because `time.Now().Format("20060102")` is evaluated inside the collector loop, the same logical event could be bucketed into different dates if it straddles midnight; consider computing the date when the event is enqueued and passing it through the channel instead.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR aims to improve request/DAU counting accuracy and safety by deduplicating per-resource daily request counts via Redis HyperLogLog and by cloning request-derived strings before sending them to background collectors.
Changes:
- Clone IP/rid/version strings before sending them across goroutines to avoid unsafe aliasing with request-scoped buffers.
- Add a per-resource-per-day HyperLogLog (
hll:resource:<rid>:<date>) to only increment the daily ranking once per unique IP. - Add expirations for the new per-resource HLL keys (and adjust collector flow accordingly).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/middleware/dau_record.go | Clone c.IP() before sending to the DAU collector goroutine. |
| internal/interfaces/rest/handler/version.go | Use per-resource daily HLL to gate ranking increments; clone tuple fields before enqueuing; add expirations for the new HLL keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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
使用 Redis HyperLogLog 和更安全的请求元数据处理方式,更准确地统计唯一资源请求以及日活用户数。
Bug 修复:
功能增强:
Original summary in English
Summary by Sourcery
Track unique resource requests and daily active users more accurately using Redis HyperLogLog and safer request metadata handling.
Bug Fixes:
Enhancements: