`Activity.SetParentId()`allows creation of `ActivitySpanId` that breaks `==` expectations

[nblumhardt]

Description

In .NET 8, System.Diagnostics.Activity.SetParentId() enables calling the internal ActivitySpanId constructor without checking for the default case "0000000000000000".

This breaks an implied invariant exploited by ActivitySpanId.operator==, where "0000000000000000" is expected to map to a null _hexString, eliminating the cost of ensuring null == "0000000000000000" and vice-versa:

        public static bool operator ==(ActivitySpanId spanId1, ActivitySpanId spandId2)
        {
            return spanId1._hexString == spandId2._hexString;
        }

I hit this because I'd assumed that activity.ParentSpanId == default was a valid way to determine whether an activity had a logical W3C parent.

Because of the bug, this comparison may return true, despite the parent actually being the invalid all-zeroes value.

Given the rest of the codebase attempts to protect against creation of ActivitySpanId values with non-null, all-zero _hexString fields, I suspect this is a bug in SetParentId():

                _traceId = traceId.ToHexString();     // The child will share the parent's traceId.
                _parentSpanId = spanId.ToHexString();
                ActivityTraceFlags = activityTraceFlags;
                _parentTraceFlags = (byte)activityTraceFlags;

The second line here should probably set _parentSpanId to null if spanId is default.

Reproduction Steps

// dotnet new console
using System.Diagnostics;

// Fails, correctly
// ActivitySpanId.CreateFromString(default(ActivitySpanId).ToHexString());

var activity = new Activity("test");

// Fails to recognize `default` second argument, and passes the string `"0000000000000000"` to the `internal ActivitySpanId(string)` constructor
activity.SetParentId(ActivityTraceId.CreateRandom(), default);

Console.WriteLine(activity.ParentSpanId);
// -> 0000000000000000

Console.WriteLine(activity.ParentSpanId.ToHexString() == default(ActivitySpanId).ToHexString());
// -> True

Console.WriteLine(activity.ParentSpanId == default);
// -> False

Expected behavior

activity.ParentSpanId == default should be true in any scenario in which ParentSpanId is an all-zeroes value.

Actual behavior

activity.ParentSpanId == default is false.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-activity See info in area-owners.md if you want to be subscribed.

[tarekgh]

This is similar/duplicate of https://github.com/dotnet/runtime/issues/85198 I guess.