Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Add @unchecked Sendable conformance to HttpTelemetry #810

Merged
merged 3 commits into from
Sep 7, 2024
Merged

fix: Add @unchecked Sendable conformance to HttpTelemetry #810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
65b6cf2
fix: Add manual @unchecked Sendable conformance to HttpTelemetry
jbelkins Sep 6, 2024
33de4b6
Merge branch 'main' into jbe/manual_sendable_telemetry
jbelkins Sep 6, 2024
086d6e2
fix swiftlint
jbelkins Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,18 +220,20 @@ public class CRTClientEngine: HTTPClient {
context: telemetryContext)
// END - smithy.client.http.requests.queued_duration

// TICK - smithy.client.http.connections.limit
telemetry.httpMetricsUsage.connectionsLimit = serialExecutor.maxConnectionsPerEndpoint

let connectionsLimit = serialExecutor.maxConnectionsPerEndpoint
let connectionMgrMetrics = connectionMgr.fetchMetrics()
telemetry.updateHTTPMetricsUsage { httpMetricsUsage in
// TICK - smithy.client.http.connections.limit
httpMetricsUsage.connectionsLimit = connectionsLimit

// TICK - smithy.client.http.connections.usage
telemetry.httpMetricsUsage.idleConnections = connectionMgrMetrics.availableConcurrency
telemetry.httpMetricsUsage.acquiredConnections = connectionMgrMetrics.leasedConcurrency
// TICK - smithy.client.http.connections.usage
httpMetricsUsage.idleConnections = connectionMgrMetrics.availableConcurrency
httpMetricsUsage.acquiredConnections = connectionMgrMetrics.leasedConcurrency

// TICK - smithy.client.http.requests.usage
telemetry.httpMetricsUsage.inflightRequests = connectionMgrMetrics.leasedConcurrency
telemetry.httpMetricsUsage.queuedRequests = connectionMgrMetrics.pendingConcurrencyAcquires
// TICK - smithy.client.http.requests.usage
httpMetricsUsage.inflightRequests = connectionMgrMetrics.leasedConcurrency
httpMetricsUsage.queuedRequests = connectionMgrMetrics.pendingConcurrencyAcquires
}

do {
// DURATION - smithy.client.http.connections.uptime
Expand Down Expand Up @@ -433,18 +435,21 @@ public class CRTClientEngine: HTTPClient {
wrappedContinuation.safeResume(error: error)
return
}
// TICK - smithy.client.http.connections.limit
telemetry.httpMetricsUsage.connectionsLimit = serialExecutor.maxConnectionsPerEndpoint

let connectionsLimit = serialExecutor.maxConnectionsPerEndpoint
let connectionMgrMetrics = connectionMgr.fetchMetrics()
telemetry.updateHTTPMetricsUsage { httpMetricsUsage in
// TICK - smithy.client.http.connections.limit
httpMetricsUsage.connectionsLimit = connectionsLimit

// TICK - smithy.client.http.connections.usage
telemetry.httpMetricsUsage.idleConnections = connectionMgrMetrics.availableConcurrency
telemetry.httpMetricsUsage.acquiredConnections = connectionMgrMetrics.leasedConcurrency
// TICK - smithy.client.http.connections.usage
httpMetricsUsage.idleConnections = connectionMgrMetrics.availableConcurrency
httpMetricsUsage.acquiredConnections = connectionMgrMetrics.leasedConcurrency

// TICK - smithy.client.http.requests.usage
telemetry.httpMetricsUsage.inflightRequests = connectionMgrMetrics.leasedConcurrency
telemetry.httpMetricsUsage.queuedRequests = connectionMgrMetrics.pendingConcurrencyAcquires
// TICK - smithy.client.http.requests.usage
httpMetricsUsage.inflightRequests = connectionMgrMetrics.leasedConcurrency
httpMetricsUsage.queuedRequests = connectionMgrMetrics.pendingConcurrencyAcquires
}

// At this point, continuation is resumed when the initial headers are received
// it is now safe to write the body
Expand Down
47 changes: 40 additions & 7 deletions Sources/ClientRuntime/Networking/Http/HttpTelemetry.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
import protocol SmithyHTTPAPI.HTTPClient
import struct Smithy.Attributes
import struct Smithy.AttributeKey
import class Foundation.NSRecursiveLock

/// Container for HTTPClient telemetry, including configurable attributes and names.
///
/// Note: This is intended to be used within generated code, not directly.
public class HttpTelemetry {
public class HttpTelemetry: @unchecked Sendable {
private static var idleConnectionAttributes: Attributes {
var attributes = Attributes()
attributes.set(key: HttpMetricsAttributesKeys.state, value: ConnectionState.idle)
Expand All @@ -37,20 +38,52 @@ public class HttpTelemetry {
internal let loggerProvider: any LoggerProvider

internal let tracerScope: String
internal let tracerAttributes: Attributes?
internal let spanName: String
internal let spanAttributes: Attributes?

internal let connectionsAcquireDuration: any Histogram
private let connectionsLimit: any AsyncMeasurementHandle
private let connectionsUsage: any AsyncMeasurementHandle
internal var httpMetricsUsage: HttpMetricsUsage
internal let connectionsUptime: any Histogram
private let requestsUsage: any AsyncMeasurementHandle
internal let requestsQueuedDuration: any Histogram
internal let bytesSent: any MonotonicCounter
internal let bytesReceived: any MonotonicCounter

// Lock to enforce exclusive access to non-Sendable properties
private let lock = NSRecursiveLock()

// The properties _tracerAttributes, _spanAttributes, and _httpMetricsUsage
// are not Sendable & must be protected by lock.
private let _tracerAttributes: Attributes?

var tracerAttributes: Attributes? {
lock.lock()
defer { lock.unlock() }
return _tracerAttributes
}

private let _spanAttributes: Attributes?

var spanAttributes: Attributes? {
lock.lock()
defer { lock.unlock() }
return _spanAttributes
}

private var _httpMetricsUsage: HttpMetricsUsage

var httpMetricsUsage: HttpMetricsUsage {
lock.lock()
defer { lock.unlock() }
return _httpMetricsUsage
}

func updateHTTPMetricsUsage(_ block: (inout HttpMetricsUsage) -> Void) {
lock.lock()
defer { lock.unlock() }
block(&_httpMetricsUsage)
}

public init(
httpScope: String,
telemetryProvider: any TelemetryProvider = DefaultTelemetry.provider,
Expand All @@ -66,11 +99,11 @@ public class HttpTelemetry {
self.loggerProvider = telemetryProvider.loggerProvider
let meterScope: String = meterScope != nil ? meterScope! : httpScope
self.tracerScope = tracerScope != nil ? tracerScope! : httpScope
self.tracerAttributes = tracerAttributes
self._tracerAttributes = tracerAttributes
self.spanName = spanName != nil ? spanName! : "HTTP"
self.spanAttributes = spanAttributes
self._spanAttributes = spanAttributes
let httpMetricsUsage = HttpMetricsUsage()
self.httpMetricsUsage = httpMetricsUsage
self._httpMetricsUsage = httpMetricsUsage

let meter = telemetryProvider.meterProvider.getMeter(scope: meterScope, attributes: meterAttributes)
self.connectionsAcquireDuration = meter.createHistogram(
Expand Down
28 changes: 15 additions & 13 deletions Sources/ClientRuntime/Networking/Http/URLSession/URLSessionHTTPClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,21 +515,23 @@ public final class URLSessionHTTPClient: HTTPClient {
context: telemetryContext)
// END - smithy.client.http.requests.queued_duration

// TICK - smithy.client.http.connections.limit
telemetry.httpMetricsUsage.connectionsLimit = session.configuration.httpMaximumConnectionsPerHost

// TICK - smithy.client.http.connections.usage
// TODO(observability): instead of the transient stores, should rely on the Key/Value observer patttern
let connectionsLimit = session.configuration.httpMaximumConnectionsPerHost
let totalCount = session.delegateQueue.operationCount
let maxConcurrentOperationCount = session.delegateQueue.maxConcurrentOperationCount
telemetry.httpMetricsUsage.acquiredConnections = totalCount < maxConcurrentOperationCount
? totalCount
: maxConcurrentOperationCount
telemetry.httpMetricsUsage.idleConnections = totalCount - telemetry.httpMetricsUsage.acquiredConnections

// TICK - smithy.client.http.requests.usage
telemetry.httpMetricsUsage.inflightRequests = telemetry.httpMetricsUsage.acquiredConnections
telemetry.httpMetricsUsage.queuedRequests = telemetry.httpMetricsUsage.idleConnections
telemetry.updateHTTPMetricsUsage { httpMetricsUsage in
// TICK - smithy.client.http.connections.limit
httpMetricsUsage.connectionsLimit = connectionsLimit

// TICK - smithy.client.http.connections.usage
// TODO(observability): instead of the transient stores, should rely on the Key/Value observer patttern
httpMetricsUsage.acquiredConnections =
totalCount < maxConcurrentOperationCount ? totalCount : maxConcurrentOperationCount
httpMetricsUsage.idleConnections = totalCount - httpMetricsUsage.acquiredConnections

// TICK - smithy.client.http.requests.usage
httpMetricsUsage.inflightRequests = httpMetricsUsage.acquiredConnections
httpMetricsUsage.queuedRequests = httpMetricsUsage.idleConnections
}

// Create the request (with a streaming body when needed.)
do {
Expand Down
Loading