Add metrics for deployment, replicaset, replication_controller, statefulset and hpa #1636

ChrsMark · 2024-11-29T15:07:26Z

Changes

Part of #1032.

Fixes #1644
Fixes #1637

Adds the following metrics:

k8s.deployment.desired_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.deployment.available_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.replicaset.desired_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.replicaset.available_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.replication_controller.desired_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.replication_controller.available_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.statefulset.desired_pods (emitted by collector) [ ✅ not a breaking change for the Collector]
k8s.statefulset.ready_pods (emitted by collector) [ ✅ not a breaking change for the Collector]
k8s.statefulset.current_pods (emitted by collector) [ ✅ not a breaking change for the Collector]
k8s.statefulset.updated_pods (emitted by collector) [ ✅ not a breaking change for the Collector]
k8s.hpa.desired_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.hpa.current_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.hpa.max_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.hpa.min_pods (emitted by collector) [⚠️ BREAKING change for the Collector]

These metrics have been in use by the Opentelemetry Collector and specifically the k8scluster receiver component.

Breaking changes between the Collector and the Semconv will be listed in the k8s migration guide once #1636 is merged.

Merge requirement checklist

CONTRIBUTING.md guidelines followed.
Change log entry added, according to the guidelines in When to add a changelog entry.
- If your PR does not need a change log, start the PR title with [chore]
schema-next.yaml updated with changes to existing conventions.

ChrsMark · 2024-12-03T12:54:31Z

@open-telemetry/specs-semconv-approvers should the .count suffix be applied here based on the guideline from

semantic-conventions/docs/general/metrics.md

Line 148 in 2d34907

#### Use `count` Instead of Pluralization for UpDownCounters

?

We don't use a pluralized metric name here but I want to double check if this rule should be taken into account.

It would be preferable if we define this metrics as is to minimize breaking changes of the Collector.

model/k8s/metrics.yaml

docs/system/k8s-metrics.md

ChrsMark · 2024-12-03T14:55:01Z

I added the replication_controller metrics in this PR since the names are already the same and group well here.

In any case, we need to have this aligned with #1637 and #1644.

dmitryax · 2024-12-03T21:02:53Z

Taking back my approval until discussion in #1637 (comment) is resolved

ChrsMark · 2024-12-04T11:03:57Z

I have updated the PR to include the statefulset and hpa metrics since those are related to the consistency discussion (#1637 (comment)).

If want to achieve consistency while also being semantically correct we either need to use the _pods or the _replicas suffix. I chose to use the _pods suffix as more "user friendly".

Providing bellow the current state (copied from the PR's description):

k8s.deployment.desired_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.deployment.available_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.replicaset.desired_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.replicaset.available_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.replication_controller.desired_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.replication_controller.available_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.statefulset.desired_pods (emitted by collector) [ ✅ not a breaking change for the Collector]
k8s.statefulset.ready_pods (emitted by collector) [ ✅ not a breaking change for the Collector]
k8s.statefulset.current_pods (emitted by collector) [ ✅ not a breaking change for the Collector]
k8s.statefulset.updated_pods (emitted by collector) [ ✅ not a breaking change for the Collector]
k8s.hpa.desired_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.hpa.current_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.hpa.max_pods (emitted by collector) [⚠️ BREAKING change for the Collector]
k8s.hpa.min_pods (emitted by collector) [⚠️ BREAKING change for the Collector]

Breaking changes between the Collector and the Semconv will be listed in the k8s migration guide once #1636 is merged.

model/k8s/metrics.yaml

docs/system/k8s-metrics.md

jinja2 · 2024-12-11T14:31:01Z

We have a few minor discrepancies in resource/metric names. While I don't think we need to change any existing names, I would like to note them down for future reference.

Short/Full names for k8s type - HPA seems to be the only resource type that uses the short name. The full object kind can be a mouthful, but best to stick with the full names as long as they are a reasonable length.
k8s object kinds use PascalCase. Some resources mirror this with underscores in the attr/metric names (e.g. k8s.replication_controller for k8s obj ReplicationController), while others don't (e.g. k8s.statefulset for k8s obj StatefulSet). I think we should be consistent with the underscore convention.

jinja2 · 2024-12-11T15:58:54Z

I think this observation from @dashpole might be relevant to some of the metrics here. Most of these timeseries seem meaningfully additive

ChrsMark · 2024-12-11T16:37:26Z

I think this observation from @dashpole might be relevant to some of the metrics here. Most of these timeseries seem meaningfully additive

As discussed in the K8s SemConv WG meeting today, we agree on making the change to updowncounter for these metrics (I think this will also apply to #1649). I will update the PR(s) to reflect this.

jaronoff97

I appreciate that we moved from repliaca -> pods for the convention here. I think that change will make the user experience a bit simpler as well. Thank you!

ChrsMark · 2024-12-13T08:21:04Z

@open-telemetry/specs-semconv-maintainers could you also take a look please?

note: I'm suggesting we introduce the metrics as-is and follow-up for enhancing their descriptions more to cover their specific relations with resources.

Signed-off-by: ChrsMark <[email protected]>

Co-authored-by: Tetiana Kravchenko <[email protected]>

Signed-off-by: ChrsMark <[email protected]>

ChrsMark · 2024-12-14T14:07:55Z

I added resource correlation note per metric with 1f91666 according to the discussion at #1649.

ChrsMark requested a review from povilasv November 29, 2024 15:07

ChrsMark requested review from a team as code owners November 29, 2024 15:07

ChrsMark force-pushed the add_deploy_replica_metrics branch from b09d792 to be17329 Compare November 29, 2024 15:08

ChrsMark added the area:k8s label Nov 29, 2024

This was referenced Nov 29, 2024

Define semantic conventions for k8s metrics #1032

Open

Add metrics for k8s statefulset #1637

Closed

ChrsMark force-pushed the add_deploy_replica_metrics branch from 07e6fd4 to eee691b Compare December 3, 2024 12:55

tetianakravchenko reviewed Dec 3, 2024

View reviewed changes

model/k8s/metrics.yaml Show resolved Hide resolved

model/k8s/metrics.yaml Outdated Show resolved Hide resolved

tetianakravchenko approved these changes Dec 3, 2024

View reviewed changes

docs/system/k8s-metrics.md Outdated Show resolved Hide resolved

ChrsMark mentioned this pull request Dec 3, 2024

[k8s] Add hpa metrics #1644

Closed

ChrsMark changed the title ~~Add desired and available metrics for deployment and replicaset~~ Add desired and available metrics for deployment, replicaset and replication_controller Dec 3, 2024

jaronoff97 approved these changes Dec 3, 2024

View reviewed changes

dashpole approved these changes Dec 3, 2024

View reviewed changes

dmitryax approved these changes Dec 3, 2024

View reviewed changes

dmitryax self-requested a review December 3, 2024 21:02

ChrsMark marked this pull request as draft December 3, 2024 21:31

ChrsMark requested review from dashpole, jaronoff97 and tetianakravchenko December 4, 2024 10:46

ChrsMark marked this pull request as ready for review December 4, 2024 11:03

ChrsMark force-pushed the add_deploy_replica_metrics branch from bc46f76 to ce46ae4 Compare December 4, 2024 11:20

ChrsMark changed the title ~~Add desired and available metrics for deployment, replicaset and replication_controller~~ Add metrics for deployment, replicaset, replication_controller, statefulset and hpa Dec 4, 2024

tetianakravchenko reviewed Dec 4, 2024

View reviewed changes

model/k8s/metrics.yaml Outdated Show resolved Hide resolved

model/k8s/metrics.yaml Outdated Show resolved Hide resolved

model/k8s/metrics.yaml Outdated Show resolved Hide resolved

model/k8s/metrics.yaml Outdated Show resolved Hide resolved

jinja2 reviewed Dec 11, 2024

View reviewed changes

docs/system/k8s-metrics.md Outdated Show resolved Hide resolved

jinja2 approved these changes Dec 11, 2024

View reviewed changes

ChrsMark force-pushed the add_deploy_replica_metrics branch 2 times, most recently from 15d8278 to dbb71af Compare December 12, 2024 08:58

dashpole approved these changes Dec 12, 2024

View reviewed changes

jaronoff97 approved these changes Dec 12, 2024

View reviewed changes

ChrsMark mentioned this pull request Dec 13, 2024

Add k8s daemonset metrics #1649

Merged

3 tasks

lmolkova approved these changes Dec 14, 2024

View reviewed changes

ChrsMark and others added 11 commits December 14, 2024 16:06

add desired and available metrics for deployment and replicaset

0858512

Signed-off-by: ChrsMark <[email protected]>

add refs to k8s api

e78c683

Signed-off-by: ChrsMark <[email protected]>

Update model/k8s/metrics.yaml

6b79810

Co-authored-by: Tetiana Kravchenko <[email protected]>

Update model/k8s/metrics.yaml

b20bee7

Co-authored-by: Tetiana Kravchenko <[email protected]>

update md files

c3c4cd3

Signed-off-by: ChrsMark <[email protected]>

add replication_controller metrics

ec7ce4e

Signed-off-by: ChrsMark <[email protected]>

add more replica related metrics & add _pod suffix

aa85c2b

Signed-off-by: ChrsMark <[email protected]>

Add Collector breaking changes in migration guide

63d3e9b

Signed-off-by: ChrsMark <[email protected]>

add extra note about the rationale of this change

ea07ce8

Signed-off-by: ChrsMark <[email protected]>

switch types to updowncounter

11e412e

Signed-off-by: ChrsMark <[email protected]>

Add notes about resource correlation

1f91666

Signed-off-by: ChrsMark <[email protected]>

ChrsMark force-pushed the add_deploy_replica_metrics branch from 0beb344 to 1f91666 Compare December 14, 2024 14:07

Merge branch 'main' into add_deploy_replica_metrics

d7a2ad5

jsuereth approved these changes Dec 17, 2024

View reviewed changes

jsuereth enabled auto-merge (squash) December 17, 2024 15:27

Merge branch 'main' into add_deploy_replica_metrics

7894f45

jsuereth merged commit b60cdba into open-telemetry:main Dec 17, 2024
13 of 14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metrics for deployment, replicaset, replication_controller, statefulset and hpa #1636

Add metrics for deployment, replicaset, replication_controller, statefulset and hpa #1636

ChrsMark commented Nov 29, 2024 •

edited

Loading

ChrsMark commented Dec 3, 2024

ChrsMark commented Dec 3, 2024 •

edited

Loading

dmitryax commented Dec 3, 2024

ChrsMark commented Dec 4, 2024 •

edited

Loading

jinja2 commented Dec 11, 2024

jinja2 commented Dec 11, 2024

ChrsMark commented Dec 11, 2024

jaronoff97 left a comment

ChrsMark commented Dec 13, 2024

ChrsMark commented Dec 14, 2024

Add metrics for deployment, replicaset, replication_controller, statefulset and hpa #1636

Add metrics for deployment, replicaset, replication_controller, statefulset and hpa #1636

Conversation

ChrsMark commented Nov 29, 2024 • edited Loading

Changes

Merge requirement checklist

ChrsMark commented Dec 3, 2024

ChrsMark commented Dec 3, 2024 • edited Loading

dmitryax commented Dec 3, 2024

ChrsMark commented Dec 4, 2024 • edited Loading

jinja2 commented Dec 11, 2024

jinja2 commented Dec 11, 2024

ChrsMark commented Dec 11, 2024

jaronoff97 left a comment

Choose a reason for hiding this comment

ChrsMark commented Dec 13, 2024

ChrsMark commented Dec 14, 2024

ChrsMark commented Nov 29, 2024 •

edited

Loading

ChrsMark commented Dec 3, 2024 •

edited

Loading

ChrsMark commented Dec 4, 2024 •

edited

Loading