Transitioned activity bounded context to slog. by getvictor · Pull Request #39773 · fleetdm/fleet

getvictor · 2026-02-12T17:23:21Z

Related issue: Resolves #38889

Checklist for submitter

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

Testing

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

Summary by CodeRabbit

Chores
- Updated internal logging infrastructure across activity service modules to enhance code consistency and maintainability.

getvictor · 2026-02-12T17:23:33Z

@coderabbitai full review

coderabbitai · 2026-02-12T17:23:45Z

✅ Actions performed

Full review triggered.

coderabbitai · 2026-02-12T17:28:18Z

Walkthrough

This PR migrates the activity logging infrastructure from go-kit/log to the standard library slog. It updates function signatures across the activity bounded context components (bootstrap, datastore, service), replacing logger parameter types from kitlog.Logger to *slog.Logger. Internal logging calls are converted from go-kit patterns to slog-based structured logging methods like WarnContext() and InfoContext(). Test utilities are similarly updated to instantiate slog loggers instead of go-kit nop loggers.

Possibly related PRs

PR #38751: Main implementation of slog migration across activity bootstrap, datastore, and service components following the ADR-0008 migration plan.
PR #39527: Performs identical kitlog.Logger to slog.Logger replacement across activity packages with matching signature and field type updates.
PR #38115: Updates activity bounded-context APIs with the same function signature modifications to NewDatastore, NewService, and bootstrap.New.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The PR description includes a related issue reference and testing checklist items, but lacks detailed explanation of what was changed, why, or how the changes address the linked issue objectives.	Expand the description to explain the scope of slog migration in the activity bounded context and clarify which specific logging patterns were updated.
Linked Issues check	❓ Inconclusive	The PR migrates activity service logging from kitlog to slog in bootstrap, datastore, and service layers. However, issue `#38889` objectives focus on creating a slog foundation with kitlog adapter in server/platform/logging and updating initLogger(), which are not addressed here.	Clarify whether this PR is a partial implementation of `#38889` or a separate follow-up. Verify that the foundation work from `#38889` has been completed separately.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Transitioned activity bounded context to slog' directly and clearly describes the main change: converting the activity service logging from kitlog to slog throughout multiple files.
Out of Scope Changes check	✅ Passed	All changes are focused on migrating the activity bounded context logging layer from kitlog.Logger to *slog.Logger, which is within the scope of issue `#38889`'s gradual slog migration approach.

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

No actionable comments were generated in the recent review. 🎉

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 the activity bounded context to use log/slog directly (instead of go-kit/log), while adapting existing bootstrap/serve/test wiring to pass slog loggers through the bounded-context constructors.

Changes:

Switched activity bounded context constructors (bootstrap/service/mysql datastore) to accept *slog.Logger and updated call sites accordingly.
Updated activity logging calls to slog APIs (InfoContext / WarnContext).
Updated test utilities and tests to construct/pass no-op slog loggers.

Reviewed changes

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

Show a summary per file

File	Description
server/service/testing_utils.go	Passes a `*slog.Logger` into activity bootstrap when adding activity routes in tests.
server/datastore/mysql/testing_utils.go	Updates test activity service bootstrap to use a no-op slog logger.
server/activity/internal/testutils/testutils.go	Changes shared activity test DB helper to store a `*slog.Logger` and adjusts truncate helper logging.
server/activity/internal/service/service_test.go	Updates service tests to use no-op slog loggers.
server/activity/internal/service/service.go	Converts service logger type to `*slog.Logger` and updates log call sites.
server/activity/internal/mysql/activity.go	Converts mysql datastore logger type to `*slog.Logger` and updates warn logging.
server/activity/bootstrap/bootstrap.go	Updates bounded context bootstrap signature to accept `*slog.Logger`.
cmd/fleet/serve.go	Passes `logger.SlogLogger()` into activity bounded context wiring and updates helper signature accordingly.

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

codecov · 2026-02-12T17:39:59Z

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.26%. Comparing base (9f60dad) to head (7b5151e).
⚠️ Report is 76 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/fleet/serve.go	0.00%	2 Missing ⚠️
server/activity/internal/mysql/activity.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #39773   +/-   ##
=======================================
  Coverage   66.26%   66.26%           
=======================================
  Files        2438     2439    +1     
  Lines      195266   195335   +69     
  Branches     8540     8540           
=======================================
+ Hits       129386   129444   +58     
- Misses      54166    54170    +4     
- Partials    11714    11721    +7

Flag	Coverage Δ
backend	`68.06% <70.00%> (+<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.

dantecatalfamo · 2026-02-13T18:06:04Z

 func (tdb *TestDB) TruncateTables(t *testing.T) {
 	t.Helper()
-	mysql_testing_utils.TruncateTables(t, tdb.DB, tdb.Logger, nil, "host_activities", "activities", "hosts", "users")
+	mysql_testing_utils.TruncateTables(t, tdb.DB, kitlog.NewNopLogger(), nil, "host_activities", "activities", "hosts", "users")


Why are we still sending kitlog here?

We need to rip it out of server/platform/mysql/testing_utils which we haven't done yet.

Since that's outside the activity bounded context, I didn't want to do it in this PR.

ksykulev

Looking at the other files (api/, handlers, interfaces, types), they have zero logger usage.

Both
server/activity/internal/tests/suite_test.go
server/activity/internal/mysql/activity_test.go
reference tdb.Logger. But as far as I understand it that's just log.NewNopLogger.

Other then the TruncateTables comment, this PR looks fine.

Transitioned activity bounded context to slog.

d1af6b5

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

getvictor requested a review from Copilot February 12, 2026 17:23

Copilot started reviewing on behalf of getvictor February 12, 2026 17:26 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

Comment thread server/activity/internal/mysql/activity.go

Comment thread server/activity/bootstrap/bootstrap.go

Comment thread server/activity/internal/service/service_test.go

Comment thread server/activity/internal/service/service.go

Code review fixes.

7b5151e

getvictor temporarily deployed to Docker Hub February 12, 2026 20:02 — with GitHub Actions Inactive

getvictor marked this pull request as ready for review February 12, 2026 20:20

getvictor requested a review from a team as a code owner February 12, 2026 20:20

getvictor assigned dantecatalfamo Feb 12, 2026

dantecatalfamo reviewed Feb 13, 2026

View reviewed changes

getvictor requested a review from dantecatalfamo February 13, 2026 20:16

getvictor assigned ksykulev Feb 16, 2026

ksykulev approved these changes Feb 16, 2026

View reviewed changes

getvictor merged commit 988f0bb into main Feb 16, 2026
48 checks passed

getvictor deleted the victor/38889-activity-slog branch February 16, 2026 20:23

This was referenced Feb 18, 2026

slog migration: platform/mysql and related logic #40072

Merged

Migrated logging and google calendar files to use slog #40541

Merged

coderabbitai Bot mentioned this pull request Feb 25, 2026

Move PostJSONWithTimeout to platform/http package and activity cleanup #40561

Merged

3 tasks

Navigation Menu

Uh oh!

Transitioned activity bounded context to slog. - #39773

Transitioned activity bounded context to slog.#39773
getvictor merged 2 commits into
mainfrom
victor/38889-activity-slog

getvictor commented Feb 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

getvictor commented Feb 12, 2026

Uh oh!

coderabbitai Bot commented Feb 12, 2026

Uh oh!

coderabbitai Bot commented Feb 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Feb 12, 2026 •

edited

Loading

Uh oh!

dantecatalfamo Feb 13, 2026

Uh oh!

getvictor Feb 13, 2026

Uh oh!

ksykulev left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

getvictor commented Feb 12, 2026 • edited by coderabbitai Bot 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 12, 2026

Uh oh!

coderabbitai Bot commented Feb 12, 2026

Uh oh!

coderabbitai Bot commented Feb 12, 2026

Walkthrough

Possibly related PRs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

dantecatalfamo Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

getvictor Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

ksykulev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

getvictor commented Feb 12, 2026 •

edited by coderabbitai Bot

Loading

codecov Bot commented Feb 12, 2026 •

edited

Loading