Apply the specified status precedence in Span.set_status

[WatchTree-19]

Description

Fixes #5661.

Span.set_status guards two cases when it is handed a Status instance: it returns early if the span is already OK, and it returns early if the incoming status is UNSET. Nothing guards a second ERROR overwriting a first one, so self._status = status runs unconditionally and a bare Status(StatusCode.ERROR) with no description silently replaces Status(StatusCode.ERROR, "..."). The status code does not change, so nothing looks wrong; only the description disappears.

This is reachable without anyone doing anything odd. A library sets a specific error on a span, and later generic error handling, exception bookkeeping or an instrumentation helper sets a plain ERROR on the same span. The specific message is the one that had diagnostic value, and it is the one that is lost.

The assignment is now declined when the incoming status carries no description, the recorded one does, and the codes match. Nothing else changes: a described status still replaces a described status, a bare status still lands when there is nothing to keep, and a different status code still takes precedence as before.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

opentelemetry-sdk/tests/trace/test_trace.py passes in full: 118 passed, 9 subtests passed.

Four cases were added to TestSpan, all exercised through the public set_status:

• test_bare_status_does_not_drop_existing_description is the regression test. It fails on an unpatched tree with AssertionError: None != 'connection refused to db-1'.
• test_described_status_still_replaces_described_status pins that a real re-description is unaffected.
• test_bare_status_lands_when_there_is_no_description_to_keep pins that the guard does not block a first bare ERROR.
• test_bare_status_of_a_different_code_still_lands pins that the guard is per status code.

The last three pass with and without the fix, by design. Only the first discriminates.

The behaviour was also reproduced against released opentelemetry-sdk 1.44.0 on CPython 3.11 before anything was changed, using the snippet in #5661.

Does This PR Require a Contrib Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

On the changelog: .changelog/ names fragments after the PR number, which does not exist until this is opened, so <pr>.fixed follows as a second commit straight after.

AI assistance was used and is disclosed on the commit with an Assisted-by: trailer, per AGENTS.md.

[herin049]

We should probably update this to be more in line with what the spec expects:

Namely that Span statuses form a total order Ok > Error > Unset and any attempts to update the status that are not strictly greater than the previous status should be ignored (i.e. all future calls to set status will be ignored when the status is set to Error unless the new status is Ok, and every future call to set status will be ignored when the status is set to Ok). A call to explicitly set the status to Unset should always be ignored.

[WatchTree-19]

Thanks for laying the ordering out, I went through each point against the spec. Three of the four are in there: the total order Ok > Error > Unset, "an attempt to set value Unset SHOULD be ignored", and Ok being final. I couldn't find the fourth one, ignoring anything that isn't strictly greater. The spec says "only the value of the last call will be recorded, and implementations are free to ignore previous calls." Taken to the letter, your rule would mean Error("first") followed by Error("second") keeps "first", which is what that last call sentence rules out.

I also wrote all twelve transitions up as tests and ran them against upstream main. Eleven of them pass, and the only one that fails is the bug I first reported. So the SDK was mostly doing the right thing already. The ordering just wasn't written down anywhere: it was split across two conditions, one of which only reads right if you know Python's and/or precedence, and nothing tested it.

What I've pushed now writes the order out explicitly and puts the decision in one method. It adds the twelve tests and keeps last call wins within the same rank, along with the exception for bare status. One thing worth mentioning: the enum is UNSET=0, OK=1, ERROR=2, which isn't the spec's order, so it can't be a plain enum comparison.

The main thing I wanted to check with you: should Error to Error be ignored as you wrote it, even though that goes against the spec's last call sentence? Or should the ordering be made explicit with last call wins kept, which is what's on the branch now?

[Shriprasad-P]

Nice focused PR. Please make sure docs/changelog stay aligned if the repo requires it for this kind of change.