Default "stats_flush_interval" to 1 minute for Consul Telemetry Collector (#19663)

* Set default of 1m for StatsFlushInterval when the collector is setup

* Add documentation on the stats_flush_interval value

* Do not default in two conditions 1) preconfigured sinks exist 2) preconfigured flush interval exists

* Fix wording of docs

* Add changelog

* Fix docs

.changelog/19663.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+connect: Default `stats_flush_interval` to 60 seconds when using the Consul Telemetry Collector, unless custom stats sink are present or an explicit flush interval is configured.
+```

command/connect/envoy/bootstrap_config.go

@@ -251,6 +251,11 @@ func (c *BootstrapConfig) ConfigureArgs(args *BootstrapTplArgs, omitDeprecatedTa
 
 	// Setup telemetry collector if needed. This MUST happen after the Static*JSON is set above
 	if c.TelemetryCollectorBindSocketDir != "" {
+		// Override StatsFlushInterval as 60 seconds (1 minute) to reduce number of metric flushes.
+		// Only perform this override if there is no custom configuration for stats sinks and flush interval.
+		if c.StatsFlushInterval == "" && args.StatsSinksJSON == "" {
+			args.StatsFlushInterval = "60s"
+		}
 		appendTelemetryCollectorConfig(args, c.TelemetryCollectorBindSocketDir)
 	}
 

command/connect/envoy/bootstrap_config_test.go

@@ -5,6 +5,7 @@ package envoy
 
 import (
 	"encoding/json"
+	"fmt"
 	"reflect"
 	"regexp"
 	"strings"
@@ -628,47 +629,50 @@ func TestBootstrapConfig_ConfigureArgs(t *testing.T) {
 				TelemetryCollectorBindSocketDir: "/tmp/consul/telemetry-collector",
 			},
 			wantArgs: BootstrapTplArgs{
+				StatsFlushInterval: "60s",
 				ProxyID:            "web-sidecar-proxy",
 				StatsConfigJSON:    defaultStatsConfigJSON,
-				StatsSinksJSON: `{
+				StatsSinksJSON:     expectedTelemetryCollectorStatsSink,
-					"name": "envoy.stat_sinks.metrics_service",
+				StaticClustersJSON: expectedTelemetryCollectorCluster,
-					"typed_config": {
-					  "@type": "type.googleapis.com/envoy.config.metrics.v3.MetricsServiceConfig",
-					  "transport_api_version": "V3",
-					  "grpc_service": {
-						"envoy_grpc": {
-						  "cluster_name": "consul_telemetry_collector_loopback"
-						}
-					  },
-					  "emit_tags_as_labels": true
-					}
-				  }`,
-				StaticClustersJSON: `{
-					"name": "consul_telemetry_collector_loopback",
-					"type": "STATIC",
-					"http2_protocol_options": {},
-					"loadAssignment": {
-					  "clusterName": "consul_telemetry_collector_loopback",
-					  "endpoints": [
-						{
-						  "lbEndpoints": [
-							{
-							  "endpoint": {
-								"address": {
-								  "pipe": {
-									"path": "/tmp/consul/telemetry-collector/gqmuzdHCUPAEY5mbF8vgkZCNI14.sock"
-								  }
-								}
-							  }
-							}
-						  ]
-						}
-					  ]
-					}
-				  }`,
 			},
 			wantErr: false,
 		},
+		{
+			name: "telemetry-collector-no-default-flush-interval-when-interval-preconfigured",
+			baseArgs: BootstrapTplArgs{
+				ProxyID: "web-sidecar-proxy",
+			},
+			input: BootstrapConfig{
+				// Explicitly defined StatsFlushInterval by end user should not be overriden.
+				StatsFlushInterval:              "10s",
+				TelemetryCollectorBindSocketDir: "/tmp/consul/telemetry-collector",
+			},
+			wantArgs: BootstrapTplArgs{
+				StatsFlushInterval: "10s",
+				ProxyID:            "web-sidecar-proxy",
+				StatsConfigJSON:    defaultStatsConfigJSON,
+				StatsSinksJSON:     expectedTelemetryCollectorStatsSink,
+				StaticClustersJSON: expectedTelemetryCollectorCluster,
+			},
+		},
+		{
+			name: "telemetry-collector-no-default-flush-interval-when-sinks-preconfigured",
+			baseArgs: BootstrapTplArgs{
+				ProxyID: "web-sidecar-proxy",
+			},
+			input: BootstrapConfig{
+				// If stats sinks are explicitly defined by end user, do not default StatsFlushInterval.
+				StatsdURL:                       "udp://127.0.0.1:9125",
+				TelemetryCollectorBindSocketDir: "/tmp/consul/telemetry-collector",
+			},
+			wantArgs: BootstrapTplArgs{
+				StatsFlushInterval: "",
+				ProxyID:            "web-sidecar-proxy",
+				StatsConfigJSON:    defaultStatsConfigJSON,
+				StatsSinksJSON:     fmt.Sprintf(`%s,%s`, expectedStatsdSink, expectedTelemetryCollectorStatsSink),
+				StaticClustersJSON: expectedTelemetryCollectorCluster,
+			},
+		},
 		{
 			name: "simple-statsd-sink",
 			input: BootstrapConfig{

command/connect/envoy/testdata/telemetry-collector.golden

@@ -219,6 +219,7 @@
     ],
     "use_all_default_tags": true
   },
+  "stats_flush_interval": "60s",
   "dynamic_resources": {
     "lds_config": {
       "ads": {},

website/content/docs/connect/proxies/envoy.mdx

@@ -194,6 +194,7 @@ the [`sidecar_service`](/consul/docs/connect/proxies/deploy-sidecar-services) bl
 - `envoy_telemetry_collector_bind_socket_dir` - Specifies the directory where Envoy creates a Unix socket.
   Envoy sends metrics to the socket where a Consul telemetry collector can collect them.
   The socket is not configured by default. 
+  Enabling this sets Envoy's [`stats_flush_interval`](https://www.envoyproxy.io/docs/envoy/v1.17.2/api-v3/config/bootstrap/v3/bootstrap.proto#envoy-v3-api-field-config-bootstrap-v3-bootstrap-stats-flush-interval) to one minute if `envoy_stats_flush_interval` is unset and if no other stats sinks are configured, like `envoy_dogstats_url`, for instance.
 
 The [Advanced Configuration](#advanced-configuration) section describes additional configurations that allow incremental or complete control over the bootstrap configuration generated.