Skip to content

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

Open

Description

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions