[api] Better obsolete message for Activity.SetStatus extension method

[Kielek]

Follow up: #5781
Found while working on errors in open-telemetry/opentelemetry-dotnet-contrib#2121

Changes

Previous message was not user friednly:

opentelemetry-dotnet-contrib\test\OpenTelemetry.Exporter.Geneva.Tests\GenevaTraceExporterTests.cs(180,17,180,49): error CS0618: 'ActivityExtensions.SetStatus(Activity?, Status)' is obsolete: 'Call Activity.SetStatus instead this method will be removed in a future version.'

It was referring to the method with the same same without any signature.

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• [ ] Unit tests added/updated
• [ ] Appropriate CHANGELOG.md files updated for non-trivial changes
• [ ] Changes in public API reviewed (if applicable)

[reyang]

I didn't spend time looking at what others are doing or what's the official guidance (if there is any). IIRC the typical pattern is to mention the method name rather than the parameters while using ObsoleteAttribute.

[Kielek]

@reyang, If you prefer, we could use this. I was aware about these changes and I needed some time to find correct method to fix warnings/errors.

Suggested change

	[Obsolete("Call Activity.SetStatus(ActivityStatusCode code, string? description = null) instead. This method will be removed in a future version.")]
	[Obsolete("Call Activity.SetStatus(ActivityStatusCode, string?) instead. This method will be removed in a future version.")]

[CodeBlanch]

Do we really need this? Really really? 🤣 Because there are 3 other obsoletes in this file we should update too if we do. And probably all the others ones sprinkled around the code. But IMO we don't really need it. The existing message is good enough IMO, and there is also a doc link "see" ref thing in <remarks> which will jump users to the overload.

[reyang]

@reyang, If you prefer, we could use this.

I don't prefer any of these, I like the current one (just the method name).

[Kielek]

Up to you : )

The reason behind this - the name is of the method is not changing. Only set of parameters. For RecordException there is a recommendation to call AddException.

Feel free to close the PR if you think that it is not worth to merge.

[reyang]

Maybe send users to this URL https://learn.microsoft.com/dotnet/api/system.diagnostics.activity.setstatus from the warning message.

[reyang]

I'm fine with the change as it brings clarity. I've left a small note regarding the verbosity (too verbose?).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.25%. Comparing base (6250307) to head (0d5f31f).
Report is 345 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5872      +/-   ##
==========================================
+ Coverage   83.38%   86.25%   +2.87%     
==========================================
  Files         297      257      -40     
  Lines       12531    11214    -1317     
==========================================
- Hits        10449     9673     -776     
+ Misses       2082     1541     -541     

Flag	Coverage Δ
unittests	?
unittests-Project-Experimental	86.00% <ø> (?)
unittests-Project-Stable	86.15% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/OpenTelemetry.Api/Trace/ActivityExtensions.cs	86.36% <ø> (-9.10%)	⬇️

... and 233 files with indirect coverage changes

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.