ref: SpanId creation (#2619)

benchmarks/Sentry.Benchmarks/SpanIdBenchmarks.cs

@@ -0,0 +1,12 @@
+using BenchmarkDotNet.Attributes;
+
+namespace Sentry.Benchmarks;
+
+public class SpanIdBenchmarks
+{
+    [Benchmark(Description = "Creates a Span ID")]
+    public void CreateSpanId()
+    {
+        SpanId.Create();
+    }
+}

src/Sentry/Internal/RandomValuesFactory.cs

@@ -7,6 +7,10 @@ internal abstract class RandomValuesFactory
     public abstract double NextDouble();
     public abstract void NextBytes(byte[] bytes);
 
+#if !(NETSTANDARD2_0 || NET461)
+    public abstract void NextBytes(Span<byte> bytes);
+#endif
+
     public bool NextBool(double rate) => rate switch
     {
         >= 1 => true,

src/Sentry/Internal/SynchronizedRandomValuesFactory.cs

@@ -7,6 +7,7 @@ internal class SynchronizedRandomValuesFactory : RandomValuesFactory
         public override int NextInt(int minValue, int maxValue) => Random.Shared.Next(minValue, maxValue);
         public override double NextDouble() => Random.Shared.NextDouble();
         public override void NextBytes(byte[] bytes) => Random.Shared.NextBytes(bytes);
+        public override void NextBytes(Span<byte> bytes) => Random.Shared.NextBytes(bytes);
 #else
     private static readonly AsyncLocal<Random> LocalRandom = new();
     private static Random Random => LocalRandom.Value ??= new Random();
@@ -15,5 +16,10 @@ internal class SynchronizedRandomValuesFactory : RandomValuesFactory
     public override int NextInt(int minValue, int maxValue) => Random.Next(minValue, maxValue);
     public override double NextDouble() => Random.NextDouble();
     public override void NextBytes(byte[] bytes) => Random.NextBytes(bytes);
+
+#if !(NETSTANDARD2_0 || NET461)
+    public override void NextBytes(Span<byte> bytes) => Random.NextBytes(bytes);
+#endif
+
 #endif
-}
+}

src/Sentry/SpanId.cs

@@ -1,4 +1,5 @@
 using Sentry.Extensibility;
+using Sentry.Internal;
 
 namespace Sentry;
 
@@ -7,58 +8,81 @@ namespace Sentry;
 /// </summary>
 public readonly struct SpanId : IEquatable<SpanId>, IJsonSerializable
 {
-    private readonly string _value;
+    private static readonly char[] HexChars = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
+    private static readonly RandomValuesFactory Random = new SynchronizedRandomValuesFactory();
 
-    private const string EmptyValue = "0000000000000000";
+    private readonly long _value;
+
+    private long GetValue() => _value;
 
     /// <summary>
     /// An empty Sentry span ID.
     /// </summary>
-    public static readonly SpanId Empty = new(EmptyValue);
+    public static readonly SpanId Empty = new(0);
 
     /// <summary>
     /// Creates a new instance of a Sentry span Id.
     /// </summary>
-    public SpanId(string value) => _value = value;
-
-    // This method is used to return a string with all zeroes in case
-    // the `_value` is equal to null.
-    // It can be equal to null because this is a struct and always has
-    // a parameterless constructor that evaluates to an instance with
-    // all fields initialized to default values.
-    // Effectively, using this method instead of just referencing `_value`
-    // makes the behavior more consistent, for example:
-    // default(SpanId).ToString() -> "0000000000000000"
-    // default(SpanId) == SpanId.Empty -> true
-    private string GetNormalizedValue() => !string.IsNullOrWhiteSpace(_value)
-        ? _value
-        : EmptyValue;
+    public SpanId(string value) => long.TryParse(value, NumberStyles.HexNumber, null, out _value);
+
+    /// <summary>
+    /// Creates a new instance of a Sentry span Id.
+    /// </summary>
+    /// <param name="value"></param>
+    public SpanId(long value) => _value = value;
 
     /// <inheritdoc />
-    public bool Equals(SpanId other) => StringComparer.Ordinal.Equals(
-        GetNormalizedValue(),
-        other.GetNormalizedValue());
+    public bool Equals(SpanId other) => GetValue().Equals(other.GetValue());
 
     /// <inheritdoc />
     public override bool Equals(object? obj) => obj is SpanId other && Equals(other);
 
     /// <inheritdoc />
-    public override int GetHashCode() => StringComparer.Ordinal.GetHashCode(GetNormalizedValue());
+    public override int GetHashCode() => StringComparer.Ordinal.GetHashCode(_value);
 
     /// <inheritdoc />
-    public override string ToString() => GetNormalizedValue();
+    public override string ToString() => _value.ToString("x8");
 
-    // Note: spans are sentry IDs with only 16 characters, rest being truncated.
-    // This is obviously a bad idea as it invalidates GUID's uniqueness properties
-    // (https://devblogs.microsoft.com/oldnewthing/20080627-00/?p=21823)
-    // but all other SDKs do it this way, so we have no choice but to comply.
     /// <summary>
     /// Generates a new Sentry ID.
     /// </summary>
-    public static SpanId Create() => new(Guid.NewGuid().ToString("n")[..16]);
+    public static SpanId Create()
+    {
+#if NETSTANDARD2_0 || NET461
+        byte[] buf = new byte[8];
+#else
+        Span<byte> buf = stackalloc byte[8];
+#endif
+
+        Random.NextBytes(buf);
+
+        var random = BitConverter.ToInt64(buf
+#if NETSTANDARD2_0 || NET461
+            , 0);
+#else
+            );
+#endif
+
+        return new SpanId(random);
+    }
 
     /// <inheritdoc />
-    public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? _) => writer.WriteStringValue(GetNormalizedValue());
+    public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? _)
+    {
+        Span<byte> convertedBytes = stackalloc byte[sizeof(long)];
+        Unsafe.As<byte, long>(ref convertedBytes[0]) = _value;
+
+        // Going backwards through the array to preserve the order of the output hex string (i.e. `4e76` -> `76e4`)
+        Span<char> output = stackalloc char[16];
+        for (var i = convertedBytes.Length - 1; i >= 0; i--)
+        {
+            var value = convertedBytes[i];
+            output[(convertedBytes.Length - 1 - i) * 2] = HexChars[value >> 4];
+            output[(convertedBytes.Length - 1 - i) * 2 + 1] = HexChars[value & 0xF];
+        }
+
+        writer.WriteStringValue(output);
+    }
 
     /// <summary>
     /// Parses from string.
@@ -91,20 +115,4 @@ public static SpanId FromJson(JsonElement json)
     /// The <see cref="Guid"/> from the <see cref="SentryId"/>.
     /// </summary>
     public static implicit operator string(SpanId id) => id.ToString();
-
-    // Note: no implicit conversion from `string` to `SpanId` as that leads to serious bugs.
-    // For example, given a method:
-    // transaction.StartChild(SpanId parentSpanId, string operation)
-    // And an *extension* method:
-    // transaction.StartChild(string operation, string description)
-    // The following code:
-    // transaction.StartChild("foo", "bar")
-    // Will resolve to the first method and not the second, which is incorrect.
-
-    /*
-    /// <summary>
-    /// A <see cref="SentryId"/> from a <see cref="Guid"/>.
-    /// </summary>
-    public static implicit operator SpanId(string value) => new(value);
-    */
 }

test/Sentry.Testing/IsolatedRandomValuesFactory.cs

@@ -11,4 +11,9 @@ internal class IsolatedRandomValuesFactory : RandomValuesFactory
     public override double NextDouble() => _random.NextDouble();
 
     public override void NextBytes(byte[] bytes) => _random.NextBytes(bytes);
+
+#if !(NETSTANDARD2_0 || NET48)
+    public override void NextBytes(Span<byte> bytes) => _random.NextBytes(bytes);
+#endif
+
 }

test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt

@@ -934,6 +934,7 @@ namespace Sentry
     public readonly struct SpanId : Sentry.IJsonSerializable, System.IEquatable<Sentry.SpanId>
     {
         public static readonly Sentry.SpanId Empty;
+        public SpanId(long value) { }
         public SpanId(string value) { }
         public bool Equals(Sentry.SpanId other) { }
         public override bool Equals(object? obj) { }

test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt

@@ -935,6 +935,7 @@ namespace Sentry
     public readonly struct SpanId : Sentry.IJsonSerializable, System.IEquatable<Sentry.SpanId>
     {
         public static readonly Sentry.SpanId Empty;
+        public SpanId(long value) { }
         public SpanId(string value) { }
         public bool Equals(Sentry.SpanId other) { }
         public override bool Equals(object? obj) { }

test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt

@@ -935,6 +935,7 @@ namespace Sentry
     public readonly struct SpanId : Sentry.IJsonSerializable, System.IEquatable<Sentry.SpanId>
     {
         public static readonly Sentry.SpanId Empty;
+        public SpanId(long value) { }
         public SpanId(string value) { }
         public bool Equals(Sentry.SpanId other) { }
         public override bool Equals(object? obj) { }

test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt

@@ -933,6 +933,7 @@ namespace Sentry
     public readonly struct SpanId : Sentry.IJsonSerializable, System.IEquatable<Sentry.SpanId>
     {
         public static readonly Sentry.SpanId Empty;
+        public SpanId(long value) { }
         public SpanId(string value) { }
         public bool Equals(Sentry.SpanId other) { }
         public override bool Equals(object? obj) { }