osquery-perf: honor interval agent options served in config responses#49247
Draft
lukeheath wants to merge 4 commits into
Draft
osquery-perf: honor interval agent options served in config responses#49247lukeheath wants to merge 4 commits into
lukeheath wants to merge 4 commits into
Conversation
lukeheath
requested review from
a team,
georgekarrv,
getvictor,
lucasmrod and
sharon-fdm
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Updates osquery-perf so its distributed/config/logger loops dynamically honor interval overrides served in Fleet’s /api/osquery/config response (plus a simulator-only “quiet” signal), enabling server-side re-pacing/silencing of large running simulations without restarts. Also exposes a Terraform variable through the loadtest infra workflow to tune MySQL connection pool sizing per Fleet container.
Changes:
- Apply served agent option intervals (
distributed_interval,config_refresh,logger_tls_period) and reset osquery tickers when values change. - Add an orbit/Fleet Desktop “quiet watcher” that stretches/restores several tickers based on a served
distributed_interval >= 1hquiet signal. - Add
fleet_mysql_max_open_connsinput to the loadtest infra GitHub Actions workflow and pass it to Terraform viaTF_VAR_mysql_max_open_conns.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/osquery-perf/agent.go | Adds parsing/storage of served interval options and ticker-reset logic across osquery + orbit/Fleet Desktop loops. |
| .github/workflows/loadtest-infra.yml | Adds a workflow_dispatch input and exports it as TF_VAR_mysql_max_open_conns for Terraform. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1046
to
+1051
| func effectiveInterval(served *atomic.Int64, def time.Duration) time.Duration { | ||
| if secs := served.Load(); secs > 0 { | ||
| return time.Duration(secs) * time.Second | ||
| } | ||
| return def | ||
| } |
Comment on lines
+976
to
+981
| configEffective := func() time.Duration { | ||
| if quietFor := a.servedDistributedInterval.Load(); quietFor >= 3600 { | ||
| return time.Duration(quietFor) * time.Second | ||
| } | ||
| return effectiveInterval(&a.servedConfigRefresh, a.ConfigInterval) | ||
| } |
Contributor
WalkthroughThe loadtest infrastructure workflow now accepts Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/osquery-perf/agent.go (1)
969-993: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winQuiet threshold (1h) is hardcoded twice — extract a shared constant.
The config loop checks
quietFor >= 3600(Line 977) while the orbitquietWatcherchecksquietFor >= time.Hour(Line 1271). Both encode the same "served distributed_interval ≥ 1h means quiet" contract described in the PR objectives, but as two independently-maintained literals. A future edit to one threshold without the other would silently desync the osquery loops from the orbit/Fleet Desktop quiet signal.♻️ Suggested fix
+const quietThreshold = time.Hour + go func() { configEffective := func() time.Duration { - if quietFor := a.servedDistributedInterval.Load(); quietFor >= 3600 { + if quietFor := a.servedDistributedInterval.Load(); time.Duration(quietFor)*time.Second >= quietThreshold { return time.Duration(quietFor) * time.Second } return effectiveInterval(&a.servedConfigRefresh, a.ConfigInterval) }case <-quietWatcher.C: quietFor := time.Duration(a.servedDistributedInterval.Load()) * time.Second - quiet := quietFor >= time.Hour + quiet := quietFor >= quietThresholdAlso applies to: 1267-1289
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/osquery-perf/agent.go` around lines 969 - 993, Extract the shared one-hour quiet threshold used by the config loop’s configEffective function and the orbit quietWatcher into a named package-level constant, then use that constant in both comparisons. Preserve the existing duration behavior when converting servedDistributedInterval values to seconds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/osquery-perf/agent.go`:
- Around line 1254-1289: Update the quietWatcher logic around orbitQuiet and
quietFor so changes to the served quiet duration are applied even when quiet
remains true. Track the previously applied quiet duration, skip only when both
quiet state and duration are unchanged, and re-run stretchOrRestore for all
affected tickers whenever the duration changes.
---
Nitpick comments:
In `@cmd/osquery-perf/agent.go`:
- Around line 969-993: Extract the shared one-hour quiet threshold used by the
config loop’s configEffective function and the orbit quietWatcher into a named
package-level constant, then use that constant in both comparisons. Preserve the
existing duration behavior when converting servedDistributedInterval values to
seconds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 67f466d8-8f8c-4f9f-a3ab-617e6aac0cc5
📒 Files selected for processing (2)
.github/workflows/loadtest-infra.ymlcmd/osquery-perf/agent.go
Comment on lines
+1254
to
+1289
| // quietWatcher makes no server requests; it watches the quiet signal | ||
| // (served distributed_interval >= 1h, parsed by the osquery config thread) | ||
| // and stretches or restores the orbit and Fleet Desktop tickers. Real | ||
| // fleetd has no server-driven interval knob, so this simulator-only | ||
| // behavior is what lets a load test silence all fleetd-originated traffic | ||
| // with the same agent-options change that quiets osquery. | ||
| quietWatcher := time.NewTicker(20 * time.Second) | ||
| defer quietWatcher.Stop() | ||
| orbitQuiet := false | ||
|
|
||
| const windowsMDMEnrollmentAttemptFrequency = time.Hour | ||
| var lastEnrollAttempt time.Time | ||
|
|
||
| for { | ||
| select { | ||
| case <-orbitConfigTicker: | ||
| case <-quietWatcher.C: | ||
| quietFor := time.Duration(a.servedDistributedInterval.Load()) * time.Second | ||
| quiet := quietFor >= time.Hour | ||
| if quiet == orbitQuiet { | ||
| continue | ||
| } | ||
| orbitQuiet = quiet | ||
| stretchOrRestore := func(t *time.Ticker, def time.Duration) { | ||
| if quiet { | ||
| t.Reset(quietFor) | ||
| } else { | ||
| t.Reset(def) | ||
| } | ||
| } | ||
| stretchOrRestore(orbitConfigTicker, orbitConfigDefault) | ||
| stretchOrRestore(orbitTokenRemoteCheckTicker, orbitTokenRemoteCheckDefault) | ||
| stretchOrRestore(orbitTokenRotationTicker, orbitTokenRotationDefault) | ||
| stretchOrRestore(capabilitiesCheckerTicker, capabilitiesCheckerDefault) | ||
| stretchOrRestore(fleetDesktopPolicyTicker, fleetDesktopPolicyDefault) | ||
| stretchOrRestore(fleetDesktopConnectivityCheck, fleetDesktopConnectivityDefault) | ||
| case <-orbitConfigTicker.C: |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
quietFor value changes are ignored while orbitQuiet stays true.
if quiet == orbitQuiet { continue } (Line 1272-1274) only reacts to on/off transitions. If servedDistributedInterval changes from, say, 3600s to 7200s while staying ≥ 1h, quiet stays true, orbitQuiet stays true, and the function returns early without calling stretchOrRestore — so all the orbit/Fleet Desktop tickers remain frozen at the first quiet duration ever observed instead of following the new served value, unlike the distributed/config/logger loops which do re-apply changed intervals every tick.
🐛 Suggested fix
quietWatcher := time.NewTicker(20 * time.Second)
defer quietWatcher.Stop()
orbitQuiet := false
+ var lastQuietFor time.Duration
const windowsMDMEnrollmentAttemptFrequency = time.Hour
var lastEnrollAttempt time.Time
for {
select {
case <-quietWatcher.C:
quietFor := time.Duration(a.servedDistributedInterval.Load()) * time.Second
quiet := quietFor >= time.Hour
- if quiet == orbitQuiet {
+ if quiet == orbitQuiet && (!quiet || quietFor == lastQuietFor) {
continue
}
orbitQuiet = quiet
+ lastQuietFor = quietFor
stretchOrRestore := func(t *time.Ticker, def time.Duration) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // quietWatcher makes no server requests; it watches the quiet signal | |
| // (served distributed_interval >= 1h, parsed by the osquery config thread) | |
| // and stretches or restores the orbit and Fleet Desktop tickers. Real | |
| // fleetd has no server-driven interval knob, so this simulator-only | |
| // behavior is what lets a load test silence all fleetd-originated traffic | |
| // with the same agent-options change that quiets osquery. | |
| quietWatcher := time.NewTicker(20 * time.Second) | |
| defer quietWatcher.Stop() | |
| orbitQuiet := false | |
| const windowsMDMEnrollmentAttemptFrequency = time.Hour | |
| var lastEnrollAttempt time.Time | |
| for { | |
| select { | |
| case <-orbitConfigTicker: | |
| case <-quietWatcher.C: | |
| quietFor := time.Duration(a.servedDistributedInterval.Load()) * time.Second | |
| quiet := quietFor >= time.Hour | |
| if quiet == orbitQuiet { | |
| continue | |
| } | |
| orbitQuiet = quiet | |
| stretchOrRestore := func(t *time.Ticker, def time.Duration) { | |
| if quiet { | |
| t.Reset(quietFor) | |
| } else { | |
| t.Reset(def) | |
| } | |
| } | |
| stretchOrRestore(orbitConfigTicker, orbitConfigDefault) | |
| stretchOrRestore(orbitTokenRemoteCheckTicker, orbitTokenRemoteCheckDefault) | |
| stretchOrRestore(orbitTokenRotationTicker, orbitTokenRotationDefault) | |
| stretchOrRestore(capabilitiesCheckerTicker, capabilitiesCheckerDefault) | |
| stretchOrRestore(fleetDesktopPolicyTicker, fleetDesktopPolicyDefault) | |
| stretchOrRestore(fleetDesktopConnectivityCheck, fleetDesktopConnectivityDefault) | |
| case <-orbitConfigTicker.C: | |
| quietWatcher := time.NewTicker(20 * time.Second) | |
| defer quietWatcher.Stop() | |
| orbitQuiet := false | |
| var lastQuietFor time.Duration | |
| const windowsMDMEnrollmentAttemptFrequency = time.Hour | |
| var lastEnrollAttempt time.Time | |
| for { | |
| select { | |
| case <-quietWatcher.C: | |
| quietFor := time.Duration(a.servedDistributedInterval.Load()) * time.Second | |
| quiet := quietFor >= time.Hour | |
| if quiet == orbitQuiet && (!quiet || quietFor == lastQuietFor) { | |
| continue | |
| } | |
| orbitQuiet = quiet | |
| lastQuietFor = quietFor | |
| stretchOrRestore := func(t *time.Ticker, def time.Duration) { | |
| if quiet { | |
| t.Reset(quietFor) | |
| } else { | |
| t.Reset(def) | |
| } | |
| } | |
| stretchOrRestore(orbitConfigTicker, orbitConfigDefault) | |
| stretchOrRestore(orbitTokenRemoteCheckTicker, orbitTokenRemoteCheckDefault) | |
| stretchOrRestore(orbitTokenRotationTicker, orbitTokenRotationDefault) | |
| stretchOrRestore(capabilitiesCheckerTicker, capabilitiesCheckerDefault) | |
| stretchOrRestore(fleetDesktopPolicyTicker, fleetDesktopPolicyDefault) | |
| stretchOrRestore(fleetDesktopConnectivityCheck, fleetDesktopConnectivityDefault) | |
| case <-orbitConfigTicker.C: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/osquery-perf/agent.go` around lines 1254 - 1289, Update the quietWatcher
logic around orbitQuiet and quietFor so changes to the served quiet duration are
applied even when quiet remains true. Track the previously applied quiet
duration, skip only when both quiet state and duration are unchanged, and re-run
stretchOrRestore for all affected tickers whenever the duration changes.
…oops Load-test-only branch. Lets a running 50k simulation be silenced (and restored) server-side via agent options, mirroring real osquery's handling of distributed_interval/config_refresh/logger_tls_period. Orbit/Fleet Desktop tickers stretch when the served distributed_interval is >= 1h. MDM loops intentionally untouched.
…vable via agent options)
Raises every orbit/Fleet Desktop polling interval to a configurable floor so a load test can approximate a persistent-connection world: payload deliveries continue at production cadence while the polling that a socket transport would eliminate is stretched out of the measurement.
lukeheath
force-pushed
the
loadtest-served-agent-options
branch
from
7021846 to
d62ba99
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: N/A — tooling improvements from the July 2026 cost-baseline load test.
Makes
osquery-perfhonor interval options served in Fleet's config response (distributed_interval,config_refresh,logger_tls_period), the way real osquery does. Stock osquery-perf reads intervals from startup flags only, so a running simulation cannot be re-paced or quieted server-side — and since the simulator is stateless, restarting containers re-enrolls every host. With this change, a load test can silence (or re-pace) tens of thousands of live simulated agents with a single agent-options change.Details:
distributed_interval≥ 1h as a quiet signal and stretch with it (real fleetd has no server-driven interval knob; simulator-only behavior, documented inline). MDM loops are intentionally untouched.config_refreshcannot be served via config options (Fleet validation routes it tocommand_line_flags), so the config loop also rides the quiet signal.mysql_max_open_connsas a loadtest-infra workflow input (terraform var already existed; default 10 is the known MySQL connection pool tuning for loadtest environment #44802 bottleneck, and the 50k reference architecture calls for 20).Note for reviewers: when hosts are in teams, team-level agent options override global — silencing a fleet requires patching each team's agent options (or global for teamless hosts).
Checklist for submitter
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Summary by CodeRabbit