feat: Refactor the log library of milvus, support scoped context logging#47610
Open
chyezh wants to merge 8 commits intomilvus-io:masterfrom
Open
feat: Refactor the log library of milvus, support scoped context logging#47610chyezh wants to merge 8 commits intomilvus-io:masterfrom
chyezh wants to merge 8 commits intomilvus-io:masterfrom
Conversation
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: chyezh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
|
@chyezh Thanks for your contribution. Please submit with DCO, see the contributing guide https://github.com/milvus-io/milvus/blob/master/CONTRIBUTING.md#developer-certificate-of-origin-dco. |
Contributor
|
@chyezh This is a feature PR ( How to resolve: Design documents location: https://github.com/milvus-io/milvus-design-docs/tree/main/design_docs |
mergify
bot
added
do-not-merge/missing-design-doc
kind/feature
Contributor
|
[ci-v2-notice] To rerun ci-v2 checks, comment with:
If you have any questions or requests, please contact @zhikunyao. |
mergify
bot
added
dco-passed
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
1 similar comment
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Codecov Report❌ Patch coverage is
❌ Your project check has failed because the head coverage (76.86%) is below the target coverage (77.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #47610 +/- ##
==========================================
- Coverage 76.98% 76.86% -0.13%
==========================================
Files 2057 2062 +5
Lines 337572 336832 -740
==========================================
- Hits 259894 258906 -988
- Misses 69566 69760 +194
- Partials 8112 8166 +54
🚀 New features to boost your workflow:
|
sre-ci-robot
added
low-code-coverage
Implement the mlog package - a context-aware logging library built on zap, designed specifically for Milvus distributed systems. Key Features: - Mandatory context passing for all logging operations - Zero-overhead abstraction using type aliases - Automatic field accumulation through call chain - Cross-service field propagation via gRPC metadata - Lazy encoding with WithLazy for deferred field encoding - Component-level Logger with field optimization - OpenTelemetry TraceID/SpanID extraction Package Structure: - pkg/mlog/logger.go: Global logging functions and component-level Logger - pkg/mlog/context.go: Context field management and caching - pkg/mlog/field.go: Field constructors (type aliases to zap) - pkg/mlog/field_enum.go: Well-known keys and FieldXXX helpers - pkg/mlog/level.go: Dynamic log level management - pkg/mlog/grpc/interceptor.go: gRPC interceptors for field propagation Test Coverage: 100% (115 tests) Related Issue: milvus-io#35917 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: chyezh <chyezh@outlook.com>
Add RatedDebug/Info/Warn/Error and RatedLog functions with per-call-site rate limiting using lazy-initialized token-bucket limiters. Suppressed entries are tracked and reported via _ignored field on next allowed log. Also expose Logger.log as Logger.Log for public use. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: chyezh <chyezh@outlook.com>
Replace intermediate log()/logWithLogger()/ratedLog() with prepareLog() that returns logger+fields before the zap call. Each public function now directly calls the zap method, ensuring exactly 1 frame between user code and zap's caller capture. AddCallerSkip(1) now correctly reports the user's call site for all 14 API paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: chyezh <chyezh@outlook.com>
Add comprehensive benchmark suite comparing mlog vs native zap.Logger across all API paths (package-level, Logger methods, rated functions). Update README with benchmark tables, rate-limited API docs, and key performance takeaways. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: chyezh <chyezh@outlook.com>
- Add rated.go and Log() to architecture diagram - Fix lowercase `log` to uppercase `Log` in performance example - Add missing Log() to Logger Methods table - Add Uintptrp/Uintptrs to field constructors table - Add GetAtomicLevel() to API reference Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: chyezh <chyezh@outlook.com>
…, and merge grpc interceptors - Add DPanic, Panic, Fatal log level functions (package-level and Logger methods) - Add FieldOption/OptPropagated() for opt-in RPC field propagation on well-known fields - Add well-known field keys: index_id, field_id, task_id, broadcast_id, job_id, build_id - Move gRPC interceptors from pkg/mlog/grpc/ into pkg/mlog/ package - Privatize propagatedStringField/propagatedInt64Field and all key constants Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: chyezh <chyezh@outlook.com>
…gent guide
- Fix prepareLog to call getLogContext once instead of twice, removing 2
unnecessary heap allocations per log call (MlogInfo: 2 allocs → 0,
MlogInfoWithFields: 3 allocs → 1)
- Replace per-call &logContext{} with shared emptyLogContext sentinel
- Remove unused loggerFromContext function
- Add LevelEnabled() global function and (*Logger).LevelEnabled() method
for guarding expensive field construction on hot paths
- Remove caller-related benchmarks (ZapInfoWithCaller, ZapInfoWithCallerAndFields)
- Update README.md with new benchmark data and LevelEnabled API docs
- Add README_AGENT.md as a concise AI Agent logging guide
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: chyezh <chyezh@outlook.com>
Contributor
|
@chyezh go-sdk check failed, comment |
sre-ci-robot
added
low-code-coverage
- Replace nil context with context.Background() in all tests - Replace "errors" import with "github.com/cockroachdb/errors" (depguard) - Fix import grouping order for gci linter - Remove nil-context benchmark (BenchmarkMlogInfoNilContext) - Update stale comment referencing removed loggerFromContext function Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: chyezh <chyezh@outlook.com>
Contributor
|
@chyezh go-sdk check failed, comment |
sre-ci-robot
added
low-code-coverage
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
2 participants
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.
issue: #35917
design doc: milvus-io/milvus-design-docs#14
Summary
This PR implements the mlog package - a context-aware logging library built on
https://github.com/uber-go/zap, designed specifically for Milvus distributed systems.
Key Features
direct zap usage
contexts inherit parent fields
is disabled
fields to minimize runtime encoding