Always publish Service Connect Metrics (#3786)

agent/stats/engine.go

@@ -461,6 +461,9 @@ func (engine *DockerStatsEngine) StartMetricsPublish() {
 		select {
 		case <-engine.publishMetricsTicker.C:
 			seelog.Debugf("publishMetricsTicker triggered. Sending telemetry messages to tcsClient through channel")
+			if includeServiceConnectStats {
+				seelog.Debugf("service connect metrics included")
+			}
 			go engine.publishMetrics(includeServiceConnectStats)
 			go engine.publishHealth()
 		case <-engine.ctx.Done():
@@ -543,16 +546,22 @@ func (engine *DockerStatsEngine) GetInstanceMetrics(includeServiceConnectStats b
 		containerMetrics, err := engine.taskContainerMetricsUnsafe(taskArn)
 		if err != nil {
 			seelog.Debugf("Error getting container metrics for task: %s, err: %v", taskArn, err)
-			// skip collecting service connect related metrics, if task is not service connect enabled
-			if !isServiceConnectTask {
+			// skip collecting service connect related metrics, if task is not service connect enabled.
+			// when task metrics and health metrics are both disabled and there is a service connect task,
+			// and we should not include service connect this time, we also need to skip following execution
+			// to avoid invalid metrics sent to TCS
+			if !isServiceConnectTask || !includeServiceConnectStats {
 				continue
 			}
 		}
 
 		if len(containerMetrics) == 0 {
 			seelog.Debugf("Empty containerMetrics for task, ignoring, task: %s", taskArn)
-			// skip collecting service connect related metrics, if task is not service connect enabled
-			if !isServiceConnectTask {
+			// skip collecting service connect related metrics, if task is not service connect enabled.
+			// when task metrics and health metrics are both disabled and there is a service connect task,
+			// and we should not include service connect this time, we also need to skip following execution
+			// to avoid invalid metrics sent to TCS
+			if !isServiceConnectTask || !includeServiceConnectStats {
 				continue
 			}
 		}
@@ -575,6 +584,7 @@ func (engine *DockerStatsEngine) GetInstanceMetrics(includeServiceConnectStats b
 			if serviceConnectStats, ok := engine.taskToServiceConnectStats[taskArn]; ok {
 				if !serviceConnectStats.HasStatsBeenSent() {
 					taskMetric.ServiceConnectMetricsWrapper = serviceConnectStats.GetStats()
+					seelog.Debugf("Adding service connect stats for task : %s", taskArn)
 					serviceConnectStats.SetStatsSent(true)
 				}
 			}
@@ -584,6 +594,7 @@ func (engine *DockerStatsEngine) GetInstanceMetrics(includeServiceConnectStats b
 
 	if len(taskMetrics) == 0 {
 		// Not idle. Expect taskMetrics to be there.
+		seelog.Debugf("Return empty metrics error")
 		return nil, nil, EmptyMetricsError
 	}
 

agent/stats/reporter/reporter.go

@@ -73,8 +73,7 @@ func NewDockerTelemetrySession(
 		return nil, cfgParseErr
 	}
 	if ok {
-		logger.Warn("Metrics were disabled, not starting the telemetry session")
-		return nil, nil
+		logger.Warn("Both metrics and health were disabled, but still starting the telemetry session for service connect metrics")
 	}
 
 	agentVersion, agentHash, containerRuntimeVersion := generateVersionInfo(taskEngine)

agent/stats/reporter/reporter_test.go

@@ -42,7 +42,7 @@ func TestNewDockerTelemetrySession(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 	mockEngine := mock_engine.NewMockTaskEngine(ctrl)
-	mockEngine.EXPECT().Version().Return(testDockerVersion, nil)
+	mockEngine.EXPECT().Version().Return(testDockerVersion, nil).AnyTimes()
 	testCases := []struct {
 		name            string
 		cfg             *config.Config
@@ -82,7 +82,7 @@ func TestNewDockerTelemetrySession(t *testing.T) {
 				AcceptInsecureCert: false,
 				DockerEndpoint:     testDockerEndpoint,
 			},
-			expectedSession: false,
+			expectedSession: true,
 			expectedError:   false,
 		},
 	}