Skip to content

chore: simplify, modernize (Go 1.26), update deps#87

Merged
rustatian merged 3 commits into
masterfrom
chore/cleanup-modernize-deps
Jun 1, 2026
Merged

chore: simplify, modernize (Go 1.26), update deps#87
rustatian merged 3 commits into
masterfrom
chore/cleanup-modernize-deps

Conversation

@rustatian
Copy link
Copy Markdown
Member

@rustatian rustatian commented May 29, 2026

Applied fixes

R (bug/correctness) — 1

  • rpc.go Log/LogWithContext: ensure each write to stderr ends with \n; without it consecutive messages run together on the same line.

S (simplify) — 2

  • rpc.go: replace parallel format/formatRaw implementations with a shared buildAttrs helper; eliminate the separate format function.
  • rpc.go formatRaw: replace hand-rolled comma-join loop with strings.Join.

M (modern Go 1.26) — 2

  • doc.go: fix stale doc comment that said "zap logger" — the plugin uses log/slog.
  • rpc.go: replace []any accumulator + variadic Error/Info/Warn/Debug with []slog.Attr + LogAttrs; propagate caller context via ErrorContext/InfoContext/WarnContext/DebugContext.

Deps

go get -u all && go mod tidy in root and tests/; go work sync. No version changes detected (already current).

Summary by CodeRabbit

  • Chores
    • Updated logging infrastructure to use Go's structured slog logger with enhanced context support for RPC operations, enabling better request tracing and debugging capabilities.
    • Improved log output formatting to ensure consistent newline-termination across all logging methods.

- doc.go: fix stale reference to "zap logger" (uses log/slog)
- rpc.go: use LogAttrs+[]slog.Attr instead of []any accumulator; propagate
  ctx through ErrorContext/InfoContext/WarnContext/DebugContext; replace
  parallel format/formatRaw impls with shared buildAttrs helper; use
  strings.Join for comma-join in formatRaw; ensure Log/LogWithContext
  append a trailing newline so lines do not run together on stderr
- rpc_test.go: update formatRaw/LogWithContext assertions for new \n suffix
Copilot AI review requested due to automatic review settings May 29, 2026 12:20
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ff68cb72-556d-4f0c-9c79-d74d05d56629

📥 Commits

Reviewing files that changed from the base of the PR and between eacb5bd and 81daa0a.

📒 Files selected for processing (3)
  • doc.go
  • rpc.go
  • rpc_test.go

📝 Walkthrough

Walkthrough

This PR migrates the app-logger RPC service from context-less logging to Go's log/slog with context support, and enforces newline-normalized log output. Package documentation, service methods, formatting helpers, and tests are all updated to reflect the new behavior.

Changes

slog migration and newline normalization

Layer / File(s) Summary
Package documentation update
doc.go
Package-level documentation now references Go's log/slog logger instead of zap.
RPC logging methods and structured attribute conversion
rpc.go
Error, Info, Warning, Debug and their *WithContext variants now call log.*Context(ctx, message) and log.LogAttrs(ctx, level, message, attrs...) using the provided context. New buildAttrs() helper converts protobuf LogAttrs into typed slog.Attr values, and new ensureNewline() helper enforces exactly-one-trailing-newline output.
Raw format output newline normalization
rpc.go
formatRaw zero-attributes path now uses ensureNewline to guarantee output ends with exactly one trailing newline.
Test expectations for newline-terminated output
rpc_test.go
TestFormatRaw expects all outputs (including nil/empty args, single/multiple attrs, and special characters) to include trailing newline. TestRPCLogWithContext expects stderr log output to end with newline.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • roadrunner-server/app-logger#78: Both PRs modify the applogger RPC implementation in rpc.go, refactoring the Error/Info/Warning/Debug/Log handler signatures and updating the stderr/log formatting helpers (including formatRaw/newline handling and structured attrs) to match the new logging/connect behavior.

Suggested labels

enhancement

Poem

🐰 From zap to slog, the context flows,
Each log now newline-blessed it shows,
Structured attrs in harmony dance,
While trailing newlines ensure a chance!
The rabbit hops through logging's delight,
Making each message properly formatted bright. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: simplify, modernize (Go 1.26), update deps' is partially related to the changeset. It mentions simplifications, modernization for Go 1.26, and dependency updates, but does not highlight the main correctness bug fix (ensuring newline termination in Log/LogWithContext to prevent messages running together).
Description check ✅ Passed The PR description provides detailed explanations of changes (bug fixes, simplifications, modernizations) and dependency updates. However, the PR Checklist section has empty checkboxes with TODO comments, indicating the author did not complete the required pre-submission verification steps.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/cleanup-modernize-deps

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

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 modernizes the app-logger RPC implementation around log/slog, simplifies attribute formatting, and adjusts raw stderr logging so emitted messages are newline-terminated.

Changes:

  • Swaps leveled logger calls to context-aware slog APIs and LogAttrs.
  • Replaces []any attribute formatting with typed []slog.Attr.
  • Updates raw stderr formatting/tests and fixes stale package documentation.

Reviewed changes

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

File Description
rpc.go Modernizes slog calls, refactors attribute helpers, and changes stderr newline handling.
rpc_test.go Updates raw formatting expectations for newline-terminated output.
doc.go Updates package documentation from zap to log/slog.

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

Comment thread rpc.go Outdated
Comment thread rpc.go Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (eacb5bd) to head (81daa0a).

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #87   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           57        62    +5     
=========================================
+ Hits            57        62    +5     

☔ 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.

rustatian added 2 commits May 29, 2026 23:38
- LogWithContext: the no-attrs path now appends a trailing newline only
  when missing (via shared ensureNewline helper), matching Log. Avoids a
  blank line for already newline-terminated messages, e.g.
  LogWithContext("hello\n", nil).
- rpc.go: reword the formatRaw comment that wrongly claimed it uses
  buildAttrs (it does not; buildAttrs yields slog.Attr, not key:value text).
- rpc_test.go: cover the newline-idempotent no-args case.
776f503 replaced the strings.Builder (added in 2d880ab) with a
pairs []string + strings.Join form under a "simplify" label,
regressing formatRaw from ~1 allocation to N+3 per raw LogWithContext
write. Restore the single-buffer Builder; output (one trailing newline,
':'/',' separators) and TestFormatRaw are unchanged.
@rustatian rustatian self-assigned this Jun 1, 2026
@rustatian rustatian merged commit 4e6e9c2 into master Jun 1, 2026
9 checks passed
@rustatian rustatian deleted the chore/cleanup-modernize-deps branch June 1, 2026 18:40
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