add k8s.container.status.current_waiting_reason resource attribute

[povilasv]

Fixes #996

Changes

Please provide a brief description of the changes here.

Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.

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]

LGTM

[povilasv]

Would appreciate a review here 🙇

Maybe @TylerHelmuth or @lmolkova you could take a look 👀

[ChrsMark]

@open-telemetry/semconv-k8s-approvers @open-telemetry/specs-semconv-approvers please take a look.

[trask]

sorry for the late comment, was just curious if there was any discussion already about this resource attribute being non-immutable?

[joaopgrassi]

@trask The collector linked PR says:

FYI I've opened a PR on semconv for last terminated reason

But I see that this is not reflected in the attribute definition and can be confusing. @povilasv can you please clarify if this is the correct assumption of this attribute? Resource attributes are immutable. Is it even possible to know the "last terminated reason"? Or are we assuming to take the current state and that's it?

[ChrsMark]

#968 introduced the k8s.container.status.last_terminated_reason. This one adds a different attribute.
From my perspective these attributes represent "states" of the container. If those cannot be expressed as resource attributes because of the immutability constraint then what would be the right way to report them?

[joaopgrassi]

Yeah, probably as events I'd say.

[povilasv]

Hey folks,

Did not know about immutability constraint on resource attributes. Sorry for this.

Looks like k8s.container.status.current_waiting_reason is not immutable. Also container.status.last_terminated_reason might be not immutable (Though it's still a question, as I think - once container is terminated it gets new container.id and is a new Entity)

But I think it would be best to model it as metric that tracks state changes. Also events are an option.

I guess we also should remove these two and model it differently.

How do I revert PRs/ deprecate the attributes?

[joaopgrassi]

We don't allow removing things anymore and just deprecating, but since this wasn't released yet I think we can just remove it. What do you think @lmolkova ? The other one that was merged container.status.last_terminated_reason is part of the last release though, so that one cannot be removed and just deprecated.

[povilasv]

I don't think container.status.last_terminated_reason should be removed, as it's AFAIK immutable. Only new containers can get last terminated reason. I.E. container has to terminate in order for container.status.last_terminated_reason to change.

[ChrsMark]

I don't think container.status.last_terminated_reason should be removed, as it's AFAIK immutable.

While that's most probably true, I wonder if we should find a consistent way to model both of them. It could be weird to find k8s.container.status.last_terminated_reason being a resource attribute while k8s.container.status.current_waiting_reason being a metric or event?

[povilasv]

Agree

[lmolkova]

We don't allow removing things anymore and just deprecating, but since this wasn't released yet I think we can just remove it.

it's ok to remove things that haven't been released - we know that nothing depends on it.

[jsuereth]

I'm way late to the party, but I think we need to back out the resource-attribute portion of this change.

See: #1181 (comment) for rationale. If I had caught this earlier I would have blocked it earlier, so apologies for being late. At a minimum, this is still experimental, so we CAN deprecate the resource attribute.

[povilasv]

@jsuereth we didn't release this, removed in #1115, so nothing to worry about :)