Add jitter to intervals (#2158)

* Add max jitter percent config

* Fix jitter calc

* Remove comment

* Reduce test jitter to make tests less flaky

* Remove jitter entirely

* Document new config

* Fix doc link

changes/add-jitter-percent

@@ -0,0 +1 @@
+* Add jitter percent for osquery update intervals to prevent all hosts from returning data at roughly the same time.

docs/02-Deploying/02-Configuration.md

@@ -433,7 +433,7 @@ The address to serve the Fleet webserver.
 
 The TLS cert to use when terminating TLS.
 
-See [TLS certificate considerations](./1-Installation.md#tls-certificate-considerations) for more information about certificates and Fleet.
+See [TLS certificate considerations](./01-Installation.md#tls-certificate-considerations) for more information about certificates and Fleet.
 
 - Default value: `./tools/osquery/fleet.crt`
 - Environment variable: `FLEET_SERVER_CERT`
@@ -727,6 +727,24 @@ Options are `filesystem`, `firehose`, `kinesis`, `lambda`, `pubsub`, and `stdout
   	result_log_plugin: firehose
   ```
 
+###### osquery_max_jitter_percent
+
+Given an update interval (label, or details), this will add up to the defined percentage in randomness to the interval.
+
+The goal of this is to prevent all hosts from checking in with data at the same time.
+
+So for example, if the label_update_interval is 1h, and this is set to 10. It'll add up a random number between 0 and 6 minutes
+to the amount of time it takes for fleet to give the host the label queries.
+
+- Default value: `10`
+- Environment variable: `FLEET_OSQUERY_MAX_JITTER_PERCENT`
+- Config file format:
+
+  ```
+  osquery:
+  	max_jitter_percent: 10
+  ```
+
 ##### Logging (Fleet server logging)
 
 ###### logging_debug

server/config/config.go

@@ -95,6 +95,7 @@ type OsqueryConfig struct {
 	StatusLogFile        string        `yaml:"status_log_file"`
 	ResultLogFile        string        `yaml:"result_log_file"`
 	EnableLogRotation    bool          `yaml:"enable_log_rotation"`
+	MaxJitterPercent     int           `yaml:"max_jitter_percent"`
 }
 
 // LoggingConfig defines configs related to logging
@@ -306,6 +307,8 @@ func (man Manager) addConfigs() {
 		"(DEPRECATED: Use filesystem.result_log_file) Path for osqueryd result logs")
 	man.addConfigBool("osquery.enable_log_rotation", false,
 		"(DEPRECATED: Use filesystem.enable_log_rotation) Enable automatic rotation for osquery log files")
+	man.addConfigInt("osquery.max_jitter_percent", 10,
+		"Maximum percentage of the interval to add as jitter")
 
 	// Logging
 	man.addConfigBool("logging.debug", false,
@@ -463,6 +466,7 @@ func (man Manager) LoadConfig() FleetConfig {
 			LabelUpdateInterval:  man.getConfigDuration("osquery.label_update_interval"),
 			DetailUpdateInterval: man.getConfigDuration("osquery.detail_update_interval"),
 			EnableLogRotation:    man.getConfigBool("osquery.enable_log_rotation"),
+			MaxJitterPercent:     man.getConfigInt("osquery.max_jitter_percent"),
 		},
 		Logging: LoggingConfig{
 			Debug:         man.getConfigBool("logging.debug"),
@@ -749,6 +753,7 @@ func TestConfig() FleetConfig {
 			ResultLogPlugin:      "filesystem",
 			LabelUpdateInterval:  1 * time.Hour,
 			DetailUpdateInterval: 1 * time.Hour,
+			MaxJitterPercent:     0,
 		},
 		Logging: LoggingConfig{
 			Debug:         true,

server/datastore/mysql/labels.go

@@ -283,39 +283,12 @@ func platformForHost(host *fleet.Host) string {
 	return host.Platform
 }
 
-func (d *Datastore) LabelQueriesForHost(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error) {
+func (d *Datastore) LabelQueriesForHost(ctx context.Context, host *fleet.Host) (map[string]string, error) {
 	var rows *sql.Rows
 	var err error
 	platform := platformForHost(host)
-	if host.LabelUpdatedAt.Before(cutoff) {
-		// Retrieve all labels (with matching platform) for this host
-		sql := `
-			SELECT id, query
-			FROM labels
-			WHERE platform = ? OR platform = ''
-			AND label_membership_type = ?
-`
-		rows, err = d.reader.QueryContext(ctx, sql, platform, fleet.LabelMembershipTypeDynamic)
-	} else {
-		// Retrieve all labels (with matching platform) iff there is a label
-		// that has been created since this host last reported label query
-		// executions
-		sql := `
-			SELECT id, query
-			FROM labels
-			WHERE ((SELECT max(created_at) FROM labels WHERE platform = ? OR platform = '') > ?)
-			AND (platform = ? OR platform = '')
-			AND label_membership_type = ?
-`
-		rows, err = d.reader.QueryContext(
-			ctx,
-			sql,
-			platform,
-			host.LabelUpdatedAt,
-			platform,
-			fleet.LabelMembershipTypeDynamic,
-		)
-	}
+	query := `SELECT id, query FROM labels WHERE platform = ? OR platform = '' AND label_membership_type = ?`
+	rows, err = d.reader.QueryContext(ctx, query, platform, fleet.LabelMembershipTypeDynamic)
 
 	if err != nil && err != sql.ErrNoRows {
 		return nil, errors.Wrap(err, "selecting label queries for host")

server/datastore/mysql/labels_test.go

@@ -80,10 +80,8 @@ func testLabelsAddAllHosts(t *testing.T, db *Datastore) {
 	host.Platform = "darwin"
 	require.NoError(t, db.SaveHost(context.Background(), host))
 
-	baseTime := time.Now()
-
 	// No labels to check
-	queries, err := db.LabelQueriesForHost(context.Background(), host, baseTime)
+	queries, err := db.LabelQueriesForHost(context.Background(), host)
 	assert.Nil(t, err)
 	assert.Len(t, queries, 0)
 
@@ -127,7 +125,7 @@ func testLabelsAddAllHosts(t *testing.T, db *Datastore) {
 	host.Platform = "darwin"
 
 	// Now queries should be returned
-	queries, err = db.LabelQueriesForHost(context.Background(), host, baseTime)
+	queries, err = db.LabelQueriesForHost(context.Background(), host)
 	assert.Nil(t, err)
 	assert.Equal(t, expectQueries, queries)
 
@@ -136,6 +134,8 @@ func testLabelsAddAllHosts(t *testing.T, db *Datastore) {
 	assert.Nil(t, err)
 	assert.Len(t, labels, 1)
 
+	baseTime := time.Now()
+
 	// Record a query execution
 	err = db.RecordLabelQueryExecutions(
 		context.Background(),
@@ -148,14 +148,6 @@ func testLabelsAddAllHosts(t *testing.T, db *Datastore) {
 	require.NoError(t, err)
 	host.LabelUpdatedAt = baseTime
 
-	// Now no queries should be returned
-	queries, err = db.LabelQueriesForHost(context.Background(), host, baseTime.Add(-1*time.Minute))
-	assert.Nil(t, err)
-	assert.Len(t, queries, 0)
-
-	// Ensure enough gap in created_at
-	time.Sleep(2 * time.Second)
-
 	// A new label targeting another platform should not effect the labels for
 	// this host
 	err = db.ApplyLabelSpecs(context.Background(), []*fleet.LabelSpec{
@@ -166,9 +158,9 @@ func testLabelsAddAllHosts(t *testing.T, db *Datastore) {
 		},
 	})
 	require.NoError(t, err)
-	queries, err = db.LabelQueriesForHost(context.Background(), host, baseTime.Add(-1*time.Minute))
+	queries, err = db.LabelQueriesForHost(context.Background(), host)
 	assert.Nil(t, err)
-	assert.Len(t, queries, 0)
+	assert.Len(t, queries, 4)
 
 	// If a new label is added, all labels should be returned
 	err = db.ApplyLabelSpecs(context.Background(), []*fleet.LabelSpec{
@@ -180,29 +172,7 @@ func testLabelsAddAllHosts(t *testing.T, db *Datastore) {
 	})
 	require.NoError(t, err)
 	expectQueries["7"] = "query6"
-	queries, err = db.LabelQueriesForHost(context.Background(), host, baseTime.Add(-1*time.Minute))
-	assert.Nil(t, err)
-	assert.Len(t, queries, 5)
-
-	// After expiration, all queries should be returned
-	queries, err = db.LabelQueriesForHost(context.Background(), host, baseTime.Add((2 * time.Minute)))
-	assert.Nil(t, err)
-	assert.Equal(t, expectQueries, queries)
-
-	// Now the two matching labels should be returned
-	labels, err = db.ListLabelsForHost(context.Background(), host.ID)
-	assert.Nil(t, err)
-	if assert.Len(t, labels, 2) {
-		labelNames := []string{labels[0].Name, labels[1].Name}
-		sort.Strings(labelNames)
-		assert.Equal(t, "All Hosts", labelNames[0])
-		assert.Equal(t, "label1", labelNames[1])
-	}
-
-	// A host that hasn't executed any label queries should still be asked
-	// to execute those queries
-	hosts[0].Platform = "darwin"
-	queries, err = db.LabelQueriesForHost(context.Background(), &hosts[0], time.Now())
+	queries, err = db.LabelQueriesForHost(context.Background(), host)
 	assert.Nil(t, err)
 	assert.Len(t, queries, 5)
 
@@ -663,9 +633,7 @@ func testLabelsQueriesForCentOSHost(t *testing.T, db *Datastore) {
 	})
 	require.NoError(t, err)
 
-	baseTime := time.Now().Add(-5 * time.Minute)
-
-	queries, err := db.LabelQueriesForHost(context.Background(), host, baseTime)
+	queries, err := db.LabelQueriesForHost(context.Background(), host)
 	require.NoError(t, err)
 	require.Len(t, queries, 1)
 	assert.Equal(t, "select 1;", queries[fmt.Sprint(label.ID)])

server/fleet/datastore.go

@@ -139,10 +139,9 @@ type Datastore interface {
 	Label(ctx context.Context, lid uint) (*Label, error)
 	ListLabels(ctx context.Context, filter TeamFilter, opt ListOptions) ([]*Label, error)
 
-	// LabelQueriesForHost returns the label queries that should be executed for the given host. The cutoff is the
-	// minimum timestamp a query execution should have to be considered "fresh". Executions that are not fresh will be
-	// repeated. Results are returned in a map of label id -> query
-	LabelQueriesForHost(ctx context.Context, host *Host, cutoff time.Time) (map[string]string, error)
+	// LabelQueriesForHost returns the label queries that should be executed for the given host.
+	// Results are returned in a map of label id -> query
+	LabelQueriesForHost(ctx context.Context, host *Host) (map[string]string, error)
 
 	// RecordLabelQueryExecutions saves the results of label queries. The results map is a map of label id -> whether or
 	// not the label matches. The time parameter is the timestamp to save with the query execution.

server/mock/datastore_mock.go

@@ -115,7 +115,7 @@ type LabelFunc func(ctx context.Context, lid uint) (*fleet.Label, error)
 
 type ListLabelsFunc func(ctx context.Context, filter fleet.TeamFilter, opt fleet.ListOptions) ([]*fleet.Label, error)
 
-type LabelQueriesForHostFunc func(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error)
+type LabelQueriesForHostFunc func(ctx context.Context, host *fleet.Host) (map[string]string, error)
 
 type RecordLabelQueryExecutionsFunc func(ctx context.Context, host *fleet.Host, results map[uint]*bool, t time.Time) error
 
@@ -963,9 +963,9 @@ func (s *DataStore) ListLabels(ctx context.Context, filter fleet.TeamFilter, opt
 	return s.ListLabelsFunc(ctx, filter, opt)
 }
 
-func (s *DataStore) LabelQueriesForHost(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error) {
+func (s *DataStore) LabelQueriesForHost(ctx context.Context, host *fleet.Host) (map[string]string, error) {
 	s.LabelQueriesForHostFuncInvoked = true
-	return s.LabelQueriesForHostFunc(ctx, host, cutoff)
+	return s.LabelQueriesForHostFunc(ctx, host)
 }
 
 func (s *DataStore) RecordLabelQueryExecutions(ctx context.Context, host *fleet.Host, results map[uint]*bool, t time.Time) error {

server/service/service.go

@@ -38,10 +38,19 @@ type Service struct {
 }
 
 // NewService creates a new service from the config struct
-func NewService(ds fleet.Datastore, resultStore fleet.QueryResultStore,
-	logger kitlog.Logger, osqueryLogger *logging.OsqueryLogger, config config.FleetConfig, mailService fleet.MailService,
-	c clock.Clock, sso sso.SessionStore, lq fleet.LiveQueryStore, carveStore fleet.CarveStore,
-	license fleet.LicenseInfo) (fleet.Service, error) {
+func NewService(
+	ds fleet.Datastore,
+	resultStore fleet.QueryResultStore,
+	logger kitlog.Logger,
+	osqueryLogger *logging.OsqueryLogger,
+	config config.FleetConfig,
+	mailService fleet.MailService,
+	c clock.Clock,
+	sso sso.SessionStore,
+	lq fleet.LiveQueryStore,
+	carveStore fleet.CarveStore,
+	license fleet.LicenseInfo,
+) (fleet.Service, error) {
 	var svc fleet.Service
 
 	authorizer, err := authz.NewAuthorizer()

server/service/service_campaign_test.go

@@ -36,7 +36,7 @@ func TestStreamCampaignResultsClosesReditOnWSClose(t *testing.T) {
 
 	campaign := &fleet.DistributedQueryCampaign{ID: 42}
 
-	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error) {
+	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
 		return map[string]string{}, nil
 	}
 	ds.SaveHostFunc = func(ctx context.Context, host *fleet.Host) error {

server/service/service_osquery.go

@@ -2,8 +2,10 @@ package service
 
 import (
 	"context"
+	"crypto/rand"
 	"encoding/json"
 	"fmt"
+	"math/big"
 	"strconv"
 	"strings"
 	"time"
@@ -404,7 +406,7 @@ const hostDistributedQueryPrefix = "fleet_distributed_query_"
 // osqueryd to fill in the host details
 func (svc *Service) hostDetailQueries(ctx context.Context, host fleet.Host) (map[string]string, error) {
 	queries := make(map[string]string)
-	if host.DetailUpdatedAt.After(svc.clock.Now().Add(-svc.config.Osquery.DetailUpdateInterval)) && !host.RefetchRequested {
+	if !svc.shouldUpdate(host.DetailUpdatedAt, svc.config.Osquery.DetailUpdateInterval) && !host.RefetchRequested {
 		// No need to update already fresh details
 		return queries, nil
 	}
@@ -438,6 +440,19 @@ func (svc *Service) hostDetailQueries(ctx context.Context, host fleet.Host) (map
 	return queries, nil
 }
 
+func (svc *Service) shouldUpdate(lastUpdated time.Time, interval time.Duration) bool {
+	var jitter time.Duration
+	if svc.config.Osquery.MaxJitterPercent > 0 {
+		maxJitter := time.Duration(svc.config.Osquery.MaxJitterPercent) * interval / time.Duration(100.0)
+		randDuration, err := rand.Int(rand.Reader, big.NewInt(int64(maxJitter)))
+		if err == nil {
+			jitter = time.Duration(randDuration.Int64())
+		}
+	}
+	cutoff := svc.clock.Now().Add(-(interval + jitter))
+	return lastUpdated.Before(cutoff)
+}
+
 func (svc *Service) GetDistributedQueries(ctx context.Context) (map[string]string, uint, error) {
 	// skipauth: Authorization is currently for user endpoints only.
 	svc.authz.SkipAuthorization(ctx)
@@ -455,14 +470,15 @@ func (svc *Service) GetDistributedQueries(ctx context.Context) (map[string]strin
 	}
 
 	// Retrieve the label queries that should be updated
-	cutoff := svc.clock.Now().Add(-svc.config.Osquery.LabelUpdateInterval)
-	labelQueries, err := svc.ds.LabelQueriesForHost(ctx, &host, cutoff)
-	if err != nil {
-		return nil, 0, osqueryError{message: "retrieving label queries: " + err.Error()}
-	}
+	if svc.shouldUpdate(host.LabelUpdatedAt, svc.config.Osquery.LabelUpdateInterval) {
+		labelQueries, err := svc.ds.LabelQueriesForHost(ctx, &host)
+		if err != nil {
+			return nil, 0, osqueryError{message: "retrieving label queries: " + err.Error()}
+		}
 
-	for name, query := range labelQueries {
-		queries[hostLabelQueryPrefix+name] = query
+		for name, query := range labelQueries {
+			queries[hostLabelQueryPrefix+name] = query
+		}
 	}
 
 	liveQueries, err := svc.liveQueryStore.QueriesForHost(host.ID)

server/service/service_osquery_test.go

@@ -324,7 +324,7 @@ func TestLabelQueries(t *testing.T) {
 		Platform: "darwin",
 	}
 
-	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error) {
+	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
 		return map[string]string{}, nil
 	}
 	ds.HostFunc = func(ctx context.Context, id uint) (*fleet.Host, error) {
@@ -361,7 +361,7 @@ func TestLabelQueries(t *testing.T) {
 	assert.Len(t, queries, 0)
 	assert.Zero(t, acc)
 
-	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error) {
+	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
 		return map[string]string{
 			"label1": "query1",
 			"label2": "query2",
@@ -539,7 +539,7 @@ func TestDetailQueriesWithEmptyStrings(t *testing.T) {
 	ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) {
 		return &fleet.AppConfig{HostSettings: fleet.HostSettings{EnableHostUsers: true}}, nil
 	}
-	ds.LabelQueriesForHostFunc = func(context.Context, *fleet.Host, time.Time) (map[string]string, error) {
+	ds.LabelQueriesForHostFunc = func(context.Context, *fleet.Host) (map[string]string, error) {
 		return map[string]string{}, nil
 	}
 	ds.PolicyQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
@@ -715,7 +715,7 @@ func TestDetailQueries(t *testing.T) {
 	ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) {
 		return &fleet.AppConfig{HostSettings: fleet.HostSettings{EnableHostUsers: true}}, nil
 	}
-	ds.LabelQueriesForHostFunc = func(context.Context, *fleet.Host, time.Time) (map[string]string, error) {
+	ds.LabelQueriesForHostFunc = func(context.Context, *fleet.Host) (map[string]string, error) {
 		return map[string]string{}, nil
 	}
 	ds.PolicyQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
@@ -920,7 +920,7 @@ func TestNewDistributedQueryCampaign(t *testing.T) {
 	mockClock := clock.NewMockClock()
 	svc := newTestServiceWithClock(ds, rs, lq, mockClock)
 
-	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error) {
+	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
 		return map[string]string{}, nil
 	}
 	ds.SaveHostFunc = func(ctx context.Context, host *fleet.Host) error {
@@ -989,7 +989,7 @@ func TestDistributedQueryResults(t *testing.T) {
 
 	campaign := &fleet.DistributedQueryCampaign{ID: 42}
 
-	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error) {
+	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
 		return map[string]string{}, nil
 	}
 	ds.PolicyQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
@@ -1721,7 +1721,7 @@ func TestObserversCanOnlyRunDistributedCampaigns(t *testing.T) {
 		}, nil
 	}
 
-	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error) {
+	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
 		return map[string]string{}, nil
 	}
 	ds.SaveHostFunc = func(ctx context.Context, host *fleet.Host) error { return nil }
@@ -1813,7 +1813,7 @@ func TestPolicyQueries(t *testing.T) {
 		Platform: "darwin",
 	}
 
-	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host, cutoff time.Time) (map[string]string, error) {
+	ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) {
 		return map[string]string{}, nil
 	}
 	ds.HostFunc = func(ctx context.Context, id uint) (*fleet.Host, error) {