[k8s] Fix updowncounters to follow the non-pluralization rule

[ChrsMark]

Fixes #2301

Changes

This brings names in alignment with the pluralisation guidelines: https://github.com/open-telemetry/semantic-conventions/blob/v1.37.0/docs/general/naming.md#do-not-pluralize-updowncounter-names

Renamed metrics:

• k8s.node.allocatable.pods -> k8s.node.pod.allocatable
• k8s.daemonset.current_scheduled_nodes -> k8s.daemonset.node.current_scheduled
• k8s.daemonset.desired_scheduled_nodes -> k8s.daemonset.node.desired_scheduled
• k8s.daemonset.misscheduled_nodes -> k8s.daemonset.node.misscheduled
• k8s.daemonset.ready_nodes -> k8s.daemonset.node.ready
• k8s.job.active_pods -> k8s.job.pod.active
• k8s.job.failed_pods -> k8s.job.pod.failed
• k8s.job.successful_pods -> k8s.job.pod.successful
• k8s.job.desired_successful_pods -> k8s.job.pod.desired_successful
• k8s.job.max_parallel_pods -> k8s.job.pod.max_parallel
• k8s.cronjob.active_jobs -> k8s.cronjob.job.active
• k8s.deployment.desired_pods -> k8s.deployment.pod.desired
• k8s.deployment.available_pods -> k8s.deployment.pod.available
• k8s.replicaset.desired_pods -> k8s.replicaset.pod.desired
• k8s.replicaset.available_pods -> k8s.replicaset.pod.available
• k8s.replicationcontroller.desired_pods -> k8s.replicationcontroller.pod.desired
• k8s.replicationcontroller.available_pods -> k8s.replicationcontroller.pod.available
• k8s.statefulset.desired_pods -> k8s.statefulset.pod.desired
• k8s.statefulset.ready_pods -> k8s.statefulset.pod.ready
• k8s.statefulset.current_pods -> k8s.statefulset.pod.current
• k8s.statefulset.updated_pods -> k8s.statefulset.pod.updated
• k8s.hpa.desired_pods -> k8s.hpa.pod.desired
• k8s.hpa.current_pods -> k8s.hpa.pod.current
• k8s.hpa.max_pods -> k8s.hpa.pod.max
• k8s.hpa.min_pods -> k8s.hpa.pod.min

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]
• Links to the prototypes or existing instrumentations (when adding or changing conventions)

[AlexanderWert]

Should we maybe add k8s.replicationcontroller.desired_pods as an additional deprecated metric?
(as this is the second time this metric is being renamed)

Otherwise k8s.replicationcontroller.desired_pods wouldn't be mentioned anywhere as a deprecated metric with only the original (from the first rename) name being documented here (i.e. k8s.replication_controller.desired_pods).

[ChrsMark]

I had both metrics originally, but the validation failed cause it was pointing to a non-existing metric (the first rename). Not sure if this is expected or an unhandled corner-case of our tooling 🤔

[jsuereth]

I believe the tooling expects all the previous metric names to remain around.

Given these are not "stable" I think there's less enforcement around keeping things around, although it is risky depending on whether the intermediate name was used.

If you have a caveat asking folks NOT to migrate to new metrics past version "X" then you can drop the intermediary if it's between "X" and now, as (theoretically) anyone taking a dependency there new this might be changing.

[lmolkova]

The automation should not allow to remove metric definitions regardless of stability - this helps minimize dependency conflicts in generated semconv artifacts.

The check was updated recently - #2836, I imagine if this PR is rebased and checks re-run, it'd fail requiring metrics to be preserved (as deprecated).

[ChrsMark]

Added that but the validation is failing:

Diagnostic report:

Violation: Metric 'k8s.replication_controller.available_pods' was renamed to 'k8s.replicationcontroller.available_pods', but the new metric does not exist or is deprecated.
  - Category         : deprecation_violation
  - Type             : semconv_attribute
  - SemConv group    : metric.k8s.replication_controller.available_pods
  - SemConv attribute: 
  - Provenance: /home/weaver/source

Violation: Metric 'k8s.replication_controller.desired_pods' was renamed to 'k8s.replicationcontroller.desired_pods', but the new metric does not exist or is deprecated.
  - Category         : deprecation_violation
  - Type             : semconv_attribute
  - SemConv group    : metric.k8s.replication_controller.desired_pods
  - SemConv attribute: 
  - Provenance: /home/weaver/source

@lmolkova could you clarify what's the expectation here? Should we preserve the renames' "history" or just point to the latest existing metric from the deprecated ones?

[lmolkova]

If I understand correctly:

• metric A was renamed to B
• then metric B was renamed to C

Should deprecation note on A now point to C?
Yes!

If you migrate from old version with metric A, you should migrate directly to C, no need to even know that there was B in the middle.
If someone is looking for migration path to older version of semconv (before C existed), they could find B in that version.

[AlexanderWert]

same as the comment above about adding additional deprecated metric

[dmitryax]

Even after #2306 is resolved, it still unclear why is this change required. Looking at these guidelines it seems like existing naming still valid.

1. They cannot be taken as namespaces
2. "value being recorded represents discrete instances of a countable quantity"

### Naming rules for counters and UpDownCounters

#### Pluralization

Metric namespaces SHOULD NOT be pluralized.

Metric names SHOULD NOT be pluralized, unless the value being recorded
represents discrete instances of a
[countable quantity](https://wikipedia.org/wiki/Count_noun).
Generally, the name SHOULD be pluralized only if the unit of the metric in
question is a non-unit (like `{fault}` or `{operation}`).

Examples:

- `system.filesystem.utilization`, `http.server.request.duration`, and `system.cpu.time`
  should not be pluralized, even if many data points are recorded.
- `system.paging.faults`, `system.disk.operations`, and `system.network.packets`
  should be pluralized, even if only a single data point is recorded.

#### Do not pluralize UpDownCounter names

UpDownCounter names SHOULD NOT be pluralized.

For example if we have a namespace `system.process` which contains all metrics related
to the processes then to represent the count of the processes we can have a metric named
`system.process.count` instead of `system.processes`. Similarly, `cicd.pipeline.run.active` is preferred
over the `cicd.pipeline.active_runs`.

What am I missing?

[dmitryax]

The PR introduces a significant number of breaking changes, and I’m not sure the current justification is sufficient.

[ChrsMark]

@dmitryax the guidance moves us towards consistency based on the discussions happened in the respective guidance PR and specifically #2317 (comment). The part that applies here is that UpDownCounters should not be pluralised, with the following example:

Similarly, `cicd.pipeline.run.active` is preferred
over the `cicd.pipeline.active_runs`.

Also note that those metrics are not in use yet in the Collector in the current format hence it's not really an impactful breaking change. Those metrics are already different to the Collector's ones as we can see in https://github.com/open-telemetry/semantic-conventions/pull/2822/files#diff-ba5fa96c108eca125c2c88f68cdfb0a3d0c95792912f5518eedd55666bc1c033.

I don't have a strong preference for this change either but it seems that we need it so as to be consistent with the rest of the project.

@lmolkova please chime in if we miss anything here.

[dmitryax]

Ok, but the guidelines in the spec are still not strong enough for this change. All of the existing metrics seem to follow this section:

Metric names SHOULD NOT be pluralized, unless the value being recorded
represents discrete instances of a
countable quantity.
Generally, the name SHOULD be pluralized only if the unit of the metric in
question is a non-unit (like {fault} or {operation}).

Do we need to remove that part from the spec?

In general, looking at the changed metrics names as a user, the existing ones seem to be more intuitive than to the new ones.

[jsuereth]

I believe the tooling expects all the previous metric names to remain around.

Given these are not "stable" I think there's less enforcement around keeping things around, although it is risky depending on whether the intermediate name was used.

If you have a caveat asking folks NOT to migrate to new metrics past version "X" then you can drop the intermediary if it's between "X" and now, as (theoretically) anyone taking a dependency there new this might be changing.

[ChrsMark]

I will update this based on the outcome of #2822 (comment).

[dmitryax]

LGTM

[ChrsMark]

@open-telemetry/semconv-k8s-approvers please take another look since this PR has been updated.

@open-telemetry/specs-semconv-maintainers could you help with the deprecation question from #2822 (comment)? Right now the checks fail because of the "double" deprecation chain, or do I miss anything else there?