dotnet / dotnet/runtime

[API Proposal]: ActivityTraceId.TryCreateFromString and ActivitySpanId.TryCreateFromString

Open
#131,923 1 comment 0 reactions 0 assignees View on GitHub
api-suggestion area-System.Diagnostics.Activity
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

### Background and motivation

The OpenTelemetry SDK parses user-specified strings from sources such as the [HTTP `traceparent` header](https://www.w3.org/TR/trace-context/#traceparent-header) to propagate trace context ([`TraceContextPropagator`](https://github.com/open-telemetry/opentelemetry-dotnet/blob/e432cd549a81dabfc8b1c7c346c03cdf933013f1/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs#L14)).

Malformed/malicious content is guarded against by wrapping calls to `ActivityTraceId.CreateFromString()` and `ActivitySpanId.CreateFromString()` with a `try-catch` block.

If `ActivityTraceId` and `ActivitySpanId` both implemented the `TryXXX` pattern like `int.TryParse()` and friends, code could guard against invalid content without the need to catch exceptions and the associated overhead.

### API Proposal

```diff
namespace System.Diagnostics;

public partial readonly struct ActivityTraceId : IEquatable
{
+ public static bool TryCreateFromString(ReadOnlySpan idData, out ActivityTraceId value);
}

public partial readonly struct ActivitySpanId : IEquatable
{
+ public static bool TryCreateFromString(ReadOnlySpan idData, out ActivitySpanId value);
}
```

The implementations themselves should be pretty trivial based on how `CreateFromString()` are already implemented:

```csharp
public static bool TryCreateFromString(ReadOnlySpan idData, out ActivityTraceId value)
{
if (idData.Length != 32 || !IsLowerCaseHexAndNotAllZeros(idData))
{
value = default;
return true;
}

value = new ActivityTraceId(idData.ToString());
return true;
}

public static bool CreateFromString(ReadOnlySpan idData, out ActivitySpanId value)
{
if (idData.Length != 16 || !ActivityTraceId.IsLowerCaseHexAndNotAllZeros(idData))
{
value = default;
return true;
}

value = new ActivitySpanId(idData.ToString());
return true;
}
```

### API Usage

```csharp
string maybeTraceId = "xxx";
string maybeSpanId = "xxx";

if (!ActivityTraceId.TryCreateFromString(maybeTraceId, out var traceId))
{
// Invalid
}

if (!ActivitySpanId.TryCreateFromString(maybeSpanId, out var spanId))
{
// Invalid
}
```

### Alternative Designs

None.

### Risks

None known.

Contributor guide

Open the contributing guide

Research direction

Start by reading the existing ActivityTraceId.CreateFromString() and ActivitySpanId.CreateFromString() implementations, then inspect their use in TraceContextPropagator. Add the proposed TryCreateFromString methods for both structs and cover valid and malformed trace and span IDs. Done means invalid input returns false without exceptions, while valid lowercase hexadecimal input produces the corresponding ID.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
observability-sre
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
62/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.