Thread Safety

[srigginsatebay]

Hi!

What is the status of thread safety with the SDK? I am finding some data race warnings from the Thread Sanitizer and I'd like some input as to how to proceed, whether we make access to the SDK thread safe on our side, or if modifying MeterProviderSharedState from different states is expected to function.

Thank you!

[ArielDemarco]

Hey @srigginsatebay! There are some concurrency requirements that the OTel SDK is expected to follow — in this case, specifically for metrics.
If you find that the SDK isn’t meeting those requirements, that would be considered a bug and should be addressed within the SDK itself.

[srigginsatebay]

Thank you for the reply! Where we seem to be hitting this, at least as far as the Thread Sanitizer is concerned, is in DoubleHistogramMeterBuilderSdk.init(meterProviderSharedState: inout MeterProviderSharedState, meterSharedState: inout StableMeterSharedState...

It is complaining about these accesses. The first is on thread T19:

    #0 OpenTelemetrySdk.InstrumentBuilder.init(meterProviderSharedState: inout OpenTelemetrySdk.MeterProviderSharedState, meterSharedState: inout OpenTelemetrySdk.StableMeterSharedState, type: OpenTelemetrySdk.InstrumentType, valueType: OpenTelemetrySdk.InstrumentValueType, description: Swift.String, unit: Swift.String, instrumentName: Swift.String) -> OpenTelemetrySdk.InstrumentBuilder <null> (Telemetry:arm64+0x15b514)
    #1 OpenTelemetrySdk.DoubleHistogramMeterBuilderSdk.init(meterProviderSharedState: inout OpenTelemetrySdk.MeterProviderSharedState, meterSharedState: inout OpenTelemetrySdk.StableMeterSharedState, name: Swift.String, description: Swift.String, unit: Swift.String) -> OpenTelemetrySdk.DoubleHistogramMeterBuilderSdk <null> (Telemetry:arm64+0x14a7dc)
    #2 OpenTelemetrySdk.DoubleHistogramMeterBuilderSdk.__allocating_init(meterProviderSharedState: inout OpenTelemetrySdk.MeterProviderSharedState, meterSharedState: inout OpenTelemetrySdk.StableMeterSharedState, name: Swift.String, description: Swift.String, unit: Swift.String) -> OpenTelemetrySdk.DoubleHistogramMeterBuilderSdk <null> (Telemetry:arm64+0x14a620)
    #3 OpenTelemetrySdk.StableMeterSdk.histogramBuilder(name: Swift.String) -> OpenTelemetryApi.DoubleHistogramBuilder <null> (Telemetry:arm64+0x175580)
    #4 protocol witness for OpenTelemetryApi.StableMeter.histogramBuilder(name: Swift.String) -> OpenTelemetryApi.DoubleHistogramBuilder in conformance OpenTelemetrySdk.StableMeterSdk : OpenTelemetryApi.StableMeter in OpenTelemetrySdk <null> (Telemetry:arm64+0x175b6c)
    #5 Telemetry.OpenTelemetryMeterHandler.histogramRecord(name: Swift.String, value: Swift.Double, attributes: Swift.Dictionary<Swift.String, Swift.String>, options: Swift.Optional<PlatformInterfaces.MetricInstrumentOptions>) -> () <null> (Telemetry:arm64+0x4baf4)

and the second is:

    #0 OpenTelemetrySdk.StableMeterSdk.meterProviderSharedState.modify : OpenTelemetrySdk.MeterProviderSharedState <null> (Telemetry:arm64+0x1745f0)
    #1 OpenTelemetrySdk.StableMeterSdk.histogramBuilder(name: Swift.String) -> OpenTelemetryApi.DoubleHistogramBuilder <null> (Telemetry:arm64+0x1754cc)
    #2 protocol witness for OpenTelemetryApi.StableMeter.histogramBuilder(name: Swift.String) -> OpenTelemetryApi.DoubleHistogramBuilder in conformance OpenTelemetrySdk.StableMeterSdk : OpenTelemetryApi.StableMeter in OpenTelemetrySdk <null> (Telemetry:arm64+0x175b6c)
    #3 Telemetry.OpenTelemetryMeterHandler.histogramRecord(name: Swift.String, value: Swift.Double, attributes: Swift.Dictionary<Swift.String, Swift.String>, options: Swift.Optional<PlatformInterfaces.MetricInstrumentOptions>) -> () <null> (Telemetry:arm64+0x4baf4)

Telemetry is our SDK that fronts OTEL. Our code looks like:

guard let builder = meter.histogramBuilder(name: name) as? DoubleHistogramMeterBuilderSdk else {
			return
		}

where meter is a shared StableMeter passed in to our init.

I am guessing that the inout is the issue here

[ArielDemarco]

Great! I was able to reproduce this issue. And it's happening in the Stable version of the Metrics API (which is the latest) so we'll probably be doing a fix to address this.