opentelemetry-collector-contrib: [k8sclusterreceiver] Some metric units don't follow Otel semantic conventions

Describe the bug Some metrics don’t follow Otel semantic conventions

Steps to reproduce Nothing to reproduce. The problem lies in the specification of the receiver itself.

What did you expect to see? Metrics that count objects should use units with brackets {} and not 1

Example:

k8s.container.restarts ==> Unit: "{restarts}"
k8s.cronjob.active_jobs ==> Unit: "{jobs}"
k8s.job.active_pods ==> Unit: "{pods}"
etc.

What did you see instead? Many metrics specify unit 1 to report a count of objects:

k8s.container.restarts
k8s.cronjob.active_jobs
k8s.job.active_pods
etc.

Unit 1 must be used only for fractions and ratios.

What version did you use? Main branch

What config did you use? N/A

Environment N/A

Additional context N/A

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 24 (24 by maintainers)

Commits related to this issue

Remove issue #10553 from CHANGELOG — committed to crobert-1/opentelemetry-collector-contrib by crobert-1 a year ago
[receiver/k8scluster] Fix k8s.deployment metric units (#26255) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a featu... — committed to open-telemetry/opentelemetry-collector-contrib by povilasv 10 months ago
[receiver/k8scluster] Fix k8s.deployment metric units (#26255) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a featu... — committed to jorgeancal/opentelemetry-collector-contrib by povilasv 10 months ago
[k8sclusterreceiver] refactor metric units to follow Otel conventions (#26708) **Description:** Refactor some metric units to follow OTEL Semantic conventions **Link to tracking Issue:** https:/... — committed to open-telemetry/opentelemetry-collector-contrib by povilasv 9 months ago
[k8sclusterreceiver] refactor metric units to follow Otel conventions (#26708) **Description:** Refactor some metric units to follow OTEL Semantic conventions **Link to tracking Issue:** https:/... — committed to harishbohara11/opentelemetry-collector-contrib by povilasv 9 months ago
[k8sclusterreceiver] refactor k8s.container.restarts metric units (#27051) **Description:** Refactor k8s.container.restarts metric unit to be `{restart}` **Link to tracking Issue:** https://g... — committed to open-telemetry/opentelemetry-collector-contrib by povilasv 9 months ago
[mdatagen] allow to skip metric unit validation (#27090) **Description:** <Describe what has changed.> Allows setting empty metric units in mdatagen. Example of using it https://github.com/ope... — committed to open-telemetry/opentelemetry-collector-contrib by povilasv 9 months ago
[k8sclusterreceiver] refactor metric units to follow Otel conventions (#26708) **Description:** Refactor some metric units to follow OTEL Semantic conventions **Link to tracking Issue:** https:/... — committed to jorgeancal/opentelemetry-collector-contrib by povilasv 9 months ago
[k8sclusterreceiver] refactor k8s.container.restarts metric units (#27051) **Description:** Refactor k8s.container.restarts metric unit to be `{restart}` **Link to tracking Issue:** https://g... — committed to jorgeancal/opentelemetry-collector-contrib by povilasv 9 months ago
[mdatagen] allow to skip metric unit validation (#27090) **Description:** <Describe what has changed.> Allows setting empty metric units in mdatagen. Example of using it https://github.com/ope... — committed to jorgeancal/opentelemetry-collector-contrib by povilasv 9 months ago
[k8sclusterreceiver] refactor state metrics to use empty units (#27091) **Description:** Change k8s.container.ready, k8s.pod.phase, k8s.pod.status_reason, k8s.namespace.phase units to empty *... — committed to open-telemetry/opentelemetry-collector-contrib by povilasv 9 months ago
[k8sclusterreceiver] change resourcequota and clusterquota units (#27662) change resourcequota and clusterquota metrics to use {resource} units. **Link to tracking Issue:** https://github.com/open... — committed to open-telemetry/opentelemetry-collector-contrib by povilasv 8 months ago
[k8sclusterreceiver] change resourcequota and clusterquota units (#27662) change resourcequota and clusterquota metrics to use {resource} units. **Link to tracking Issue:** https://github.com/open... — committed to openinsight-proj/opentelemetry-collector-contrib by povilasv 8 months ago
[k8sclusterreceiver] change resourcequota and clusterquota units (#27662) change resourcequota and clusterquota metrics to use {resource} units. **Link to tracking Issue:** https://github.com/open... — committed to carlossscastro/opentelemetry-collector-contrib by povilasv 8 months ago
[k8sclusterreceiver] refactor metric units to follow Otel conventions (#26708) **Description:** Refactor some metric units to follow OTEL Semantic conventions **Link to tracking Issue:** https:/... — committed to jmsnll/opentelemetry-collector-contrib by povilasv 9 months ago
[k8sclusterreceiver] refactor k8s.container.restarts metric units (#27051) **Description:** Refactor k8s.container.restarts metric unit to be `{restart}` **Link to tracking Issue:** https://g... — committed to jmsnll/opentelemetry-collector-contrib by povilasv 9 months ago
[mdatagen] allow to skip metric unit validation (#27090) **Description:** <Describe what has changed.> Allows setting empty metric units in mdatagen. Example of using it https://github.com/ope... — committed to jmsnll/opentelemetry-collector-contrib by povilasv 9 months ago
[k8sclusterreceiver] refactor state metrics to use empty units (#27091) **Description:** Change k8s.container.ready, k8s.pod.phase, k8s.pod.status_reason, k8s.namespace.phase units to empty *... — committed to jmsnll/opentelemetry-collector-contrib by povilasv 9 months ago
[k8sclusterreceiver] change resourcequota and clusterquota units (#27662) change resourcequota and clusterquota metrics to use {resource} units. **Link to tracking Issue:** https://github.com/open... — committed to jmsnll/opentelemetry-collector-contrib by povilasv 8 months ago

Most upvoted comments

I’ve looked thru some mores docs and Otel Metrics API says that Instrument units are optional:

An optional unit of measure … A unit of measure.

Users can provide a unit, but it is up to their discretion. Therefore, this API needs to be structured to accept a unit, but > MUST NOT obligate a user to provide one.

So I think we should allow empty units.

I opened PR which allows to set units: "", but still fails validation if “unit” field is not set -> https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/27090

povilasv on Sep 26, 2023

I believe this can be closed.

povilasv on Dec 18, 2023

Updated to use {resource} as unit. Will be released in next version.

I believe we are done with this task, all metrics seems to follow Otel semantic conventions.

@bertysentry feel free to validate and let me know if something isn’t right.

povilasv on Oct 18, 2023

@povilasv You could use either {resources} or empty unit.

bertysentry on Oct 11, 2023

I’ve looked thru some mores docs and Otel Metrics API says that Instrument units are optional:

An optional unit of measure … A unit of measure. Users can provide a unit, but it is up to their discretion. Therefore, this API needs to be structured to accept a unit, but > MUST NOT obligate a user to provide one.

So I think we should allow empty units.

Thanks for looking this up. Taking a quick look at opentelemetry-go it seems like the unit is left empty if not specified by the user so I agree we can leave this empty.

mx-psi on Sep 26, 2023

Considering these 2 points, I strongly recommend we don’t specify a unit where it’s not required by UCUM. If we put 1 for state sets and enumerations, we’ll break the Otel-to-Prometheus conversion, which is a large part of our ecosystem.

This makes sense to me.

Potentially we can tweek the mdatagen to explicitly require unit field while allowing it to be ""

dmitryax on Sep 25, 2023