Skip to content

Conversation

@odubajDT
Copy link
Contributor

@odubajDT odubajDT commented Sep 9, 2025

Description

Sync allocatable metric names with the latest semconv. This change is being handled with a feature gate, so users have a clean transition phase

Link to tracking issue

Fixes #40708

Documentation

README

@odubajDT odubajDT marked this pull request as ready for review September 9, 2025 09:00
@github-actions github-actions bot requested a review from povilasv September 9, 2025 09:00
## Feature Gates
### `k8scluster.allocatableNamespace.enabled`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank's @odubajDT for pushing this forward!

Shall we start using a generic feature gate for all these changes that aim to align the k8s components with the k8s semantic conventions?
We can start using the feature gate described at https://opentelemetry.io/docs/specs/semconv/non-normative/k8s-migration/ even though the Semantic Conventions are not stable yet.

@dmitryax @TylerHelmuth WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A global feature gate may potentially enable features, that users do not want to enable, therefore I would rather stick to the concept of having a feature gate per feature, but I am opened for opinions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here is to have a single feature gate for porting SemConv to Collector k8s components, i.e. semconv.k8s.enableStable. In this we handle SemConv porting with one single gate making the migration easier. At least that's the plan we have defined from SemConv's stability groups and documented at https://opentelemetry.io/docs/specs/semconv/non-normative/k8s-migration/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand, but if multiple features in multiple different components are enabled/disabled by a single feature gate, it may cause problems for the users. For example: User want to enable feature X in component Z, but with the feature gate, also feature B in component A is enabled. Therefore user enables behavior, that his infrastructure is not prepared yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feature gate semconv.k8s.enableStable will only enable stable Semantic Conventions for k8s across the k8s components. Can this be considered a different feature? In any case the counterpart of this feature gate will be the semconv.k8s.enableLegacy and there will be a period where the dual scheme will be needed for a smooth migration. Metrics can always be switched on/off individually.

I think this will give us less overhead during the migration period, because having to maintain multiple feature gates will be a pain both from code perspective but also for the users that would need to handle them differently in their configurations.

However, I'd be open to hear if we can do better than what the decided single feature gate suggests. Also interested to hear what @dmitryax @TylerHelmuth @braydonk think on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrsMark does it mean that semconv.k8s.disableLegacy and semconv.k8s.enableLegacy will be applied to all k8s related receivers including kubeletstats?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial thought is to have feature gates per component, but I'm ok if we want a collector-wide "use the stable k8s semantic convention" feature gate. Also, should we move this topic to an issue? It will be nice to have an official "this is where we decided how to handle the transition to stable k8s semantic conventions" issue to point to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrsMark does it mean that semconv.k8s.disableLegacy and semconv.k8s.enableLegacy will be applied to all k8s related receivers including kubeletstats?

If we go with these yes those will be for all components that use the k8s semconv. Do you see any issues with that? Per component would also be fine and probably more flexible? I don't have any strong preference here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial thought is to have feature gates per component, but I'm ok if we want a collector-wide "use the stable k8s semantic convention" feature gate. Also, should we move this topic to an issue? It will be nice to have an official "this is where we decided how to handle the transition to stable k8s semantic conventions" issue to point to.

We have the following two, per component, for tracking this but they are not really updated:

Should we also create a "global" k8s related adding them as sub-issues?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #43869

@odubajDT odubajDT marked this pull request as draft September 9, 2025 10:00
@odubajDT odubajDT marked this pull request as ready for review September 9, 2025 11:27
@odubajDT odubajDT force-pushed the k8scluster-allocatable branch 2 times, most recently from 93eaf01 to 894ee22 Compare September 12, 2025 04:13
@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2025

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 9, 2025
@odubajDT
Copy link
Contributor Author

odubajDT commented Oct 9, 2025

not stale

@odubajDT odubajDT removed the Stale label Oct 9, 2025
@odubajDT odubajDT changed the title [receiver/k8scluster] Sync allocatable metric names with the latest semconv [receiver/k8s_cluster] Sync allocatable metric names with the latest semconv Oct 21, 2025
enabled: true
k8s.node.allocatable.memory:
enabled: true
k8s.node.allocatable.pods:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the back and forth here. This is going to change again with open-telemetry/semantic-conventions#2822 because it needs to be aligned with SemConv project. I think the rest of the metrics should change too i.e. k8s.node.cpu.allocatable (I'll send a follow-up PR after). In this, I suggest we wait until we have the semconv part ready, which hopefully will be soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for taking care of it!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, this PR should be adapted

Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before moving on with this we need to clarify 2 things at #43869.

This PR stands as a very useful example to validate the process, thank's @odubajDT!

After thinking of it again and having in mind the next the steps of the K8s SemConv SIG, my early suggestion is that going with multiple feature gates per metrics' group will be our best bet. This will allow us having even alpha/development semconv groups already introduced in the Collector behind their own feature gates pair. But let's move the discussion at #43869.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Nov 13, 2025
@odubajDT odubajDT removed the Stale label Nov 13, 2025
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Nov 28, 2025
@odubajDT
Copy link
Contributor Author

not stale

@github-actions github-actions bot removed the Stale label Nov 29, 2025
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 13, 2025
@odubajDT odubajDT added never stale Issues marked with this label will be never staled and automatically removed and removed Stale labels Dec 15, 2025
@odubajDT odubajDT force-pushed the k8scluster-allocatable branch from 404fd53 to b02639e Compare January 9, 2026 06:55
@odubajDT
Copy link
Contributor Author

odubajDT commented Jan 9, 2026

Hey @ChrsMark this PR should be ready for review. Adapted the metric names and also the feature gate pair according to semconv. Hope I didn't miss anything!

Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #45307 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

never stale Issues marked with this label will be never staled and automatically removed receiver/k8scluster waiting-for-code-owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[receiver/k8scluster] Sync allocatable metric names with the latest semconv

7 participants