From ec1d60768cfc3764487058c33ac28747894887ce Mon Sep 17 00:00:00 2001 From: "Pavan Sai Vasireddy (Vasi)" Date: Mon, 27 Jun 2022 11:05:40 -0700 Subject: [PATCH 1/6] Pod Detection changes --- internal/aws/containerinsight/const.go | 3 +++ .../cadvisor/container_info_processor.go | 21 +++++++++++++------ .../cadvisor/extractors/cpu_extractor.go | 4 +++- .../internal/cadvisor/extractors/extractor.go | 8 +++---- .../cadvisor/extractors/fs_extractor.go | 3 ++- .../cadvisor/extractors/mem_extractor.go | 3 ++- .../cadvisor/extractors/net_extractor.go | 7 ++++++- 7 files changed, 35 insertions(+), 14 deletions(-) diff --git a/internal/aws/containerinsight/const.go b/internal/aws/containerinsight/const.go index 21409a340b91f..b7ad2e7bc7a26 100644 --- a/internal/aws/containerinsight/const.go +++ b/internal/aws/containerinsight/const.go @@ -121,6 +121,9 @@ const ( TypeContainer = "Container" TypeContainerFS = "ContainerFS" TypeContainerDiskIO = "ContainerDiskIO" + // Special type for pause container + // because containerd does not set container name pause container name to POD like docker does. + TypeInfraContainer = "InfraContainer" //unit UnitBytes = "Bytes" diff --git a/receiver/awscontainerinsightreceiver/internal/cadvisor/container_info_processor.go b/receiver/awscontainerinsightreceiver/internal/cadvisor/container_info_processor.go index 0b7c9f3cdf8e5..c502c3d6d5459 100644 --- a/receiver/awscontainerinsightreceiver/internal/cadvisor/container_info_processor.go +++ b/receiver/awscontainerinsightreceiver/internal/cadvisor/container_info_processor.go @@ -117,7 +117,9 @@ func processContainer(info *cInfo.ContainerInfo, mInfo extractors.CPUMemInfoProv namespace := info.Spec.Labels[namespaceLabel] podName := info.Spec.Labels[podNameLabel] podID := info.Spec.Labels[podIDLabel] - if containerName == "" || namespace == "" || podName == "" { + // NOTE: containerName can be empty for pause container on containerd + // https://github.com/containerd/cri/issues/922#issuecomment-423729537 + if namespace == "" || podName == "" { logger.Debug("Container labels are missing", zap.String("containerName", containerName), zap.String("namespace", namespace), @@ -136,16 +138,23 @@ func processContainer(info *cInfo.ContainerInfo, mInfo extractors.CPUMemInfoProv tags[ci.PodIDKey] = podID tags[ci.K8sPodNameKey] = podName tags[ci.K8sNamespace] = namespace - if containerName != infraContainerName { + switch containerName { + // For docker, pause container name is set to POD while containerd does not set it. + // See https://github.com/aws/amazon-cloudwatch-agent/issues/188 + case "", infraContainerName: + // NOTE: the pod here is only used by NetMetricExtractor, + // other pod info like CPU, Mem are dealt within in processPod. + containerType = TypeInfraContainer + default: tags[ci.ContainerNamekey] = containerName containerID := path.Base(info.Name) tags[ci.ContainerIDkey] = containerID pKey.containerIds = []string{containerID} containerType = ci.TypeContainer - } else { - // NOTE: the pod here is only used by NetMetricExtractor, - // other pod info like CPU, Mem are dealt within in processPod. - containerType = ci.TypePod + // TODO(pvasir): wait for upstream fix https://github.com/google/cadvisor/issues/2785 + if !info.Spec.HasFilesystem { + log.Printf("D! containerd does not have container filesystem metrics from cadvisor, See https://github.com/aws/amazon-cloudwatch-agent/issues/192") + } } } else { containerType = ci.TypeNode diff --git a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/cpu_extractor.go b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/cpu_extractor.go index a6638f68e73c8..034ed34a42806 100644 --- a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/cpu_extractor.go +++ b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/cpu_extractor.go @@ -37,13 +37,15 @@ func (c *CPUMetricExtractor) HasValue(info *cInfo.ContainerInfo) bool { func (c *CPUMetricExtractor) GetValue(info *cInfo.ContainerInfo, mInfo CPUMemInfoProvider, containerType string) []*CAdvisorMetric { var metrics []*CAdvisorMetric - if info.Spec.Labels[containerNameLable] == infraContainerName { + // Skip infra container and handle node, pod, other containers in pod + if containerType == ci.TypeInfraContainer { return metrics } // When there is more than one stats point, always use the last one curStats := GetStats(info) metric := newCadvisorMetric(containerType, c.logger) + metric.cgroupPath = info.Name multiplier := float64(decimalToMillicores) assignRateValueToField(&c.rateCalculator, metric.fields, ci.MetricName(containerType, ci.CPUTotal), info.Name, float64(curStats.Cpu.Usage.Total), curStats.Timestamp, multiplier) assignRateValueToField(&c.rateCalculator, metric.fields, ci.MetricName(containerType, ci.CPUUser), info.Name, float64(curStats.Cpu.Usage.User), curStats.Timestamp, multiplier) diff --git a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go index b583373f812ea..01c3748642273 100644 --- a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go +++ b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go @@ -27,9 +27,7 @@ import ( ) const ( - containerNameLable = "io.kubernetes.container.name" - // TODO: https://github.com/containerd/cri/issues/922#issuecomment-423729537 the container name can be empty on containerd - infraContainerName = "POD" + containerNameLabel = "io.kubernetes.container.name" ) func GetStats(info *cinfo.ContainerInfo) *cinfo.ContainerStats { @@ -47,10 +45,12 @@ type CPUMemInfoProvider interface { type MetricExtractor interface { HasValue(*cinfo.ContainerInfo) bool - GetValue(*cinfo.ContainerInfo, CPUMemInfoProvider, string) []*CAdvisorMetric + GetValue(info *cinfo.ContainerInfo, CPUMemInfoProvider string) []*CAdvisorMetric } type CAdvisorMetric struct { + // source of the metric for debugging merge conflict + cgroupPath string //key/value pairs that are typed and contain the metric (numerical) data fields map[string]interface{} //key/value string pairs that are used to identify the metrics diff --git a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/fs_extractor.go b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/fs_extractor.go index c51d445a071df..e7153bca217dc 100644 --- a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/fs_extractor.go +++ b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/fs_extractor.go @@ -39,7 +39,7 @@ func (f *FileSystemMetricExtractor) HasValue(info *cinfo.ContainerInfo) bool { func (f *FileSystemMetricExtractor) GetValue(info *cinfo.ContainerInfo, _ CPUMemInfoProvider, containerType string) []*CAdvisorMetric { var metrics []*CAdvisorMetric - if containerType == ci.TypePod || info.Spec.Labels[containerNameLable] == infraContainerName { + if containerType == ci.TypePod || containerType == ci.TypeInfraContainer { return metrics } @@ -71,6 +71,7 @@ func (f *FileSystemMetricExtractor) GetValue(info *cinfo.ContainerInfo, _ CPUMem metric.fields[ci.MetricName(containerType, ci.FSInodesfree)] = v.InodesFree } + metric.cgroupPath = info.Name metrics = append(metrics, metric) } return metrics diff --git a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/mem_extractor.go b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/mem_extractor.go index 53d68435f8e5c..36e7f03b1c352 100644 --- a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/mem_extractor.go +++ b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/mem_extractor.go @@ -35,11 +35,12 @@ func (m *MemMetricExtractor) HasValue(info *cinfo.ContainerInfo) bool { func (m *MemMetricExtractor) GetValue(info *cinfo.ContainerInfo, mInfo CPUMemInfoProvider, containerType string) []*CAdvisorMetric { var metrics []*CAdvisorMetric - if info.Spec.Labels[containerNameLable] == infraContainerName { + if containerType == ci.TypeInfraContainer { return metrics } metric := newCadvisorMetric(containerType, m.logger) + metric.cgroupPath = info.Name curStats := GetStats(info) metric.fields[ci.MetricName(containerType, ci.MemUsage)] = curStats.Memory.Usage diff --git a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/net_extractor.go b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/net_extractor.go index 3ef70132ba9a9..8338439c3018e 100644 --- a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/net_extractor.go +++ b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/net_extractor.go @@ -46,10 +46,15 @@ func (n *NetMetricExtractor) GetValue(info *cinfo.ContainerInfo, _ CPUMemInfoPro var metrics []*CAdvisorMetric // Just a protection here, there is no Container level Net metrics - if (containerType == ci.TypePod && info.Spec.Labels[containerNameLable] != infraContainerName) || containerType == ci.TypeContainer { + if containerType == ci.TypePod || containerType == ci.TypeContainer { return metrics } + // Rename type to pod so the metric name prefix is pod_ + if containerType == ci.TypeInfraContainer { + containerType = ci.TypePod + } + curStats := GetStats(info) curIfceStats := getInterfacesStats(curStats) From 06db54e5a2f1f2dc5bdfc363f5bd3c7a23742038 Mon Sep 17 00:00:00 2001 From: "Pavan Sai Vasireddy (Vasi)" Date: Mon, 27 Jun 2022 20:16:00 -0700 Subject: [PATCH 2/6] Logger message --- .../internal/cadvisor/container_info_processor.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/receiver/awscontainerinsightreceiver/internal/cadvisor/container_info_processor.go b/receiver/awscontainerinsightreceiver/internal/cadvisor/container_info_processor.go index c502c3d6d5459..6bae8f75203d1 100644 --- a/receiver/awscontainerinsightreceiver/internal/cadvisor/container_info_processor.go +++ b/receiver/awscontainerinsightreceiver/internal/cadvisor/container_info_processor.go @@ -144,7 +144,7 @@ func processContainer(info *cInfo.ContainerInfo, mInfo extractors.CPUMemInfoProv case "", infraContainerName: // NOTE: the pod here is only used by NetMetricExtractor, // other pod info like CPU, Mem are dealt within in processPod. - containerType = TypeInfraContainer + containerType = ci.TypeInfraContainer default: tags[ci.ContainerNamekey] = containerName containerID := path.Base(info.Name) @@ -153,7 +153,7 @@ func processContainer(info *cInfo.ContainerInfo, mInfo extractors.CPUMemInfoProv containerType = ci.TypeContainer // TODO(pvasir): wait for upstream fix https://github.com/google/cadvisor/issues/2785 if !info.Spec.HasFilesystem { - log.Printf("D! containerd does not have container filesystem metrics from cadvisor, See https://github.com/aws/amazon-cloudwatch-agent/issues/192") + logger.Debug("D! containerd does not have container filesystem metrics from cadvisor, See https://github.com/google/cadvisor/issues/2785") } } } else { From 74447ff88370bd3373201fa077f5fd58ec9542e7 Mon Sep 17 00:00:00 2001 From: "Pavan Sai Vasireddy (Vasi)" Date: Mon, 27 Jun 2022 20:21:31 -0700 Subject: [PATCH 3/6] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 529d8a8178320..f7078435b7e97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,8 @@ - `jmxreceiver`: Add latest releases of jmx metrics gatherer & wildfly jar to supported jars hash list (#11134) - `rabbitmqreceiver`: Add integration test for rabbitmq receiver (#10865) - `transformprocessor`: Allow using trace_state with key-value struct (#11029) +- `awscontainerinsightsreciever`: Pod detection Logic to support k8's on containerd runtime (#11666) + ### 🧰 Bug fixes 🧰 From 26dec7dcf2bc1ac1dd87b6d411ee0559720ac9c0 Mon Sep 17 00:00:00 2001 From: "Pavan Sai Vasireddy (Vasi)" Date: Mon, 27 Jun 2022 21:20:19 -0700 Subject: [PATCH 4/6] ReadMe changes --- receiver/awscontainerinsightreceiver/README.md | 6 ++++++ .../internal/cadvisor/extractors/extractor.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/receiver/awscontainerinsightreceiver/README.md b/receiver/awscontainerinsightreceiver/README.md index 9d14eb864c57d..f4b45f599da57 100644 --- a/receiver/awscontainerinsightreceiver/README.md +++ b/receiver/awscontainerinsightreceiver/README.md @@ -265,6 +265,9 @@ spec: - name: varlibdocker mountPath: /var/lib/docker readOnly: true + - name: containerdsock + mountPath: /run/containerd/containerd.sock + readOnly: true - name: sys mountPath: /sys readOnly: true @@ -296,6 +299,9 @@ spec: - name: varlibdocker hostPath: path: /var/lib/docker + - name: containerdsock + hostPath: + path: /run/containerd/containerd.sock - name: sys hostPath: path: /sys diff --git a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go index 01c3748642273..e0a10e8fc4f2a 100644 --- a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go +++ b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go @@ -45,7 +45,7 @@ type CPUMemInfoProvider interface { type MetricExtractor interface { HasValue(*cinfo.ContainerInfo) bool - GetValue(info *cinfo.ContainerInfo, CPUMemInfoProvider string) []*CAdvisorMetric + GetValue(*cinfo.ContainerInfo, CPUMemInfoProvider, string) []*CAdvisorMetric } type CAdvisorMetric struct { From f1991c59cc853f412ac2f43d9bf96cb3368422c7 Mon Sep 17 00:00:00 2001 From: "Pavan Sai Vasireddy (Vasi)" Date: Mon, 27 Jun 2022 21:35:33 -0700 Subject: [PATCH 5/6] go fmt correction --- internal/aws/containerinsight/const.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/aws/containerinsight/const.go b/internal/aws/containerinsight/const.go index b7ad2e7bc7a26..e6a37b451608b 100644 --- a/internal/aws/containerinsight/const.go +++ b/internal/aws/containerinsight/const.go @@ -123,7 +123,7 @@ const ( TypeContainerDiskIO = "ContainerDiskIO" // Special type for pause container // because containerd does not set container name pause container name to POD like docker does. - TypeInfraContainer = "InfraContainer" + TypeInfraContainer = "InfraContainer" //unit UnitBytes = "Bytes" From 519cb91eed9809533b6f8d77b92f3b17c803424a Mon Sep 17 00:00:00 2001 From: "Pavan Sai Vasireddy (Vasi)" Date: Mon, 27 Jun 2022 22:59:40 -0700 Subject: [PATCH 6/6] remove unused constant --- .../internal/cadvisor/extractors/extractor.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go index e0a10e8fc4f2a..9761b216bd1a7 100644 --- a/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go +++ b/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors/extractor.go @@ -26,10 +26,6 @@ import ( awsmetrics "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/metrics" ) -const ( - containerNameLabel = "io.kubernetes.container.name" -) - func GetStats(info *cinfo.ContainerInfo) *cinfo.ContainerStats { if len(info.Stats) == 0 { return nil