Make replication controller metrics consistent with resource attributes #1848
Conversation
Signed-off-by: ChrsMark <[email protected]>
ChrsMark
force-pushed
the
fix_k8s_metrics
branch
from
48ad6e9 to
bfa427f
Compare
Signed-off-by: ChrsMark <[email protected]>
| component: k8s | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: Change k8s.replication_controller metrics to k8s.replicationcontroller |
There was a problem hiding this comment.
sorry I missed the discussion in #1742 (comment)
I feel like this opens a pretty big door across semconv for debating when to apply underscores, because we could point to lots of domain-specific apis that concat words instead of underscoring them
I understand (and agree) with replicaset, statefulset, daemonset, even outside of the k8s domain. could we just make an exception for those (*set things) across all semconv domains?
There was a problem hiding this comment.
to me this boils down to #1755 (comment): k8s uses replicationcontroller for everything and we're being consistent with the domain.
Similarly to
semantic-conventions/model/azure/registry.yaml
Lines 78 to 80 in b865f63
there is some external source of this value and we pick this external spelling for attributes limited to this use-case.
There was a problem hiding this comment.
the benefit of doing this for attribute values seems clear to me (e.g. avoids instrumentations having to map domain-specific values to something else)
the benefit to doing this for attribute names is less clear to me
There was a problem hiding this comment.
how would we approach something like replicaset, statefulset, daemonset vs replication_controller in the future? There are some other examples in #1742 (comment) like cronjob or apiservice where we'd have ambiguity
There was a problem hiding this comment.
also mariadb and cosmosdb could arguably be maria_db and cosmos_db. I'm not suggesting it, I'm saying that I genuinely don't know when and why we apply _
Changes
Follow up from #1742 to follow the naming of K8s api for resources. This PR applies the following changes:
Merge requirement checklist
[chore]