Skip to content

Switched Android from go-kit/log to slog - #39785

Merged
getvictor merged 2 commits into
mainfrom
victor/38889-android-slog
Feb 13, 2026
Merged

Switched Android from go-kit/log to slog#39785
getvictor merged 2 commits into
mainfrom
victor/38889-android-slog

Conversation

@getvictor

@getvictor getvictor commented Feb 12, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #39785

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

Testing

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

Summary by CodeRabbit

  • Chores
    • Updated internal logging infrastructure across Android MDM services and background jobs to use standard Go logging.
    • Enhanced test coverage for access control and permission enforcement across various endpoints and user roles.

@codecov

codecov Bot commented Feb 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.26%. Comparing base (6f52646) to head (1b30de8).
⚠️ Report is 32 commits behind head on main.

Files with missing lines Patch % Lines
server/mdm/android/service/pubsub.go 45.94% 20 Missing ⚠️
server/mdm/android/service/service.go 27.27% 16 Missing ⚠️
...r/mdm/android/service/androidmgmt/google_client.go 0.00% 12 Missing ⚠️
...er/mdm/android/service/androidmgmt/proxy_client.go 0.00% 8 Missing ⚠️
server/mdm/android/service/reconcile_devices.go 0.00% 5 Missing ⚠️
cmd/fleet/cron.go 0.00% 3 Missing ⚠️
cmd/fleet/serve.go 0.00% 1 Missing ⚠️
server/mdm/android/service/profiles.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #39785   +/-   ##
=======================================
  Coverage   66.26%   66.26%           
=======================================
  Files        2438     2439    +1     
  Lines      195266   195333   +67     
  Branches     8643     8643           
=======================================
+ Hits       129386   129443   +57     
- Misses      54167    54169    +2     
- Partials    11713    11721    +8     
Flag Coverage Δ
backend 68.06% <33.33%> (+<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

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@getvictor
getvictor requested a review from Copilot February 12, 2026 20:37
@coderabbitai

coderabbitai Bot commented Feb 12, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@getvictor getvictor linked an issue Feb 12, 2026 that may be closed by this pull request
36 tasks
@coderabbitai

coderabbitai Bot commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR systematically migrates Android MDM logging from the go-kit/log framework to the standard library slog across multiple components. The migration updates function signatures to accept *slog.Logger instead of kitlog.Logger, replaces go-kit level-based logging calls with slog context-aware methods (ErrorContext, InfoContext, DebugContext), and updates cron jobs, service initialization, and test setup to pass logger.SlogLogger() adapters. Affected files include service constructors, cron job definitions, client implementations, certificate renewal and profile reconciliation logic, device reconciliation, PubSub message processing, worker functions, and corresponding test suites.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 4
❌ Failed checks (4 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is incomplete. It includes only partial checklist items and omits several required sections such as input validation, testing details, and database migration checks that are present in the template. Complete the PR description by filling out all applicable checklist items from the template, including testing methodology, validation checks, and any migration considerations.
Out of Scope Changes check ⚠️ Warning The changes in integration_mdm_test.go include extensive additions of security/access control tests that appear unrelated to the logging migration objective, representing scope creep. Either move the access control and security-related test changes to a separate PR focused on permission enforcement, or clarify their relationship to the logging migration in the PR description.
Docstring Coverage ⚠️ Warning Docstring coverage is 24.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 (37 files):

⚔️ articles/enroll-hosts.md (content)
⚔️ cmd/fleet/cron.go (content)
⚔️ cmd/fleet/serve.go (content)
⚔️ docs/Contributing/guides/committing-changes.md (content)
⚔️ docs/solutions/android/configuration-profiles/disable-camera.json (content)
⚔️ ee/server/service/condaccess/idp.go (content)
⚔️ ee/server/service/condaccess/idp_test.go (content)
⚔️ frontend/interfaces/host.ts (content)
⚔️ frontend/pages/hosts/details/DeviceUserPage/AutoEnrollMdmModal/AutoEnrollMdmModal.tsx (content)
⚔️ frontend/pages/hosts/details/DeviceUserPage/DeviceUserPage.tests.tsx (content)
⚔️ frontend/pages/hosts/details/DeviceUserPage/DeviceUserPage.tsx (content)
⚔️ frontend/pages/hosts/details/cards/Software/HostSoftwareTableConfig.tsx (content)
⚔️ frontend/pages/policies/ManagePoliciesPage/ManagePoliciesPage.tsx (content)
⚔️ frontend/pages/policies/ManagePoliciesPage/components/InstallSoftwareModal/InstallSoftwareModal.tsx (content)
⚔️ frontend/services/entities/device_user.ts (content)
⚔️ frontend/test/handlers/device-handler.ts (content)
⚔️ it-and-security/lib/macos/policies/latest-macos.yml (content)
⚔️ it-and-security/lib/macos/policies/update-safari.yml (content)
⚔️ server/mdm/android/service/androidmgmt/google_client.go (content)
⚔️ server/mdm/android/service/androidmgmt/proxy_client.go (content)
⚔️ server/mdm/android/service/certificate_renewal.go (content)
⚔️ server/mdm/android/service/enterprises_test.go (content)
⚔️ server/mdm/android/service/profiles.go (content)
⚔️ server/mdm/android/service/profiles_test.go (content)
⚔️ server/mdm/android/service/pubsub.go (content)
⚔️ server/mdm/android/service/pubsub_test.go (content)
⚔️ server/mdm/android/service/reconcile_devices.go (content)
⚔️ server/mdm/android/service/service.go (content)
⚔️ server/mdm/android/tests/testing_utils.go (content)
⚔️ server/service/integration_android_certificate_templates_test.go (content)
⚔️ server/service/integration_mdm_test.go (content)
⚔️ server/service/integrationtest/android/suite.go (content)
⚔️ server/vulnerabilities/nvd/cve.go (content)
⚔️ server/vulnerabilities/nvd/cve_test.go (content)
⚔️ server/worker/software_worker.go (content)
⚔️ website/assets/js/pages/osquery-table-details.page.js (content)
⚔️ website/scripts/build-static-content.js (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Switched Android from go-kit/log to slog' clearly and concisely summarizes the main objective of the changeset, which is replacing the logging library across the Android codebase.
Linked Issues check ✅ Passed The code changes comprehensively implement the objective from issue #39785 to replace go-kit/log with slog across the Android codebase, including service functions, tests, and related components.

✏️ 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-android-slog
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch victor/38889-android-slog
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
server/mdm/android/service/androidmgmt/proxy_client.go (1)

44-52: Hardcoded os.Stdout in the Google API logger.

The slogLogger for the Google API client always writes to os.Stdout regardless of the application's logging configuration. This may diverge from how other parts of the application route logs (e.g., to stderr, or structured JSON in production). Consider deriving this from the passed-in logger's handler or making the writer configurable.

That said, if the previous implementation also logged to stdout, this is a faithful migration and can be addressed separately.

server/worker/software_worker.go (1)

7-7: Partial migration leaves a mixed logger API surface in this file.

QueueRunAndroidSetupExperience now takes *slog.Logger, while the sibling Queue* functions (lines 457, 475, 492, 576) and the SoftwareWorker.Log field (line 27) still use kitlog.Logger. This is fine as an incremental migration step, but worth tracking to complete later for consistency.

Also applies to: 13-14

server/service/integration_android_certificate_templates_test.go (1)

435-435: Replace logging.DiscardHandler with Go 1.24's standard library slog.DiscardHandler.

Go 1.24 includes a built-in slog.DiscardHandler in the standard library—use slog.New(slog.DiscardHandler) instead of slog.New(logging.DiscardHandler{}). This removes the dependency on the internal logging package in tests.


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 migrates Android MDM services from go-kit/log to slog as part of the broader logging modernization effort. The changes update service signatures, logging calls, and test initialization to use slog's structured logging with proper context propagation.

Changes:

  • Updated Android service signatures to accept *slog.Logger instead of kitlog.Logger
  • Converted all logging calls from go-kit/log's level.X(logger).Log() pattern to slog's logger.XContext(ctx, ...) pattern
  • Updated test code to create slog loggers and use logging.Logger wrapper for components requiring kitlog interface

Reviewed changes

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

Show a summary per file
File Description
server/worker/software_worker.go Changed QueueRunAndroidSetupExperience signature to accept *slog.Logger and updated logging call
server/service/integrationtest/android/suite.go Updated to call .SlogLogger() when passing logger to Android service
server/service/integration_mdm_test.go Updated Android service initialization to use .SlogLogger() method
server/service/integration_android_certificate_templates_test.go Updated test logger initialization to use slog.New(logging.DiscardHandler{})
server/mdm/android/tests/testing_utils.go Updated test setup to create slog logger and wrap with logging.NewLogger() for kitlog compatibility
server/mdm/android/service/service.go Changed service logger field and constructor to use *slog.Logger, converted all logging calls
server/mdm/android/service/reconcile_devices.go Updated ReconcileAndroidDevices signature and logging calls to use slog
server/mdm/android/service/pubsub_test.go Updated test logger initialization to use slog
server/mdm/android/service/pubsub.go Converted all logging calls to slog pattern with context
server/mdm/android/service/profiles_test.go Updated test loggers to use slog.New(logging.DiscardHandler{})
server/mdm/android/service/profiles.go Updated ReconcileProfiles and related function signatures to accept *slog.Logger
server/mdm/android/service/enterprises_test.go Updated test setup to use logging.NewLogfmtLogger() and call .SlogLogger()
server/mdm/android/service/certificate_renewal.go Updated RenewCertificateTemplates signature and logging to use slog
server/mdm/android/service/androidmgmt/proxy_client.go Changed ProxyClient logger field and constructor to use *slog.Logger, converted logging calls
server/mdm/android/service/androidmgmt/google_client.go Changed GoogleClient logger field and constructor to use *slog.Logger, converted logging calls
cmd/fleet/serve.go Updated Android service initialization to pass .SlogLogger()
cmd/fleet/cron.go Updated cron job calls to pass .SlogLogger() for Android service functions

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

@getvictor
getvictor marked this pull request as ready for review February 12, 2026 20:53
@getvictor
getvictor requested a review from a team as a code owner February 12, 2026 20:53
@getvictor
getvictor merged commit 44aebdf into main Feb 13, 2026
54 checks passed
@getvictor
getvictor deleted the victor/38889-android-slog branch February 13, 2026 14:45
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