Add kubernetes-cluster receiver #175

asuresh4 · 2020-04-14T15:54:28Z

Description

Add k8s_cluster receiver. This receiver monitors resources in a cluster and
collects metrics and metadata to correlate between resources. This receiver
watches for changes using the K8s API.

Key Data Structures

kubernetesReceiver: Sends metrics along the pipeline
resourceWatcher : Handles events from the K8s API, collecting metrics and
metadata from the cluster. This struct uses a informers.SharedInformerFactory
to watch for events.
dataCollector : Handles collection of data from Kubernetes resources. The
collector has a metricsStore to keep track of latest metrics representing the
cluster state and a metadataStore (a wrapper around cache.Store) to track
latest metadata from the cluster.

Workflow

resourceWatcher setup
- setup SharedInformerFactory, add event handler to informers. The following
  methods:onAdd, onUpdate and onDelete on the resourceWatcher handle
  resource creations, deletions and updates.
Event Handling
- Add/Update: On receiving an event corresponding to a resource creation or
  update, the latest metrics are collected by syncMetrics method on dataCollector.
  The collected metrics are cached in metricsStore. Methods responsible for
  collecting data from each supported Kubernetes resource type are prefixed with
  getMetricsFor. For example, getMetricsForPod collects metrics from Pod.
- Delete: On deletion of resources, the cached entry will be removed from metricsStore.
- Note that only metric collection is turned on right now. The metadata collection code
  is currently inactive (It is controlled by the collectMedata field).
Metric Syncing: Metrics from the metricStore are send along the pipeline once
every collection_interval seconds.
Metadata Syncing: TODO (The metadata collection code is inactive)

Testing

Unit tests for each resource type
Integration test for receiver
Manual testing with SignalFx exporter

Documentation

https://github.com/signalfx/opentelemetry-collector-contrib/blob/k8s-cluster/receiver/kubernetesclusterreceiver/README.md

jrcamp

Still working through it.

receiver/kubernetesclusterreceiver/README.md

receiver/kubernetesclusterreceiver/collector.go

receiver/kubernetesclusterreceiver/metricsstore.go

receiver/kubernetesclusterreceiver/watcher.go

receiver/kubernetesclusterreceiver/go.mod

receiver/k8sclusterreceiver/collector.go

jrcamp

Still working on it

receiver/k8sclusterreceiver/containers.go

receiver/k8sclusterreceiver/daemonsets.go

receiver/k8sclusterreceiver/deployments.go

receiver/k8sclusterreceiver/factory.go

tigrannajaryan

I did a shallow pass and left some comments.

Since this is pretty big chunk of code ported from SmartAgent please either point to the new code or break this down into smaller Prs.

tigrannajaryan · 2020-04-16T16:09:13Z

receiver/k8sclusterreceiver/README.md

+#### node_conditions_to_report
+
+The node conditions this receiver should report. See [here](https://kubernetes.io/docs/concepts/architecture/nodes/#condition) 
+for list of node conditions. If this option is set to `[Ready]`, a metrics called 


It would be useful to explain the format for this option. Is it an array? Can we list multiple values in the brackets? How are other condition names mapped to metric names?

Added an example depicting metrics created for a particular config value.

tigrannajaryan · 2020-04-16T16:09:20Z

receiver/k8sclusterreceiver/README.md

@@ -0,0 +1,215 @@
+# Kubernetes Cluster Receiver


receiver/k8sclusterreceiver/containers.go

tigrannajaryan · 2020-04-16T16:19:47Z

receiver/k8sclusterreceiver/collector.go

+)
+
+// TODO: Consider moving some of these constants to
+// https://github.com/open-telemetry/opentelemetry-collector/blob/master/translator/conventions/opentelemetry.go.


Yes. And we probably also need to make a proposal to add some of these constants to OpenTelemetry semantic conventions.

receiver/k8sclusterreceiver/collector.go

receiver/k8sclusterreceiver/metricsstore.go

receiver/k8sclusterreceiver/pods.go

tigrannajaryan · 2020-04-16T17:53:18Z

receiver/k8sclusterreceiver/receiver.go

+}
+
+func (kr *kubernetesReceiver) Shutdown(context.Context) error {
+	kr.cancel()


👍
Nice that cancellation is taken care of.

tigrannajaryan · 2020-04-16T17:53:23Z

receiver/k8sclusterreceiver/receiver.go

+
+func (kr *kubernetesReceiver) Start(ctx context.Context, host component.Host) error {
+	var c context.Context
+	c, kr.cancel = context.WithCancel(ctx)


jrcamp

Still working on it, couple more things I noticed.

receiver/k8sclusterreceiver/collection/metadata_test.go

receiver/k8sclusterreceiver/collection/metadatastore.go

receiver/k8sclusterreceiver/go.mod

asuresh4 · 2020-04-17T17:04:05Z

I did a shallow pass and left some comments.

Since this is pretty big chunk of code ported from SmartAgent please either point to the new code or break this down into smaller Prs.

I've moved all refactored code from SignalFx Agent to a package called collection. The new code is in the k8sclusterreceiver package and it uses collection.DataCollector to interact with the refactored collection logic. Let me know if that'll make it easier to review.

tigrannajaryan · 2020-04-20T21:04:31Z

receiver/k8sclusterreceiver/receiver.go

+
+func (kr *kubernetesReceiver) dispatchMetricData(ctx context.Context) {
+	for _, m := range kr.resourceWatcher.dataCollector.CollectMetricData() {
+		kr.consumer.ConsumeMetricsData(ctx, m)


This can return an error. It needs to be handled using obsreport. See for example prometheus receiver or opencensus receiver.
If we don't do this it will be silently ignored.

Updated to handle errors returned by ConsumeMetricsData. I noticed the other receivers look like the handling of these errors are done per ConsumeMetricsData call but in those cases, it also seems like the metrics are from the same source? For this receiver metrics originate from various sources, so I've clubbed errors from all ConsumeMetricsData calls in an interval. Let me know what you think.

Also, these internal metrics about number of timeseries and number of datapoints, are these send to the exporter too?

bogdandrutu · 2020-04-22T16:15:16Z

receiver/k8sclusterreceiver/collection/collector.go

+
+	// Resource labels for container.
+	containerKeyID       = "container.id"
+	containerKeySpecName = "container.spec.name"


This should be just "container.name". See https://github.com/open-telemetry/opentelemetry-specification/tree/master/specification/resource/semantic_conventions#container

This is the name of the container provided in a Pod spec. I don't think this will reflect the name of the the container that's actually running. If I understand correctly container.name should be the name of the actual running container, right?

Not sure I understand. Can you provide an example of the config + the name that will be create, something from real kubernetes.

bogdandrutu · 2020-04-22T16:18:22Z

receiver/k8sclusterreceiver/collection/collector.go

+	k8sKeyResourceQuotaName         = "k8s.resourcequota.name"
+
+	// Resource labels for container.
+	containerKeyID       = "container.id"


Is "container.id" a k8s concept or is this generic enough? Currently this is not defined in the semantic conventions.

It is generic as far as I know. I know Docker's got it and also CRI-O. I think we could include it in the convention.

Do you have any documentation?

receiver/k8sclusterreceiver/collection/pods_test.go

bogdandrutu · 2020-04-22T16:24:07Z

receiver/k8sclusterreceiver/collection/containers.go

+	containerKeyStatus       = "container.status"
+	containerKeyStatusReason = "container.status.reason"
+
+	// Values for container properties
+	containerStatusRunning    = "running"
+	containerStatusWaiting    = "waiting"
+	containerStatusTerminated = "terminated"


Should this be a metric or attributes in the resource/properties?

The container status would be part of the properties associated with a container.id. That way one could group containers by status while doing some analytics on container metrics.

Why? What is the rational to have that way vs a proper metric or event?

receiver/k8sclusterreceiver/collection/collector.go

bogdandrutu · 2020-04-22T16:31:07Z

receiver/k8sclusterreceiver/collection/metadata.go

+
+const (
+	// Keys for K8s properties
+	k8sKeyWorkLoad     = "k8s.workload"


This should be "k8s.workload.kind"? Also not sure where is this used, can you point me to where do we use this?

Based on the usage in this file this seems to be unnecessary, you do have the kind/type in the resourceIDKey.

Yes, k8s.worload.kind is better.

Will ping you offline with some details about these workload properties.

receiver/k8sclusterreceiver/README.md

receiver/k8sclusterreceiver/collection/metadata.go

bogdandrutu · 2020-04-22T16:38:41Z

receiver/k8sclusterreceiver/collection/metadata.go

+const (
+	// Keys for K8s properties
+	k8sKeyWorkLoad     = "k8s.workload"
+	k8sKeyWorkLoadName = "k8s.workload.name"


Should we use the k8s.{type}.name?

I think it is nice to have this key be resource type agnostic.

bogdandrutu · 2020-04-22T16:57:33Z

receiver/k8sclusterreceiver/collection/pods_test.go

+			"container.id":         "container-id",
+			"container.spec.name":  "container-name",
+			"container.image.name": "container-image-name",
+			"k8s.pod.uid":          "test-pod-1-uid",


It is very unclear to me when do we expect to see uid vs name in the resource. Why do we not have a k8s.node.uid in this resource? May worth adding an issue to clarify when to use one or the other.

Node information for a Pod is obtained from the Pod spec. In this case we don't add k8s.node.uid because it is not available in the Pod spec whereas the node name is available.

Where can I find what is available or not in the Pod Specs?

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podspec-v1-core

bogdandrutu · 2020-04-24T19:04:07Z

Let's file issues for the 3 remaining items and move forward with this PR. I don't want to block this work but it is good that we identify these inconsistencies.

bogdandrutu · 2020-04-24T19:05:28Z

Also we should revisit the metric names to be consistent with opentelemetry conventions which are not yet defined

tigrannajaryan · 2020-04-27T21:09:02Z

@bogdandrutu I am merging this since you appear to be OK (issues were created).

### Description Add `k8s_cluster` receiver. This receiver monitors resources in a cluster and collects metrics and metadata to correlate between resources. This receiver watches for changes using the K8s API. ##### Key Data Structures - `kubernetesReceiver`: Sends metrics along the pipeline - `resourceWatcher` : Handles events from the K8s API, collecting metrics and metadata from the cluster. This struct uses a `informers.SharedInformerFactory` to watch for events. - `dataCollector` : Handles collection of data from Kubernetes resources. The collector has a `metricsStore` to keep track of latest metrics representing the cluster state and a `metadataStore` (a wrapper around `cache.Store`) to track latest metadata from the cluster. ##### Workflow - **resourceWatcher setup** - setup SharedInformerFactory, add event handler to informers. The following methods:`onAdd`, `onUpdate` and `onDelete` on the `resourceWatcher` handle resource creations, deletions and updates. - **Event Handling** - _Add/Update_: On receiving an event corresponding to a resource creation or update, the latest metrics are collected by `syncMetrics` method on `dataCollector`. The collected metrics are cached in `metricsStore`. Methods responsible for collecting data from each supported Kubernetes resource type are prefixed with `getMetricsFor`. For example, `getMetricsForPod` collects metrics from Pod. - _Delete_: On deletion of resources, the cached entry will be removed from `metricsStore`. - Note that only metric collection is turned on right now. The metadata collection code is currently inactive (It is controlled by the `collectMedata` field). - **Metric Syncing**: Metrics from the `metricStore` are send along the pipeline once every `collection_interval` seconds. - **Metadata Syncing**: TODO (The metadata collection code is inactive) ### Testing - Unit tests for each resource type - Integration test for receiver - Manual testing with SignalFx exporter ### Documentation https://github.com/signalfx/opentelemetry-collector-contrib/blob/k8s-cluster/receiver/kubernetesclusterreceiver/README.md

…y-picked commits (open-telemetry#168)" (open-telemetry#175) This reverts commit c9246c3.

asuresh4 requested review from bogdandrutu, flands, owais, pjanotti, rghetia, songy23 and tigrannajaryan as code owners April 14, 2020 15:54

jrcamp reviewed Apr 14, 2020

View reviewed changes

receiver/kubernetesclusterreceiver/go.mod Outdated Show resolved Hide resolved

jrcamp reviewed Apr 14, 2020

View reviewed changes

receiver/k8sclusterreceiver/collector.go Outdated Show resolved Hide resolved

jrcamp reviewed Apr 15, 2020

View reviewed changes

tigrannajaryan reviewed Apr 16, 2020

View reviewed changes

jrcamp reviewed Apr 16, 2020

View reviewed changes

receiver/k8sclusterreceiver/collection/metadata_test.go Outdated Show resolved Hide resolved

receiver/k8sclusterreceiver/collection/metadatastore.go Outdated Show resolved Hide resolved

receiver/k8sclusterreceiver/go.mod Outdated Show resolved Hide resolved

asuresh4 mentioned this pull request Apr 20, 2020

Add ability to sync metadata to SignalFx from Kubernetes #180

Closed

asuresh4 requested a review from tigrannajaryan April 20, 2020 20:33

jrcamp approved these changes Apr 20, 2020

View reviewed changes

tigrannajaryan reviewed Apr 20, 2020

View reviewed changes

bogdandrutu reviewed Apr 22, 2020

View reviewed changes

bogdandrutu approved these changes Apr 24, 2020

View reviewed changes

This was referenced Apr 24, 2020

Effectively handle Kubernetes container spec name in naming convention #188

Closed

Finalize whether container status should be a metric or part of resource metadata #189

Closed

Revisit association of workloads and pods in k8s_cluster receiver #190

Closed

asuresh4 added 4 commits April 27, 2020 11:18

Add kubernetes-cluster receiver

0ffb11f

Tweak failing load test

b05e3bc

Rename kubernetesclusterreceiver to k8sclusterreceiver

b3b8c97

Address feedback

fc8f81a

asuresh4 added 16 commits April 27, 2020 11:18

Do not watch services

af0c317

Add more tests

8e2fe83

Refactor based on feedback

e0ea83c

Tweaks based on feedback

b589b70

Use dimensionalized container resource metrics

bffc3a9

Move Smart Agent refactored code to separate package

ad6a6d5

Switch to using otel constants

c2d4fcc

Address feedback

b11614b

Import correct module

7fdc822

Add issue to track metadata sync

dee7c77

Add more tests

6480247

Refactor metadatastore based on feedback

f49a10c

Setup informer for services

cb8b79b

Handle errors from ConsumeMetricsData calls

c04c5c1

Address feedback

dc09206

Incorporate latest updates

4cda15e

tigrannajaryan merged commit 9ee65b1 into open-telemetry:master Apr 27, 2020

asuresh4 deleted the k8s-cluster branch April 29, 2020 20:16

asuresh4 mentioned this pull request May 28, 2020

REQUEST: New membership for asuresh4 open-telemetry/community#358

Closed

6 tasks

bogdandrutu pushed a commit that referenced this pull request May 12, 2022

Fix codecov configuration (#175)

9050cb7

bjrara pushed a commit to bjrara/opentelemetry-collector-contrib that referenced this pull request Apr 3, 2024

Revert "Merge Container Insights on Windows to aws-cwa-dev with cherr…

d6cc539

…y-picked commits (open-telemetry#168)" (open-telemetry#175) This reverts commit c9246c3.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add kubernetes-cluster receiver #175

Add kubernetes-cluster receiver #175

asuresh4 commented Apr 14, 2020

jrcamp left a comment

jrcamp left a comment

tigrannajaryan left a comment

tigrannajaryan Apr 16, 2020

asuresh4 Apr 17, 2020

tigrannajaryan Apr 16, 2020

tigrannajaryan Apr 16, 2020

tigrannajaryan Apr 16, 2020

tigrannajaryan Apr 16, 2020

jrcamp left a comment

asuresh4 commented Apr 17, 2020

tigrannajaryan Apr 20, 2020

asuresh4 Apr 21, 2020

bogdandrutu Apr 22, 2020

asuresh4 Apr 23, 2020

bogdandrutu Apr 24, 2020

bogdandrutu Apr 22, 2020

asuresh4 Apr 23, 2020 •

edited

Loading

bogdandrutu Apr 24, 2020

bogdandrutu Apr 22, 2020

asuresh4 Apr 23, 2020

bogdandrutu Apr 24, 2020 •

edited

Loading

bogdandrutu Apr 22, 2020

asuresh4 Apr 23, 2020

bogdandrutu Apr 22, 2020

asuresh4 Apr 23, 2020

bogdandrutu Apr 22, 2020

asuresh4 Apr 23, 2020

bogdandrutu Apr 24, 2020

asuresh4 Apr 24, 2020

bogdandrutu commented Apr 24, 2020

bogdandrutu commented Apr 24, 2020

tigrannajaryan commented Apr 27, 2020

Add kubernetes-cluster receiver #175

Add kubernetes-cluster receiver #175

Conversation

asuresh4 commented Apr 14, 2020

Description

Key Data Structures

Workflow

Testing

Documentation

jrcamp left a comment

Choose a reason for hiding this comment

jrcamp left a comment

Choose a reason for hiding this comment

tigrannajaryan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrcamp left a comment

Choose a reason for hiding this comment

asuresh4 commented Apr 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asuresh4 Apr 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bogdandrutu Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bogdandrutu commented Apr 24, 2020

bogdandrutu commented Apr 24, 2020

tigrannajaryan commented Apr 27, 2020

asuresh4 Apr 23, 2020 •

edited

Loading

bogdandrutu Apr 24, 2020 •

edited

Loading