Skip to content

Commit 048f419

Browse files
Fix thread race conditions in new metrics (#871)
* Updated SDK locks to use a more similar to Mutex approach, which allows being used in the future when we transition to Swift 6 * Removed inout to properties that dont need it. * Updated some properties to constants when mutability is not needed * Cleaned also most warnings
1 parent e61159e commit 048f419

26 files changed

+189
-103
lines changed

Examples/OTLP HTTP Exporter/main.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,20 +75,20 @@
7575
createSpans()
7676

7777
// Metrics
78-
let otlpMetricExporter = StableOtlpHTTPMetricExporter(endpoint: defaultStableOtlpHTTPMetricsEndpoint())
79-
let meterProvider = StableMeterProviderSdk.builder()
78+
let otlpMetricExporter = OtlpHttpMetricExporter(endpoint: defaultOtlpHttpMetricsEndpoint())
79+
let meterProvider = MeterProviderSdk.builder()
8080
.registerMetricReader(
81-
reader: StablePeriodicMetricReaderBuilder(
81+
reader: PeriodicMetricReaderBuilder(
8282
exporter: otlpMetricExporter).setInterval(timeInterval: 60)
8383
.build()
8484
)
8585
.registerView(
8686
selector: InstrumentSelector.builder().setInstrument(name: ".*").build(),
87-
view: StableView.builder().build()
87+
view: View.builder().build()
8888
)
8989
.build()
9090

91-
OpenTelemetry.registerStableMeterProvider(meterProvider: meterProvider)
91+
OpenTelemetry.registerMeterProvider(meterProvider: meterProvider)
9292

9393
let labels1 = ["dim1": AttributeValue.string("value1")]
9494

Sources/Importers/SwiftMetricsShim/Locks.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,29 @@ extension ReadWriteLock {
200200
try withWriterLock(body)
201201
}
202202
}
203+
204+
public final class Locked<Value> : @unchecked Sendable {
205+
206+
private var internalValue: Value
207+
208+
private let lock = Lock()
209+
210+
public var protectedValue: Value {
211+
get {
212+
lock.withLock { internalValue }
213+
}
214+
_modify {
215+
lock.lock()
216+
defer { lock.unlock() }
217+
yield &internalValue
218+
}
219+
}
220+
221+
public init(initialValue: Value) {
222+
self.internalValue = initialValue
223+
}
224+
225+
public func locking<T>(_ block: (inout Value) throws -> T) rethrows -> T {
226+
try lock.withLock { try block(&internalValue) }
227+
}
228+
}

Sources/Importers/SwiftMetricsShim/MetricHandlers.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,21 @@ import OpenTelemetryApi
99
class SwiftCounterMetric: CounterHandler, SwiftMetric {
1010
let metricName: String
1111
let metricType: MetricType = .counter
12-
var counter: LongCounter
12+
let counter: Locked<LongCounter>
1313
let labels: [String: AttributeValue]
1414

1515
required init(name: String,
1616
labels: [String: String],
1717
meter: any OpenTelemetryApi.Meter) {
1818
metricName = name
19-
counter = meter.counterBuilder(name: name).build()
19+
counter = .init(initialValue: meter.counterBuilder(name: name).build())
2020
self.labels = labels.mapValues { value in
2121
return AttributeValue.string(value)
2222
}
2323
}
2424

2525
func increment(by: Int64) {
26-
counter.add(value: Int(by), attributes: labels)
26+
counter.protectedValue.add(value: Int(by), attributes: labels)
2727
}
2828

2929
func reset() {}

Sources/OpenTelemetrySdk/Common/ComponentRegistry.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class ComponentRegistry<T> {
1212
private var componentByNameVersion = [String: [String: T]]()
1313
private var componentByNameSchema = [String: [String: T]]()
1414
private var componentByNameVersionSchema = [String: [String: [String: T]]]()
15-
private var allComponents = [T]()
15+
private let allComponents = ReadWriteLocked<[T]>(initialValue: [])
1616

1717
private let builder: (InstrumentationScopeInfo) -> T
1818

@@ -84,11 +84,13 @@ class ComponentRegistry<T> {
8484

8585
private func buildComponent(_ scope: InstrumentationScopeInfo) -> T {
8686
let component = builder(scope)
87-
allComponents.append(component)
87+
allComponents.writeLocking {
88+
$0.append(component)
89+
}
8890
return component
8991
}
9092

9193
public func getComponents() -> [T] {
92-
return [T](allComponents)
94+
return [T](allComponents.protectedValue)
9395
}
9496
}

Sources/OpenTelemetrySdk/Internal/Locks.swift

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ extension ReadWriteLock {
163163
/// - Parameter body: The block to execute while holding the lock.
164164
/// - Returns: The value returned by the block.
165165
@inlinable
166-
func withReaderLock<T>(_ body: () throws -> T) rethrows -> T {
166+
public func withReaderLock<T>(_ body: () throws -> T) rethrows -> T {
167167
lockRead()
168168
defer {
169169
self.unlock()
@@ -180,7 +180,7 @@ extension ReadWriteLock {
180180
/// - Parameter body: The block to execute while holding the lock.
181181
/// - Returns: The value returned by the block.
182182
@inlinable
183-
func withWriterLock<T>(_ body: () throws -> T) rethrows -> T {
183+
public func withWriterLock<T>(_ body: () throws -> T) rethrows -> T {
184184
lockWrite()
185185
defer {
186186
self.unlock()
@@ -200,3 +200,61 @@ extension ReadWriteLock {
200200
try withWriterLock(body)
201201
}
202202
}
203+
204+
public final class Locked<Value> : @unchecked Sendable {
205+
206+
private var internalValue: Value
207+
208+
private let lock = Lock()
209+
210+
public var protectedValue: Value {
211+
get {
212+
lock.withLock { internalValue }
213+
}
214+
_modify {
215+
lock.lock()
216+
defer { lock.unlock() }
217+
yield &internalValue
218+
}
219+
}
220+
221+
public init(initialValue: Value) {
222+
self.internalValue = initialValue
223+
}
224+
225+
public func locking<T>(_ block: (inout Value) throws -> T) rethrows -> T {
226+
try lock.withLock { try block(&internalValue) }
227+
}
228+
}
229+
230+
public final class ReadWriteLocked<Value> : @unchecked Sendable {
231+
232+
private var internalValue: Value
233+
234+
private let rwlock = ReadWriteLock()
235+
236+
public var protectedValue: Value {
237+
get {
238+
rwlock.withReaderLock { internalValue }
239+
}
240+
_modify {
241+
rwlock.lockWrite()
242+
defer { rwlock.unlock() }
243+
yield &internalValue
244+
}
245+
}
246+
247+
public init(initialValue: Value) {
248+
self.internalValue = initialValue
249+
}
250+
251+
public func readLocking<T>(_ block: (Value) throws -> T) rethrows -> T {
252+
try rwlock.withReaderLock{ try block(internalValue) }
253+
}
254+
255+
public func writeLocking<T>(_ block: (inout Value) throws -> T) rethrows -> T {
256+
try rwlock.withWriterLock{ try block(&internalValue) }
257+
}
258+
259+
260+
}

Sources/OpenTelemetrySdk/Logs/LoggerBuilderSdk.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import Foundation
77
import OpenTelemetryApi
88

99
public class LoggerBuilderSdk: LoggerBuilder {
10-
private var registry: ComponentRegistry<LoggerSdk>
11-
private var instrumentationScopeName: String
10+
private let registry: ComponentRegistry<LoggerSdk>
11+
private let instrumentationScopeName: String
1212
private var eventDomain: String?
1313
private var schemaUrl: String?
1414
private var attributes: [String: AttributeValue]?

Sources/OpenTelemetrySdk/Logs/LoggerProviderSdk.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import OpenTelemetryApi
88

99
public class LoggerProviderSdk: LoggerProvider {
1010
private var sharedState: LoggerSharedState
11-
private var loggerRegistry: ComponentRegistry<LoggerSdk>
11+
private let loggerRegistry: ComponentRegistry<LoggerSdk>
1212
public init(clock: Clock = MillisClock(),
1313
resource: Resource = EnvVarResource.get(),
1414
logLimits: LogLimits = LogLimits(),

Sources/OpenTelemetrySdk/Metrics/DoubleCounterMeterBuilderSdk.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import Foundation
77
import OpenTelemetryApi
88

99
public class DoubleCounterMeterBuilderSdk: InstrumentBuilder, DoubleCounterBuilder {
10-
init(meterProviderSharedState: inout MeterProviderSharedState,
11-
meterSharedState: inout MeterSharedState,
10+
init(meterProviderSharedState: MeterProviderSharedState,
11+
meterSharedState: MeterSharedState,
1212
name: String,
1313
description: String,
1414
unit: String) {
1515
super.init(
16-
meterProviderSharedState: &meterProviderSharedState,
17-
meterSharedState: &meterSharedState,
16+
meterProviderSharedState: meterProviderSharedState,
17+
meterSharedState: meterSharedState,
1818
type: .counter,
1919
valueType: .double,
2020
description: description,

Sources/OpenTelemetrySdk/Metrics/DoubleGaugeBuilderSdk.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ import Foundation
77
import OpenTelemetryApi
88

99
public class DoubleGaugeBuilderSdk: InstrumentBuilder, DoubleGaugeBuilder {
10-
init(meterProviderSharedState: inout MeterProviderSharedState, meterSharedState: inout MeterSharedState, name: String) {
10+
init(meterProviderSharedState: MeterProviderSharedState, meterSharedState: MeterSharedState, name: String) {
1111
super.init(
12-
meterProviderSharedState: &meterProviderSharedState,
13-
meterSharedState: &meterSharedState,
12+
meterProviderSharedState: meterProviderSharedState,
13+
meterSharedState: meterSharedState,
1414
type: .observableGauge,
1515
valueType: .double,
1616
description: "",

Sources/OpenTelemetrySdk/Metrics/DoubleHistogramMeterBuilderSdk.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import Foundation
77
import OpenTelemetryApi
88

99
public class DoubleHistogramMeterBuilderSdk: InstrumentBuilder, DoubleHistogramBuilder {
10-
init(meterProviderSharedState: inout MeterProviderSharedState,
11-
meterSharedState: inout MeterSharedState,
10+
init(meterProviderSharedState: MeterProviderSharedState,
11+
meterSharedState: MeterSharedState,
1212
name: String,
1313
description: String = "",
1414
unit: String = "") {
1515
super.init(
16-
meterProviderSharedState: &meterProviderSharedState,
17-
meterSharedState: &meterSharedState,
16+
meterProviderSharedState: meterProviderSharedState,
17+
meterSharedState: meterSharedState,
1818
type: .histogram,
1919
valueType: .double,
2020
description: description,

0 commit comments

Comments
 (0)