Skip to content

feat(logger): add CtxLogger interface for OpenTelemetry trace integration#3195

Open
MrSibe wants to merge 16 commits intoapache:developfrom
MrSibe:feat/ctxlogger-interface
Open

feat(logger): add CtxLogger interface for OpenTelemetry trace integration#3195
MrSibe wants to merge 16 commits intoapache:developfrom
MrSibe:feat/ctxlogger-interface

Conversation

@MrSibe
Copy link
Contributor

@MrSibe MrSibe commented Feb 4, 2026

Summary

Implements context-aware logging with automatic OpenTelemetry trace correlation for distributed tracing.

Related issue: #3182

What's New

  • CtxLogger interface: Extends Logger with context-aware methods (CtxInfof, CtxErrorf, etc.)
  • Automatic trace injection: Logs automatically include trace_id, span_id, trace_flags from context
  • Dual logger support: Works with both Zap and Logrus
  • Optional error recording: Can record error logs to OpenTelemetry spans
  • Backward compatible: Existing code works without changes
  • Opt-in: Disabled by default, enabled via configuration

Configuration

dubbo:
  logger:
    driver: zap
    level: info
    format: json
    trace-integration:
      enabled: true
      record-error-to-span: true

Add context-aware logging support to integrate with OpenTelemetry traces:

- Add CtxLogger interface extending Logger with context-aware methods
- Create trace_extractor.go to extract traceId, spanId, and trace_flags from context
- Add configuration constants for trace integration (enabled, record-error-to-span)

This is the foundation for issue apache#3182 - providing ctxLogger interface
to correlate logs with distributed traces.
Implement context-aware logging adapter for Zap:

- Add ZapCtxLogger that wraps DubboLogger
- Automatically inject trace_id, span_id, trace_flags from context
- Support optional error recording to span
- Integrate with existing Zap instantiation logic
Implement context-aware logging adapter for Logrus:

- Add LogrusCtxLogger that wraps DubboLogger
- Automatically inject trace_id, span_id, trace_flags from context
- Support optional error recording to span
- Integrate with existing Logrus instantiation logic
Extend LoggerConfig to support trace integration:

- Add TraceIntegrationConfig struct with enabled and record-error-to-span options
- Update toURL() method to pass trace integration parameters
- Enable users to configure trace logging via YAML
Add comprehensive unit tests:

- trace_extractor_test.go: test trace field extraction from context
- zap/ctx_logger_test.go: test Zap adapter with trace injection
- logrus/ctx_logger_test.go: test Logrus adapter with trace injection
- Fix import error in logrus/ctx_logger.go (move errors import to top)

All tests pass successfully, verifying:
- Trace fields are correctly extracted from valid span context
- Logs include trace_id, span_id, trace_flags when context has trace
- Logs work normally without trace context (backward compatible)
@MrSibe MrSibe force-pushed the feat/ctxlogger-interface branch from 6337c06 to 7d57f4f Compare February 4, 2026 09:08
@codecov-commenter
Copy link

codecov-commenter commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 73.93162% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.09%. Comparing base (60d1c2a) to head (60cf5ab).
⚠️ Report is 734 commits behind head on develop.

Files with missing lines Patch % Lines
logger/core/logrus/ctx_logger.go 68.11% 11 Missing and 11 partials ⚠️
logger/core/zap/ctx_logger.go 68.11% 11 Missing and 11 partials ⚠️
config/logger_config.go 53.33% 5 Missing and 2 partials ⚠️
logger/trace_extractor.go 73.33% 2 Missing and 2 partials ⚠️
logger/core/logrus/logrus.go 50.00% 2 Missing and 1 partial ⚠️
logger/core/zap/zap.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3195      +/-   ##
===========================================
+ Coverage    46.76%   48.09%   +1.32%     
===========================================
  Files          295      466     +171     
  Lines        17172    33959   +16787     
===========================================
+ Hits          8031    16333    +8302     
- Misses        8287    16296    +8009     
- Partials       854     1330     +476     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Alanxtl Alanxtl linked an issue Feb 4, 2026 that may be closed by this pull request
2 tasks
@Alanxtl Alanxtl added ✏️ Feature 3.3.2 version 3.3.2 labels Feb 4, 2026
@Rinai-R
Copy link
Contributor

Rinai-R commented Feb 4, 2026

合并之后可以在samples里面加个示例

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements context-aware logging with automatic OpenTelemetry trace correlation to enable distributed tracing support in Dubbo-Go. The implementation adds a new CtxLogger interface that extends the base Logger interface with context-aware methods, allowing logs to automatically include trace_id, span_id, and trace_flags from the OpenTelemetry context.

Changes:

  • Added CtxLogger interface with context-aware logging methods (CtxDebug, CtxInfo, CtxWarn, CtxError and their formatted variants)
  • Implemented trace extraction utilities to extract OpenTelemetry trace information from context
  • Provided implementations for both Zap and Logrus logging frameworks with optional error recording to spans
  • Added configuration support for trace integration via YAML with opt-in enablement

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
logger/base.go Added CtxLogger interface extending Logger with context-aware methods
logger/trace_extractor.go Implemented trace field extraction from OpenTelemetry context
logger/trace_extractor_test.go Comprehensive tests for trace extraction and validation logic
logger/core/zap/ctx_logger.go Zap implementation of CtxLogger with trace field injection and optional span error recording
logger/core/zap/ctx_logger_test.go Tests for Zap context logger covering various logging scenarios
logger/core/zap/zap.go Integration of CtxLogger into Zap logger instantiation based on configuration
logger/core/logrus/ctx_logger.go Logrus implementation of CtxLogger with trace field injection and optional span error recording
logger/core/logrus/ctx_logger_test.go Tests for Logrus context logger covering various logging scenarios
logger/core/logrus/logrus.go Integration of CtxLogger into Logrus logger instantiation based on configuration
config/logger_config.go Added TraceIntegrationConfig structure and configuration mapping to URL parameters
common/constant/key.go Added configuration keys for trace integration settings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add comprehensive unit tests for recordErrorToSpan functionality in both Zap and Logrus CtxLogger implementations
- Test coverage includes: enabled/disabled states, span status verification, and error event recording
- Fix error creation inconsistency in Zap implementation to use errors.New instead of fmt.Errorf for consistency with Logrus
Fix testifylint errors by replacing assert.Len(t, events, 0) with assert.Empty(t, events) for better readability.
Copy link
Contributor

@Alanxtl Alanxtl left a comment

Choose a reason for hiding this comment

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

  1. 这个新的api兼容new api吗
  2. dubbo-website 里面写一下文档,介绍一下使用方式
  3. 写 samples

@MrSibe
Copy link
Contributor Author

MrSibe commented Feb 5, 2026

  1. 这个新的api兼容new api吗
  2. dubbo-website 里面写一下文档,介绍一下使用方式
  3. 写 samples

目前仅支持 Old API, 我把 New API 的加上

Add TraceIntegration support to global.LoggerConfig to enable
trace integration features in New API.
@MrSibe MrSibe requested a review from Alanxtl February 5, 2026 09:06
Copy link
Contributor

@Alanxtl Alanxtl left a comment

Choose a reason for hiding this comment

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

现在还是没有new api的使用接口呀

Add WithTraceIntegration and WithRecordErrorToSpan options to enable CtxLogger configuration through New API.
Use idiomatic Go pattern for creating pointer copies.
Add SetTraceIntegrationEnabled() and SetRecordErrorToSpan() methods to
LoggerConfigBuilder to support trace integration configuration via the
Old API (Builder pattern).
@MrSibe
Copy link
Contributor Author

MrSibe commented Feb 6, 2026

Usage

Old API

config.NewLoggerConfigBuilder().
    SetDriver("zap").
    SetTraceIntegrationEnabled(true).
    SetRecordErrorToSpan(true).
    Build()

New API

logger.NewOptions(
    logger.WithZap(),
    logger.WithTraceIntegration(true),
    logger.WithRecordErrorToSpan(true),
)

YAML config

dubbo:
  logger:
    driver: zap
    trace-integration:
      enabled: true
      record-error-to-span: true

Logging with CtxLogger

// get logger
log := logger.GetLogger().(logger.CtxLogger)

// log with context
log.CtxInfo(ctx, "message")
log.CtxInfof(ctx, "user: %s", name)
log.CtxError(ctx, "error")
log.CtxErrorf(ctx, "failed: %v", err)

@MrSibe MrSibe requested a review from Alanxtl February 6, 2026 11:16
Copy link
Contributor

@Alanxtl Alanxtl left a comment

Choose a reason for hiding this comment

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

cool work lgtm
don't forget to write doc in dubbo-website

@sonarqubecloud
Copy link

return lcb
}

func (lcb *LoggerConfigBuilder) SetTraceIntegrationEnabled(enabled bool) *LoggerConfigBuilder {
Copy link
Contributor

Choose a reason for hiding this comment

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

封装这么一个函数,在哪些地方调用它了呢?

Copy link
Contributor

@AlexStocks AlexStocks left a comment

Choose a reason for hiding this comment

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

整体可合:接口、配置、old/new API 兼容和测试覆盖都完整。
建议后续补一条文档约束:CtxLogger 的使用前提是启用 trace integration,避免业务方直接断言类型时出现使用偏差。

traceEnabled := config.GetParamBool(constant.LoggerTraceEnabledKey, false)
if traceEnabled {
recordErrorToSpan := config.GetParamBool(constant.LoggerTraceRecordErrorKey, true)
return NewZapCtxLogger(baseLogger, recordErrorToSpan), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

ZapCtxLogger 没有实现 OpsLogger 接口(SetLoggerLevel(string) bool)。logger.go 中的类型断言 if l, ok := logger.(OpsLogger); ok 会失败,开启 trace 后动态调整日志级别的功能静默失效,没有任何报错。

建议:ZapCtxLogger 实现 SetLoggerLevel,委托给内部的 DubboLogger

return logrusLogger.WithFields(logrus.Fields{
"trace_id": fields.TraceID,
"span_id": fields.SpanID,
"trace_flags": fields.TraceFlags,
Copy link
Contributor

Choose a reason for hiding this comment

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

无 trace 信息时返回 logrus.NewEntry(logrusLogger)(非 nil),所有调用方的 if entry != nil 判断都走进 entry 路径,fallback 逻辑永远不触发。和 Zap 实现语义不一致(Zap 无 trace 时直接返回原始 logger)。

类型断言失败(!ok)时返回 nil,反而触发 fallback,与无 trace 时行为相反,逻辑混乱。

建议:无 trace 时改为 return nil,与 Zap 保持一致。

Comment on lines +64 to +65
return zapLogger
}
Copy link
Contributor

Choose a reason for hiding this comment

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

l.Logger.(*zap.SugaredLogger) 类型断言失败时静默返回 nil,所有 trace 字段注入被跳过,没有任何 warning,问题只在运行时暴露,排查困难。构造函数 NewZapCtxLogger 里也没有做类型校验。

建议:类型断言失败时至少 log 一次 warn;或者在构造函数里提前做校验,直接 panic/return error。

if span == nil || !span.IsRecording() {
return
}

Copy link
Contributor

Choose a reason for hiding this comment

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

errors.New(msg) 把日志消息字符串包装成新 error,span event 里的 exception.type 变成 *errors.errorString,丢失了原始错误类型。在 Jaeger/Zipkin 等系统里无法按错误类型查询和聚合。

args 里可能本身就包含原始 error 对象,格式化后再 errors.New 是降级处理。

建议:新增 CtxErrorWithErr(ctx, err, template, args...) 变体,让调用方传入原始 error 供 span recording 使用。

// Add trace integration parameters if configured
if l.TraceIntegration != nil {
if l.TraceIntegration.Enabled != nil {
url.AddParam(constant.LoggerTraceEnabledKey, strconv.FormatBool(*l.TraceIntegration.Enabled))
Copy link
Contributor

Choose a reason for hiding this comment

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

原有参数统一在 NewURL 时通过 common.WithParamsValue 传入,新增的 trace 参数改用 url.AddParam() 追加,风格不统一。

建议:改用 common.WithParamsValueNewURL 调用中统一传入 trace 参数。

Comment on lines +129 to +138

if l.recordErrorToSpan {
l.recordErrorToSpanIfPresent(ctx, template, args...)
}
}

// CtxError logs error and optionally records to span.
func (l *ZapCtxLogger) CtxError(ctx context.Context, args ...any) {
if ctxLogger := l.withTraceFields(ctx); ctxLogger != nil {
ctxLogger.Error(args...)
Copy link
Contributor

Choose a reason for hiding this comment

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

ctxLogger.Errorf(template, args...)recordErrorToSpanIfPresent 内部的 fmt.Sprintf(template, args...) 对同一 template+args 做了两次格式化。如果 args 包含实现了 fmt.Stringer 且有状态的对象,可能产生非预期行为。

建议:先 msg := fmt.Sprintf(template, args...),然后把 msg 分别传给 log 和 span,避免重复格式化。

Comment on lines +50 to +60
type CtxLogger interface {
Logger
CtxDebug(ctx context.Context, args ...any)
CtxDebugf(ctx context.Context, template string, args ...any)
CtxInfo(ctx context.Context, args ...any)
CtxInfof(ctx context.Context, template string, args ...any)
CtxWarn(ctx context.Context, args ...any)
CtxWarnf(ctx context.Context, template string, args ...any)
CtxError(ctx context.Context, args ...any)
CtxErrorf(ctx context.Context, template string, args ...any)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Logger 基接口包含 Fatal/Fatalf,但 CtxLogger 没有对应的 CtxFatal/CtxFatalf,接口层面不对称。

建议:补充这两个方法;或者在注释里明确说明 fatal 场景故意排除在外(如进程退出,span 无法完整提交)。

Comment on lines +154 to +161
if span == nil || !span.IsRecording() {
return
}

msg := fmt.Sprintf(template, args...)
span.SetStatus(codes.Error, msg)
span.RecordError(errors.New(msg))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ZapCtxLoggerLogrusCtxLoggerrecordErrorToSpanIfPresent、所有 CtxXxx 方法结构完全相同,只有底层 logger 类型不同。后续修改 span recording 逻辑时需要同步改两处,容易产生不一致。

建议:将 recordErrorToSpanIfPresent、span 状态设置等公共逻辑提取到 logger 包的共享函数中。

@MrSibe
Copy link
Contributor Author

MrSibe commented Mar 7, 2026

收到,下周统一解决👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] 提供 ctxLogger 接口

6 participants