Skip to content

Migrate HTTP request logging from go-kit/log to slog - #39729

Merged
getvictor merged 6 commits into
mainfrom
victor/38889-slog-http
Feb 14, 2026
Merged

Migrate HTTP request logging from go-kit/log to slog#39729
getvictor merged 6 commits into
mainfrom
victor/38889-slog-http

Conversation

@getvictor

@getvictor getvictor commented Feb 11, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #38889

Checklist for submitter

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually

Summary by CodeRabbit

  • Refactor
    • Updated internal logging infrastructure to improve standardization and maintainability. Logging functionality remains unchanged from an end-user perspective.

@getvictor

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Feb 11, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@coderabbitai

coderabbitai Bot commented Feb 11, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This pull request implements a large-scale migration of the Fleet server codebase from go-kit/log to Go's standard library slog. The changes update logging middleware and handler signatures to use *slog.Logger instead of kitlog.Logger, replace go-kit logging level constants with slog equivalents, migrate from key-value pair logging to slog's structured attribute-based API, and update test utilities to work with the new logging backend. All changes are internal refactoring with no modifications to public APIs or externally-facing functionality.

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 4
❌ Failed checks (3 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR migrates HTTP logging but does not fully meet issue #38889 requirements: Phase 1 required creating server/platform/logging package with backward-compatible format (ts key, RFC3339 timestamps, lowercase levels), integrating into initLogger(), and preserving level.Info() patterns. The PR only updates existing call sites to pass slog.Logger instead of kitlog.Logger, which is Phase 2 work. Ensure the server/platform/logging package exists with proper backward-compatible formatting, update initLogger() integration as required by Phase 1, and verify existing level.Info() patterns still work as documented in the ADR.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning PR description is incomplete. It covers testing and changes files but omits multiple required checklist sections including input validation, endpoint compatibility, database migrations, and fleetd compatibility verification. Complete the PR description by adding the missing checklist sections. For each section, either check the box and explain the verification performed, or clearly state why it doesn't apply to this change.
Out of Scope Changes check ❓ Inconclusive Many changes update logging call sites to use slog directly (e.g., slog.LevelInfo, *slog.Logger signatures), which appears to be Phase 2 work explicitly marked out-of-scope in issue #38889 Phase 1. However, without full context of whether these changes support a necessary Phase 1 infrastructure, the scope cannot be definitively assessed. Clarify whether updating existing code to use slog directly (Phase 2) was intentionally included as part of this PR, or if these changes should be deferred per the stated issue scope.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: migrating HTTP request logging from go-kit/log to slog, which matches the primary objective of the PR.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch victor/38889-slog-http

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.

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

This PR advances the slog migration by switching HTTP request/end-of-request logging and the contexts/logging request logging context from go-kit/log patterns to log/slog, while keeping existing middleware and handlers working via the adapter.

Changes:

  • Updated contexts/logging.LoggingContext to log via *slog.Logger and switched WithLevel to accept slog.Level.
  • Migrated request logging middleware/hooks (LogRequestEnd, LogResponseEndMiddleware, NewLoggingMiddleware) and key call sites to use logger.SlogLogger().
  • Updated affected tests and a few handlers (debug handler, enroll endpoints, SCIM/conditional access) to pass slog loggers.

Reviewed changes

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

Show a summary per file
File Description
server/contexts/logging/logging.go Switch request logging context to emit via *slog.Logger and slog.Level.
server/contexts/logging/logging_test.go Update tests to validate slog records via a test handler instead of parsing logfmt output.
server/service/middleware/log/log.go Update request logging middleware APIs to accept *slog.Logger.
server/service/handler.go Pass logger.SlogLogger() into LogRequestEnd for go-kit transport ServerAfter.
server/service/debug_handler.go Convert debug handler logging to slog (ErrorContext, slog logger param).
cmd/fleet/serve.go Pass logger.SlogLogger() to MakeDebugHandler.
server/service/testing_utils.go Pass logger.SlogLogger() to MakeDebugHandler in test server setup.
server/service/sessions.go Replace request-level WithLevel(..., level.Info) with slog.LevelInfo.
server/service/osquery.go Replace request-level WithLevel(..., level.Info) with slog.LevelInfo in enroll path.
server/service/orbit.go Replace request-level WithLevel(..., level.Info) with slog.LevelInfo in enroll path.
server/service/microsoft_mdm.go Replace request-level WithLevel(..., level.Info) with slog.LevelInfo for SOAP fault logging.
ee/server/service/mdm.go Replace request-level WithLevel(..., level.Info) with slog.LevelInfo for MDM SSO endpoints.
ee/server/service/condaccess/idp.go Switch conditional access IdP raw-handler logging middleware to slog.
ee/server/scim/scim.go Switch SCIM response-end logging middleware to slog.
server/service/osquery_test.go Update request logging test to call lCtx.Log(ctx, logger.SlogLogger()).
server/service/endpoint_utils_test.go Update tests to use a slog logger for LogRequestEnd.
server/mdm/android/tests/testing_utils.go Use platformlogging.NewLogfmtLogger and update request-end logging hook to slog.

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

Comment thread server/contexts/logging/logging.go
Comment thread server/service/debug_handler.go Outdated
@codecov

codecov Bot commented Feb 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.67925% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.27%. Comparing base (0ff707a) to head (a5a2dd9).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
server/contexts/logging/logging.go 92.85% 1 Missing and 1 partial ⚠️
server/service/debug_handler.go 33.33% 2 Missing ⚠️
cmd/fleet/serve.go 0.00% 1 Missing ⚠️
server/service/sessions.go 83.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #39729   +/-   ##
=======================================
  Coverage   66.26%   66.27%           
=======================================
  Files        2439     2439           
  Lines      195437   195409   -28     
  Branches     8550     8528   -22     
=======================================
- Hits       129512   129506    -6     
+ Misses      54197    54174   -23     
- Partials    11728    11729    +1     
Flag Coverage Δ
backend 68.06% <88.67%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@getvictor
getvictor marked this pull request as ready for review February 12, 2026 16:57
@getvictor
getvictor requested a review from a team as a code owner February 12, 2026 16:57
ksykulev
ksykulev previously approved these changes Feb 14, 2026
# Conflicts:
#	server/mdm/android/tests/testing_utils.go
@getvictor
getvictor merged commit de55ecf into main Feb 14, 2026
69 of 71 checks passed
@getvictor
getvictor deleted the victor/38889-slog-http branch February 14, 2026 19:04
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.

slog migration (1)

3 participants