Migrate platform/endpointer and platform/middleware/ratelimit to slog. by getvictor · Pull Request #39928 · fleetdm/fleet

getvictor · 2026-02-16T17:56:29Z

Related issue: Resolves #38889

Also delete the unnecessary logging.DiscardHandler and replace it with slog.DiscardHandler

Checklist for submitter

If some of the following don't apply, delete the relevant line.

Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
- Changes file already present from previous PR

Testing

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

Summary by CodeRabbit

Chores
- Modernized internal logging infrastructure by transitioning from a custom logging implementation to the standard library's structured logging approach across multiple services and middleware components.
Refactor
- Updated logging method signatures and error handling paths to use standardized logging interfaces consistently throughout the platform codebase.

getvictor · 2026-02-16T17:56:48Z

@coderabbitai full review

coderabbitai · 2026-02-16T17:56:55Z

✅ Actions performed

Full review triggered.

coderabbitai · 2026-02-16T18:02:46Z

Walkthrough

This PR migrates logging infrastructure across multiple server components from go-kit/log to Go's standard slog library. It replaces the custom logging.DiscardHandler with slog.DiscardHandler, updates logger type signatures from kitlog.Logger to *slog.Logger, converts logging calls from go-kit/log patterns (like level.Warn(logger).Log(...)) to slog patterns (like logger.WarnContext(...)), and removes the custom DiscardHandler implementation from the logging package in favor of the standard library equivalent. Behavioral changes to error handling paths and rate limiting logic are not introduced.

Possibly related PRs

#38890: Foundational work that establishes the slog-based logging package in server/platform/logging with kitlog adapter support.
#38751: Implements concrete code-level migrations from go-kit/log to slog across multiple service components and test utilities.
#39527: Continues slog/adapter logging work by modifying logger types and handlers in the platform logging package and related components.

🚥 Pre-merge checks | ✅ 3 | ❌ 3

❌ Failed checks (3 warnings)

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.
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (10 files): ⚔️ `orbit/pkg/execuser/execuser_linux.go` (content) ⚔️ `orbit/pkg/execuser/execuser_linux_test.go` (content) ⚔️ `server/mdm/android/service/profiles_test.go` (content) ⚔️ `server/mdm/android/tests/testing_utils.go` (content) ⚔️ `server/platform/endpointer/endpoint_utils.go` (content) ⚔️ `server/platform/logging/logging.go` (content) ⚔️ `server/platform/middleware/ratelimit/ratelimit.go` (content) ⚔️ `server/service/endpoint_utils_test.go` (content) ⚔️ `server/service/handler.go` (content) ⚔️ `server/service/integration_android_certificate_templates_test.go` (content) These conflicts must be resolved before merging into `main`.	Resolve conflicts locally and push changes to this branch.
Description check	⚠️ Warning	PR description is incomplete and lacks several required template sections and details needed for proper review and merge.	Complete the PR description with required sections: explicitly address database migrations, input validation, SQL injection prevention, endpoint compatibility, automated tests details, configuration settings, and fleetd/orbit/Fleet Desktop compatibility checks. Provide context on how the slog migration relates to issue `#38889` objectives.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: migrating platform/endpointer and platform/middleware/ratelimit to slog, which matches the primary objectives of the changeset.
Linked Issues check	✅ Passed	The PR implements Phase 1 changes from issue `#38889` by migrating logging to slog in endpointer and ratelimit, removing custom DiscardHandler, and updating error/warning logging calls [`#38889`].
Out of Scope Changes check	✅ Passed	All changes align with the Phase 1 scope: migration of platform/endpointer and platform/middleware/ratelimit to slog, removal of logging.DiscardHandler, and related test updates [`#38889`].

_{✏️ 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-platform

⚔️ Resolve merge conflicts (beta)

Auto-commit resolved conflicts to branch victor/38889-slog-platform
Create stacked PR with resolved conflicts
Post resolved changes as copyable diffs in a comment

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

Pull request overview

Migrates Fleet’s platform endpointer error logging and the IP-based rate limit error middleware from go-kit/log to log/slog, while removing a custom discard handler in favor of the stdlib discard handler.

Changes:

Switch platform/endpointer.ErrorHandler to use *slog.Logger and slog-style attribute logging.
Switch platform/middleware/ratelimit.ErrorMiddleware logging to *slog.Logger + WarnContext.
Remove platform/logging.DiscardHandler and update tests/callers to use slog.DiscardHandler.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
server/platform/endpointer/endpoint_utils.go	Migrates `ErrorHandler` from go-kit/log to `*slog.Logger` and updates error logging implementation.
server/platform/middleware/ratelimit/ratelimit.go	Migrates error-rate-limit middleware logging from go-kit/log to slog.
server/platform/logging/logging.go	Removes custom `DiscardHandler` and uses `slog.DiscardHandler` in `NewNopLogger`.
server/service/handler.go	Updates handler wiring to pass `logger.SlogLogger()` into endpointer error handler and rate limit middleware.
server/service/endpoint_utils_test.go	Updates tests to use slog discard logger and removes now-unused go-kit/platform logging imports.
server/service/integration_android_certificate_templates_test.go	Updates integration test to use `slog.DiscardHandler` instead of the removed custom discard handler.
server/mdm/android/tests/testing_utils.go	Updates Android test server wiring to pass `logger.SlogLogger()` into endpointer error handler.
server/mdm/android/service/profiles_test.go	Updates Android service tests to use `slog.DiscardHandler` instead of the removed custom discard handler.

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

codecov · 2026-02-16T18:16:17Z

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.27%. Comparing base (3f8875c) to head (d9a2682).
⚠️ Report is 63 commits behind head on main.

Files with missing lines	Patch %	Lines
server/platform/middleware/ratelimit/ratelimit.go	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #39928      +/-   ##
==========================================
- Coverage   66.27%   66.27%   -0.01%     
==========================================
  Files        2439     2439              
  Lines      195446   195454       +8     
  Branches     8514     8514              
==========================================
- Hits       129540   129536       -4     
- Misses      54175    54187      +12     
  Partials    11731    11731

Flag	Coverage Δ
backend	`68.07% <93.75%> (-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.

kilo-code-bot · 2026-02-16T19:08:47Z

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Well hello there, neighbor! I took a nice careful look through all the changes in this PR, and I'm happy to say everything looks just wonderful.

This PR cleanly migrates the ErrorHandler in endpoint_utils.go and the ErrorMiddleware.Limit in ratelimit.go from go-kit/log to log/slog, and replaces the custom DiscardHandler with the standard library's slog.DiscardHandler (available since Go 1.24). The key-value pair contract from LogFields() remains compatible with slog's variadic args pattern, and all test files have been updated consistently.

Files Reviewed (8 files)

server/platform/endpointer/endpoint_utils.go - Core migration from go-kit/log to slog
server/platform/middleware/ratelimit/ratelimit.go - Migrated Limit() signature and logging
server/platform/logging/logging.go - Removed custom DiscardHandler, uses slog.DiscardHandler
server/service/handler.go - Updated callers to use SlogLogger()
server/service/endpoint_utils_test.go - Test updates
server/mdm/android/service/profiles_test.go - Test updates
server/mdm/android/tests/testing_utils.go - Test updates
server/service/integration_android_certificate_templates_test.go - Test updates

jacobshandling · 2026-02-18T15:29:35Z

 // Use this in tests instead of kitlog.NewNopLogger() to maintain type safety.
 func NewNopLogger() *Logger {
-	return NewLogger(slog.New(DiscardHandler{}))
+	return NewLogger(slog.New(slog.DiscardHandler))


jacobshandling · 2026-02-18T15:35:07Z

-					"ip", publicIP,
-					"msg", "limit exceeded",
-				)
+				logger.WarnContext(ctx, "limit exceeded", "ip", publicIP)


Are you grabbing anything from this context elsewhere in the logging stack, or is adding context here more for opening possibilities for future use?

Yes, we're logging trace_id and span_id from context. So, in dogfood logs, you'll be able to search by the trace_id to get all the logs belonging to a specific API call or cron job run. Or, you can go to our OTEL backend and see the trace by the trace_id.

jacobshandling

LGTM, left a couple clarifying questions

Migrate platform/endpointer and platform/middleware/ratelimit to slog.

d9a2682

getvictor temporarily deployed to Docker Hub February 16, 2026 17:56 — with GitHub Actions Inactive

getvictor requested a review from Copilot February 16, 2026 17:56

Copilot started reviewing on behalf of getvictor February 16, 2026 17:58 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

Comment thread server/platform/endpointer/endpoint_utils.go

getvictor marked this pull request as ready for review February 16, 2026 19:06

getvictor requested a review from a team as a code owner February 16, 2026 19:06

getvictor assigned jacobshandling Feb 16, 2026

getvictor requested a review from jacobshandling February 16, 2026 19:06

jacobshandling reviewed Feb 18, 2026

View reviewed changes

jacobshandling self-requested a review February 18, 2026 15:35

jacobshandling approved these changes Feb 18, 2026

View reviewed changes

getvictor merged commit d447b32 into main Feb 18, 2026
54 checks passed

getvictor deleted the victor/38889-slog-platform branch February 18, 2026 16:09

Navigation Menu

Uh oh!

Migrate platform/endpointer and platform/middleware/ratelimit to slog. - #39928

Migrate platform/endpointer and platform/middleware/ratelimit to slog.#39928
getvictor merged 1 commit into
mainfrom
victor/38889-slog-platform

getvictor commented Feb 16, 2026 •

edited

Loading

Uh oh!

getvictor commented Feb 16, 2026

Uh oh!

coderabbitai Bot commented Feb 16, 2026

Uh oh!

coderabbitai Bot commented Feb 16, 2026 •

edited

Loading

❌ Failed checks (3 warnings)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codecov Bot commented Feb 16, 2026 •

edited

Loading

Uh oh!

kilo-code-bot Bot commented Feb 16, 2026

Uh oh!

jacobshandling Feb 18, 2026

Uh oh!

jacobshandling Feb 18, 2026

Uh oh!

getvictor Feb 18, 2026

Uh oh!

jacobshandling Feb 18, 2026

Uh oh!

jacobshandling left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

getvictor commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist for submitter

Testing

Summary by CodeRabbit

Uh oh!

getvictor commented Feb 16, 2026

Uh oh!

coderabbitai Bot commented Feb 16, 2026

Uh oh!

coderabbitai Bot commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Possibly related PRs

❌ Failed checks (3 warnings)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov Bot commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kilo-code-bot Bot commented Feb 16, 2026

Code Review Summary

Uh oh!

jacobshandling Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

jacobshandling Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

getvictor Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

jacobshandling Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

jacobshandling left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

getvictor commented Feb 16, 2026 •

edited

Loading

coderabbitai Bot commented Feb 16, 2026 •

edited

Loading

codecov Bot commented Feb 16, 2026 •

edited

Loading