Skip to content

Commit c02cbdb

Browse files
andrewlockandrewlock
committed
Refactor ToArray as TryGetArray() instead
1 parent 32e2660 commit c02cbdb

File tree

4 files changed

+52
-44
lines changed

4 files changed

+52
-44
lines changed

‎tracer/src/Datadog.Trace/Agent/MessagePack/TraceChunkModel.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,11 @@ private int IndexOf(ulong spanId, int startIndex)
260260
return _spans[0].SpanId == spanId ? 0 : -1;
261261
}
262262

263-
var array = _spans.ToArray();
263+
if (_spans.TryGetArray() is not { } array)
264+
{
265+
// Shouldn't be possible, because we handle 0 and 1 spans above
266+
return -1;
267+
}
264268

265269
// iterate over the span array starting at the specified index + 1
266270
for (var i = startIndex; i < count; i++)

‎tracer/src/Datadog.Trace/Agent/SpanCollection.cs‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -151,31 +151,23 @@ private static Span OutOfBounds()
151151
}
152152

153153
/// <summary>
154-
/// Creates a Span array from the current <see cref="SpanCollection"/> object.
155-
/// Note that this allocates if the <see cref="SpanCollection"/> does not have a count greater than 1
154+
/// Try to get the underlying Span array from the current <see cref="SpanCollection"/> object as an `ArraySegment`.
155+
/// If the <see cref="SpanCollection"/> does _not_ contain an array (because it contains 0 or 1 spans) then returns
156+
/// <c>null</c>.
156157
/// </summary>
157-
/// <returns>A Span array represented by this instance.</returns>
158-
/// <remarks>
159-
/// <para>If the <see cref="SpanCollection"/> contains a single Span internally, it is copied to a new array.</para>
160-
/// <para>If the <see cref="SpanCollection"/> contains an array internally it returns that array instance.</para>
161-
/// </remarks>
162-
public ArraySegment<Span> ToArray()
158+
/// <returns>A wrapper around the Span array represented by this instance if this instance represents more than
159+
/// one span, otherwise null.</returns>
160+
public ArraySegment<Span>? TryGetArray()
163161
{
164162
// Take local copy of _values so type checks remain valid even if the SpanCollection is overwritten in memory
165163
object? value = _values;
166164
if (value is Span[] values)
167165
{
168166
return new ArraySegment<Span>(values, 0, Count);
169167
}
170-
else if (value != null)
171-
{
172-
// value not array, can only be Span
173-
return new ArraySegment<Span>([Unsafe.As<Span>(value)], 0, 1);
174-
}
175-
else
176-
{
177-
return new ArraySegment<Span>(Array.Empty<Span>(), 0, 0);
178-
}
168+
169+
// zero or one spans, so return null
170+
return null;
179171
}
180172

181173
private static Span[] GrowIfNeeded(Span[] array, int currentCount)

‎tracer/src/Datadog.Trace/Ci/Processors/TestSuiteVisibilityProcessor.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// <copyright file="TestSuiteVisibilityProcessor.cs" company="Datadog">
1+
// <copyright file="TestSuiteVisibilityProcessor.cs" company="Datadog">
22
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
33
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
44
// </copyright>
@@ -46,7 +46,12 @@ public SpanCollection Process(in SpanCollection trace)
4646
}
4747

4848
// we know we have multiple spans, so get the underlying array
49-
var segment = trace.ToArray();
49+
if (trace.TryGetArray() is not { } segment)
50+
{
51+
// Shouldn't be possible, because we handle 0 and 1 spans above
52+
return trace;
53+
}
54+
5055
Span[]? spans = null;
5156
var copiedCount = 0;
5257
var haveDrops = false;

‎tracer/test/Datadog.Trace.Tests/Agent/SpanCollectionTests.cs‎

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public void DefaultValue()
2020

2121
collection.Count.Should().Be(0);
2222
collection.FirstSpan.Should().BeNull();
23-
collection.ToArray().Array.Should().BeEmpty();
23+
collection.TryGetArray().HasValue.Should().BeFalse();
2424
foreach (var span in collection)
2525
{
2626
Assert.Fail("We shouldn't have a span to enumerate: " + span);
@@ -37,7 +37,7 @@ public void SingleSpanConstructor()
3737

3838
collection.Count.Should().Be(1);
3939
collection.FirstSpan.Should().BeSameAs(span);
40-
collection.ToArray().Array.Should().ContainSingle().Which.Should().BeSameAs(span);
40+
collection.TryGetArray().HasValue.Should().BeFalse();
4141
var spans = new List<Span>();
4242
foreach (var x in collection)
4343
{
@@ -57,10 +57,11 @@ public void ArrayCapacityConstructor()
5757

5858
collection.Count.Should().Be(0);
5959
collection.FirstSpan.Should().BeNull();
60-
var array = collection.ToArray();
61-
array.Count.Should().Be(0);
62-
array.Offset.Should().Be(0);
63-
array.Array!.Length.Should().Be(length);
60+
var array = collection.TryGetArray();
61+
array.HasValue.Should().BeTrue();
62+
array!.Value.Count.Should().Be(0);
63+
array.Value.Offset.Should().Be(0);
64+
array.Value.Array!.Length.Should().Be(length);
6465
foreach (var span in collection)
6566
{
6667
Assert.Fail("We shouldn't have a span to enumerate: " + span);
@@ -81,10 +82,11 @@ public void ArrayConstructor()
8182
collection[1].Should().BeSameAs(spans[1]);
8283
FluentActions.Invoking(() => collection[2]).Should().Throw<IndexOutOfRangeException>();
8384

84-
var array = collection.ToArray();
85-
array.Count.Should().Be(2);
86-
array.Offset.Should().Be(0);
87-
array.Array.Should().BeSameAs(spans);
85+
var array = collection.TryGetArray();
86+
array.HasValue.Should().BeTrue();
87+
array!.Value.Count.Should().Be(2);
88+
array.Value.Offset.Should().Be(0);
89+
array.Value.Array.Should().BeSameAs(spans);
8890

8991
var enumerated = new List<Span>();
9092
foreach (var span in collection)
@@ -137,19 +139,21 @@ public void Append_ToSingleSpanCollection()
137139
result[1].Should().BeSameAs(span2);
138140

139141
// Default array size is 4
140-
var array = result.ToArray();
141-
array.Count.Should().Be(2);
142-
array.Array.Length.Should().Be(4);
142+
var array = result.TryGetArray();
143+
array.HasValue.Should().BeTrue();
144+
array!.Value.Count.Should().Be(2);
145+
array.Value.Array!.Length.Should().Be(4);
143146
}
144147

145148
[Fact]
146149
public void Append_ToArrayWithCapacityCollection()
147150
{
148151
var spans = new[] { CreateSpan("span1"), CreateSpan("span2"), null, null };
149152
var collection = new SpanCollection(spans, 2);
150-
var array = collection.ToArray();
151-
array.Count.Should().Be(2);
152-
array.Array.Should().BeSameAs(spans);
153+
var array = collection.TryGetArray();
154+
array.HasValue.Should().BeTrue();
155+
array!.Value.Count.Should().Be(2);
156+
array.Value.Array.Should().BeSameAs(spans);
153157

154158
var span3 = CreateSpan("span3");
155159
var result = SpanCollection.Append(in collection, span3);
@@ -159,7 +163,7 @@ public void Append_ToArrayWithCapacityCollection()
159163
result[1].Should().BeSameAs(spans[1]);
160164
result[2].Should().BeSameAs(span3);
161165

162-
collection.ToArray().Array.Should().BeSameAs(spans);
166+
collection.TryGetArray()!.Value.Array.Should().BeSameAs(spans);
163167
}
164168

165169
[Fact]
@@ -168,9 +172,10 @@ public void Append_ToFullArrayCollection()
168172
// Create a collection with an array at capacity
169173
var spans = new[] { CreateSpan("span1"), CreateSpan("span2") };
170174
var collection = new SpanCollection(spans);
171-
var array = collection.ToArray();
172-
array.Count.Should().Be(2);
173-
array.Array.Should().BeSameAs(spans);
175+
var array = collection.TryGetArray();
176+
array.HasValue.Should().BeTrue();
177+
array!.Value.Count.Should().Be(2);
178+
array.Value.Array.Should().BeSameAs(spans);
174179

175180
var span3 = CreateSpan("span3");
176181
var result = SpanCollection.Append(in collection, span3);
@@ -181,8 +186,9 @@ public void Append_ToFullArrayCollection()
181186
result[2].Should().BeSameAs(span3);
182187

183188
// Array should have grown from 2 to 4
184-
var array2 = result.ToArray();
185-
array2.Array.Should().HaveCount(4).And.NotBeSameAs(spans);
189+
var array2 = result.TryGetArray();
190+
array2.HasValue.Should().BeTrue();
191+
array2!.Value.Array.Should().HaveCount(4).And.NotBeSameAs(spans);
186192
}
187193

188194
[Fact]
@@ -203,8 +209,9 @@ public void Append_GrowsArrayExponentially()
203209
}
204210

205211
// Verify array grew: starts at 4, grows to 8, then needs to grow to 16
206-
var array = collection.ToArray();
207-
array.Array!.Length.Should().Be(16);
212+
var array = collection.TryGetArray();
213+
array.HasValue.Should().BeTrue();
214+
array!.Value.Array!.Length.Should().Be(16);
208215
}
209216

210217
private static Span CreateSpan(string operationName = "test-span")

0 commit comments

Comments
 (0)