Skip to content

Commit bc0e9df

Browse files
committed
Relax the crashes on mutating closed span; that's overdoing it
We just ignore writes made after a span was closed; and we ensure a double close won't mutate the timestamp NOR call the the onEnd multiple times. That's the expected baseline behavior.
1 parent f1ffc1c commit bc0e9df

File tree

2 files changed

+57
-104
lines changed

2 files changed

+57
-104
lines changed

Sources/InMemoryTracing/InMemorySpan.swift

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
import Tracing
1717

1818
/// A ``Span`` created by the ``InMemoryTracer`` that will be retained in memory when ended.
19-
/// See ``InMemoryTracer/
19+
///
20+
/// - SeeAlso: ``InMemoryTracer``
2021
public struct InMemorySpan: Span {
2122
public let context: ServiceContext
2223
public let spanContext: InMemorySpanContext
@@ -57,7 +58,7 @@ public struct InMemorySpan: Span {
5758
_operationName.withValue { $0 }
5859
}
5960
nonmutating set {
60-
assertIsRecording()
61+
guard isRecording else { return }
6162
_operationName.withValue { $0 = newValue }
6263
}
6364
}
@@ -67,7 +68,7 @@ public struct InMemorySpan: Span {
6768
_attributes.withValue { $0 }
6869
}
6970
nonmutating set {
70-
assertIsRecording()
71+
guard isRecording else { return }
7172
_attributes.withValue { $0 = newValue }
7273
}
7374
}
@@ -77,7 +78,7 @@ public struct InMemorySpan: Span {
7778
}
7879

7980
public func addEvent(_ event: SpanEvent) {
80-
assertIsRecording()
81+
guard isRecording else { return }
8182
_events.withValue { $0.append(event) }
8283
}
8384

@@ -86,7 +87,7 @@ public struct InMemorySpan: Span {
8687
}
8788

8889
public func addLink(_ link: SpanLink) {
89-
assertIsRecording()
90+
guard isRecording else { return }
9091
_links.withValue { $0.append(link) }
9192
}
9293

@@ -99,7 +100,7 @@ public struct InMemorySpan: Span {
99100
attributes: SpanAttributes,
100101
at instant: @autoclosure () -> some TracerInstant
101102
) {
102-
assertIsRecording()
103+
guard isRecording else { return }
103104
_errors.withValue {
104105
$0.append(RecordedError(error: error, attributes: attributes, instant: instant()))
105106
}
@@ -110,12 +111,12 @@ public struct InMemorySpan: Span {
110111
}
111112

112113
public func setStatus(_ status: SpanStatus) {
113-
assertIsRecording()
114+
guard isRecording else { return }
114115
_status.withValue { $0 = status }
115116
}
116117

117118
public func end(at instant: @autoclosure () -> some TracerInstant) {
118-
assertIsRecording()
119+
guard isRecording else { return }
119120
let finishedSpan = FinishedInMemorySpan(
120121
operationName: operationName,
121122
context: context,
@@ -138,18 +139,6 @@ public struct InMemorySpan: Span {
138139
public let attributes: SpanAttributes
139140
public let instant: any TracerInstant
140141
}
141-
142-
private func assertIsRecording(
143-
file: StaticString = #file,
144-
line: UInt = #line
145-
) {
146-
assert(
147-
_isRecording.withValue { $0 } == true,
148-
"Attempted to mutate already ended span.",
149-
file: file,
150-
line: line
151-
)
152-
}
153142
}
154143

155144
/// Represents a finished span (a ``Span`` that `end()` was called on)

Tests/InMemoryTracingTests/InMemoryTracerTests.swift

Lines changed: 48 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
@_spi(Locking) import Instrumentation
1717
import Testing
1818
import Tracing
19-
import InMemoryTracing
19+
@_spi(Testing) import InMemoryTracing
2020

2121
@Suite("InMemoryTracer")
2222
struct InMemoryTracerTests {
@@ -353,95 +353,26 @@ struct InMemoryTracerTests {
353353
}
354354
}
355355

356-
#if compiler(>=6.2) // Exit tests require Swift 6.2
357-
@Suite("TestSpan can't be mutated after being ended")
358-
struct FinishedSpanImmutability {
359-
@Test("Operation name is immutable on ended span")
360-
func operationName() async {
361-
await #expect(processExitsWith: .failure) {
362-
let span = InMemorySpan.stub
363-
span.operationName = ""
364-
365-
span.end()
366-
367-
span.operationName = "💥"
368-
}
369-
}
370-
371-
@Test("Attributes are immutable on ended span")
372-
func attributes() async {
373-
await #expect(processExitsWith: .failure) {
374-
let span = InMemorySpan.stub
375-
span.attributes["before"] = ""
376-
377-
span.end()
378-
379-
span.attributes["after"] = "💥"
356+
@Test("Span can't be ended repeatedly")
357+
func inMemoryDoubleEnd() async {
358+
let endCounter = LockedValueBox<Int>(0)
359+
let span = InMemorySpan.stub { finished in
360+
endCounter.withValue { counter in
361+
counter += 1
362+
#expect(counter < 2, "Must not end() a span multiple times.")
380363
}
381364
}
365+
span.setStatus(SpanStatus(code: .ok))
382366

383-
@Test("Events are immutable on ended span")
384-
func events() async {
385-
await #expect(processExitsWith: .failure) {
386-
let span = InMemorySpan.stub
387-
span.addEvent("")
388-
389-
span.end()
390-
391-
span.addEvent("💥")
392-
}
393-
}
394-
395-
@Test("Links are immutable on ended span")
396-
func links() async {
397-
await #expect(processExitsWith: .failure) {
398-
let span = InMemorySpan.stub
399-
span.addLink(.stub)
400-
401-
span.end()
367+
let clock = MockClock()
368+
clock.setTime(111)
369+
span.end()
402370

403-
span.addLink(.stub)
404-
}
405-
}
406-
407-
@Test("Errors are immutable on ended span")
408-
func errors() async {
409-
await #expect(processExitsWith: .failure) {
410-
struct TestError: Error {}
411-
let span = InMemorySpan.stub
412-
span.recordError(TestError())
413-
414-
span.end()
415-
416-
span.recordError(TestError())
417-
}
418-
}
419-
420-
@Test("Status is immutable on ended span")
421-
func status() async {
422-
await #expect(processExitsWith: .failure) {
423-
let span = InMemorySpan.stub
424-
span.setStatus(SpanStatus(code: .ok))
425-
426-
span.end()
427-
428-
span.setStatus(SpanStatus(code: .error))
429-
}
430-
}
371+
clock.setTime(222)
372+
span.end(at: clock.now) // should not blow up, but also, not update time again
431373

432-
@Test("Span can't be ended repeatedly")
433-
func end() async {
434-
await #expect(processExitsWith: .failure) {
435-
let span = InMemorySpan.stub
436-
span.setStatus(SpanStatus(code: .ok))
437-
438-
span.end()
439-
440-
span.end()
441-
}
442-
}
374+
#expect(endCounter.withValue { $0 } == 1)
443375
}
444-
#endif
445376
}
446377

447378
extension InMemorySpan {
@@ -455,6 +386,17 @@ extension InMemorySpan {
455386
onEnd: { _ in }
456387
)
457388
}
389+
390+
fileprivate static func stub(onEnd: @Sendable @escaping (FinishedInMemorySpan) -> ()) -> InMemorySpan {
391+
InMemorySpan(
392+
operationName: "stub",
393+
context: .topLevel,
394+
spanContext: InMemorySpanContext(traceID: "stub", spanID: "stub", parentSpanID: nil),
395+
kind: .internal,
396+
startInstant: DefaultTracerClock().now,
397+
onEnd: onEnd
398+
)
399+
}
458400
}
459401

460402
private struct DictionaryInjector: Injector {
@@ -472,4 +414,26 @@ private struct DictionaryExtractor: Extractor {
472414
private struct UnrelatedContextKey: ServiceContextKey {
473415
typealias Value = Int
474416
}
417+
418+
final class MockClock {
419+
var _now: UInt64 = 0
420+
421+
init() {}
422+
423+
func setTime(_ time: UInt64) {
424+
self._now = time
425+
}
426+
427+
struct Instant: TracerInstant {
428+
var nanosecondsSinceEpoch: UInt64
429+
static func < (lhs: MockClock.Instant, rhs: MockClock.Instant) -> Bool {
430+
lhs.nanosecondsSinceEpoch < rhs.nanosecondsSinceEpoch
431+
}
432+
}
433+
434+
var now: Instant {
435+
Instant(nanosecondsSinceEpoch: self._now)
436+
}
437+
}
438+
475439
#endif

0 commit comments

Comments
 (0)