Skip to content

adopt structured logging from dapr/kit - #4525

Open
JoshVanL wants to merge 1 commit into
dapr:mainfrom
JoshVanL:structured-logging
Open

adopt structured logging from dapr/kit#4525
JoshVanL wants to merge 1 commit into
dapr:mainfrom
JoshVanL:structured-logging

Conversation

@JoshVanL

Copy link
Copy Markdown
Contributor

dapr/kit's logger is now backed by log/slog (dapr/kit#165) and exposes a structured API, *logger.Log, alongside the existing printf-style Logger interface. Log output is byte-identical to the previous logrus backend, so nothing here changes what components emit; the tests that capture and assert on log text pass unchanged.

Component constructors keep their func NewX(logger.Logger) signature, which is public API for out-of-tree component authors. A component reaches the structured API without changing that signature by wrapping the logger it receives:

    func NewRedis(l logger.Logger) state.Store {
        return &redis{log: logger.FromLogger(l)}
    }

This change converts the shared third-party log adapters, which duplicated what slog now provides:

  • common/component/kafka: SaramaLogBridge renders sarama's pre-formatted strings through the structured logger, output unchanged.
  • middleware/http/sentinel: sentinel already passes a message plus alternating keys and values, which previously got flattened into one string via logging.AssembleMsg; the pairs now pass straight through as attributes. This also fixes the adaptor logging sentinel warnings and errors at info level.
  • middleware/http/wasm: level checks go through the structured logger; guest messages and the "wasm stdout:"/"wasm stderr:" pipe lines keep their exact wording, which the e2e tests assert on.
  • common/component/cloudflare/workers: worker deploy logging carries the URL as an attribute and errors via logger.Err.

bindings/dubbo gains a compile-time assertion that logger.Logger still satisfies dubbo-go's Logger interface: dubbo consumes the printf methods structurally, so a change to either interface now fails in this repo rather than surfacing as a confusing downstream break.

Lint configuration:

  • staticcheck SA1019 findings for the deprecated printf logging methods are excluded while the ~877 existing call sites are converted incrementally; the rule is scoped to exactly those deprecation messages and is deleted when the migration completes.
  • sloglint enforces the shared conventions from kit: snake_case keys, no mixed key-value pairs and attributes, and the reserved log schema field names (time, level, msg, scope, type, instance, ver) are forbidden as per-call attr

dapr/kit's logger is now backed by log/slog (dapr/kit#165) and exposes a
structured API, *logger.Log, alongside the existing printf-style Logger
interface. Log output is byte-identical to the previous logrus backend,
so nothing here changes what components emit; the tests that capture and
assert on log text pass unchanged.

Component constructors keep their func NewX(logger.Logger) signature,
which is public API for out-of-tree component authors. A component
reaches the structured API without changing that signature by wrapping
the logger it receives:

```go
    func NewRedis(l logger.Logger) state.Store {
        return &redis{log: logger.FromLogger(l)}
    }
```

This change converts the shared third-party log adapters, which
duplicated what slog now provides:

* common/component/kafka: SaramaLogBridge renders sarama's pre-formatted
  strings through the structured logger, output unchanged.
* middleware/http/sentinel: sentinel already passes a message plus
  alternating keys and values, which previously got flattened into one
  string via logging.AssembleMsg; the pairs now pass straight through as
  attributes. This also fixes the adaptor logging sentinel warnings and
  errors at info level.
* middleware/http/wasm: level checks go through the structured logger;
  guest messages and the "wasm stdout:"/"wasm stderr:" pipe lines keep
  their exact wording, which the e2e tests assert on.
* common/component/cloudflare/workers: worker deploy logging carries the
  URL as an attribute and errors via logger.Err.

bindings/dubbo gains a compile-time assertion that logger.Logger still
satisfies dubbo-go's Logger interface: dubbo consumes the printf methods
structurally, so a change to either interface now fails in this repo
rather than surfacing as a confusing downstream break.

Lint configuration:

* staticcheck SA1019 findings for the deprecated printf logging methods
  are excluded while the ~877 existing call sites are converted
  incrementally; the rule is scoped to exactly those deprecation
  messages and is deleted when the migration completes.
* sloglint enforces the shared conventions from kit: snake_case keys,
  no mixed key-value pairs and attributes, and the reserved log schema
  field names (time, level, msg, scope, type, instance, ver) are
  forbidden as per-call attr

Signed-off-by: joshvanl <me@joshvanl.dev>
@JoshVanL
JoshVanL requested a balanced review from Copilot August 13, 2026 19:19
@JoshVanL
JoshVanL requested review from a team as code owners August 13, 2026 19:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adopts dapr/kit’s structured logging API across shared adapters while preserving public constructor signatures.

Changes:

  • Migrates Kafka, Sentinel, WASM, and Cloudflare logging.
  • Adds Dubbo logger interface compatibility assertion.
  • Configures structured-logging lint rules and temporary deprecation exclusions.

Reviewed changes

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

Show a summary per file
File Description
.golangci.yml Adds migration and sloglint rules.
bindings/dubbo/dubbo_output.go Adds logger compatibility assertion.
common/component/cloudflare/workers/workers.go Uses structured worker logs.
common/component/kafka/kafka.go Installs the structured Sarama bridge.
common/component/kafka/sarama_log_bridge.go Migrates Sarama logging.
go.mod Replaces dapr/kit with a fork.
go.sum Updates dependency checksums.
middleware/http/sentinel/logger.go Migrates Sentinel’s logger adapter.
middleware/http/sentinel/middleware.go Uses the new adapter constructor.
middleware/http/wasm/httpwasm.go Migrates WASM logging and level checks.
middleware/http/wasm/httpwasm_test.go Updates structured logger fixtures.
Suppressed comments (1)

middleware/http/sentinel/logger.go:64

  • keysAndValues is a key/value sequence, but appending logger.Err(err) adds an attribute to that same argument list. This violates the no-mixed-args convention enabled in .golangci.yml:79, and slice expansion prevents sloglint from detecting it. Append the shared error key and value instead, or convert every input pair to attributes.
	args := make([]any, 0, len(keysAndValues)+1)
	args = append(args, keysAndValues...)

	if err != nil {
		args = append(args, logger.Err(err))

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread go.mod
// Don't commit with this uncommented!
//
// replace github.com/dapr/kit => ../kit
replace github.com/dapr/kit => github.com/joshvanl/kit v0.0.0-20260813182616-fd5d465c8baa
Comment on lines 35 to +36
func (l *loggerAdaptor) Debug(msg string, keysAndValues ...interface{}) {
s := logging.AssembleMsg(logging.GlobalCallerDepth, "DEBUG", msg, nil, keysAndValues...)
l.logger.Debug(s)
l.log.Debug(msg, keysAndValues...)
Comment on lines 33 to +34
func (b SaramaLogBridge) Print(v ...interface{}) {
b.daprLogger.Debug(v...)
b.log.Debug(fmt.Sprint(v...))
Comment on lines 138 to +142
if stdout := rh.stdout.String(); len(stdout) > 0 {
rh.logger.Debugf("wasm stdout: %s", stdout)
rh.log.Debug("wasm stdout: " + stdout)
}
if stderr := rh.stderr.String(); len(stderr) > 0 {
rh.logger.Debugf("wasm stderr: %s", stderr)
rh.log.Debug("wasm stderr: " + stderr)
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.64103% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.21%. Comparing base (cc03682) to head (c92415b).
⚠️ Report is 37 commits behind head on main.

Files with missing lines Patch % Lines
middleware/http/wasm/httpwasm.go 26.66% 11 Missing ⚠️
common/component/cloudflare/workers/workers.go 0.00% 7 Missing ⚠️
common/component/kafka/sarama_log_bridge.go 0.00% 5 Missing ⚠️
middleware/http/sentinel/logger.go 50.00% 5 Missing ⚠️
common/component/kafka/kafka.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4525      +/-   ##
==========================================
+ Coverage   31.94%   32.21%   +0.27%     
==========================================
  Files         353      352       -1     
  Lines       47723    37896    -9827     
==========================================
- Hits        15243    12207    -3036     
+ Misses      31277    24484    -6793     
- Partials     1203     1205       +2     

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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