Skip to content

Commit 18902ff

Browse files
tomerdtomerd
authored
convert the PackageCollections module to the new diagnostics apis (#3755)
motivation: adapt new diagnostics apis changes: refactor the PackageCollections module to use new diagnostics api
1 parent fb98989 commit 18902ff

File tree

14 files changed

+142
-163
lines changed

14 files changed

+142
-163
lines changed

‎Sources/Basics/HTTPClient.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,16 @@ public struct HTTPClient: HTTPClientProtocol {
5656
public typealias Handler = (Request, ProgressHandler?, @escaping (Result<Response, Error>) -> Void) -> Void
5757

5858
public var configuration: HTTPClientConfiguration
59-
private let diagnosticsEngine: DiagnosticsEngine?
59+
private let observabilityScope: ObservabilityScope
6060
private let underlying: Handler
6161

6262
// static to share across instances of the http client
6363
private static var hostsErrorsLock = Lock()
6464
private static var hostsErrors = [String: [Date]]()
6565

66-
public init(configuration: HTTPClientConfiguration = .init(), handler: Handler? = nil, diagnosticsEngine: DiagnosticsEngine? = nil) {
66+
public init(configuration: HTTPClientConfiguration = .init(), handler: Handler? = nil, observabilityScope: ObservabilityScope = ObservabilitySystem.topScope) {
6767
self.configuration = configuration
68-
self.diagnosticsEngine = diagnosticsEngine
68+
self.observabilityScope = observabilityScope
6969
// FIXME: inject platform specific implementation here
7070
self.underlying = handler ?? URLSessionHTTPClient().execute
7171
}
@@ -121,7 +121,7 @@ public struct HTTPClient: HTTPClientProtocol {
121121

122122
private func _execute(request: Request, requestNumber: Int, progress: ProgressHandler?, completion: @escaping CompletionHandler) {
123123
if self.shouldCircuitBreak(request: request) {
124-
diagnosticsEngine?.emit(warning: "Circuit breaker triggered for \(request.url)")
124+
self.observabilityScope.emit(warning: "Circuit breaker triggered for \(request.url)")
125125
return completion(.failure(HTTPClientError.circuitBreakerTriggered))
126126
}
127127

@@ -145,7 +145,7 @@ public struct HTTPClient: HTTPClientProtocol {
145145
self.recordErrorIfNecessary(response: response, request: request)
146146
// handle retry strategy
147147
if let retryDelay = self.shouldRetry(response: response, request: request, requestNumber: requestNumber) {
148-
self.diagnosticsEngine?.emit(warning: "\(request.url) failed, retrying in \(retryDelay)")
148+
self.observabilityScope.emit(warning: "\(request.url) failed, retrying in \(retryDelay)")
149149
// TODO: dedicated retry queue?
150150
return self.configuration.callbackQueue.asyncAfter(deadline: .now() + retryDelay) {
151151
self._execute(request: request, requestNumber: requestNumber + 1, progress: progress, completion: completion)

‎Sources/Basics/SQLiteBackedCache.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public final class SQLiteBackedCache<Value: Codable>: Closable {
2525
private var state = State.idle
2626
private let stateLock = Lock()
2727

28-
private let diagnosticsEngine: DiagnosticsEngine?
28+
private let observabilityScope: ObservabilityScope
2929
private let jsonEncoder: JSONEncoder
3030
private let jsonDecoder: JSONDecoder
3131

@@ -36,7 +36,7 @@ public final class SQLiteBackedCache<Value: Codable>: Closable {
3636
/// - location: SQLite.Location
3737
/// - configuration: Optional. Configuration for the cache.
3838
/// - diagnosticsEngine: DiagnosticsEngine
39-
public init(tableName: String, location: SQLite.Location, configuration: SQLiteBackedCacheConfiguration = .init(), diagnosticsEngine: DiagnosticsEngine? = nil) {
39+
public init(tableName: String, location: SQLite.Location, configuration: SQLiteBackedCacheConfiguration = .init(), observabilityScope: ObservabilityScope = ObservabilitySystem.topScope) {
4040
self.tableName = tableName
4141
self.location = location
4242
switch self.location {
@@ -46,7 +46,7 @@ public final class SQLiteBackedCache<Value: Codable>: Closable {
4646
self.fileSystem = InMemoryFileSystem()
4747
}
4848
self.configuration = configuration
49-
self.diagnosticsEngine = diagnosticsEngine
49+
self.observabilityScope = observabilityScope
5050
self.jsonEncoder = JSONEncoder.makeWithDefaults()
5151
self.jsonDecoder = JSONDecoder.makeWithDefaults()
5252
}
@@ -58,8 +58,8 @@ public final class SQLiteBackedCache<Value: Codable>: Closable {
5858
/// - path: The path of the SQLite database.
5959
/// - configuration: Optional. Configuration for the cache.
6060
/// - diagnosticsEngine: DiagnosticsEngine
61-
public convenience init(tableName: String, path: AbsolutePath, configuration: SQLiteBackedCacheConfiguration = .init(), diagnosticsEngine: DiagnosticsEngine? = nil) {
62-
self.init(tableName: tableName, location: .path(path), configuration: configuration, diagnosticsEngine: diagnosticsEngine)
61+
public convenience init(tableName: String, path: AbsolutePath, configuration: SQLiteBackedCacheConfiguration = .init(), observabilityScope: ObservabilityScope = ObservabilitySystem.topScope) {
62+
self.init(tableName: tableName, location: .path(path), configuration: configuration, observabilityScope: observabilityScope)
6363
}
6464

6565
deinit {
@@ -97,7 +97,7 @@ public final class SQLiteBackedCache<Value: Codable>: Closable {
9797
if !self.configuration.truncateWhenFull {
9898
throw error
9999
}
100-
self.diagnosticsEngine?.emit(warning: "truncating \(self.tableName) cache database since it reached max size of \(self.configuration.maxSizeInBytes ?? 0) bytes")
100+
self.observabilityScope.emit(warning: "truncating \(self.tableName) cache database since it reached max size of \(self.configuration.maxSizeInBytes ?? 0) bytes")
101101
try self.executeStatement("DELETE FROM \(self.tableName);") { statement -> Void in
102102
try statement.step()
103103
}

‎Sources/PackageCollections/PackageCollections.swift

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ public struct PackageCollections: PackageCollectionsProtocol {
2222
#endif
2323

2424
let configuration: Configuration
25-
private let diagnosticsEngine: DiagnosticsEngine?
2625
private let storageContainer: (storage: Storage, owned: Bool)
2726
private let collectionProviders: [Model.CollectionSourceType: PackageCollectionProvider]
2827
let metadataProvider: PackageMetadataProvider
@@ -32,17 +31,17 @@ public struct PackageCollections: PackageCollectionsProtocol {
3231
}
3332

3433
// initialize with defaults
35-
public init(configuration: Configuration = .init(), diagnosticsEngine: DiagnosticsEngine = ObservabilitySystem.topScope.makeDiagnosticsEngine()) {
36-
let storage = Storage(sources: FilePackageCollectionsSourcesStorage(diagnosticsEngine: diagnosticsEngine),
37-
collections: SQLitePackageCollectionsStorage(diagnosticsEngine: diagnosticsEngine))
34+
public init(configuration: Configuration = .init()) {
35+
let storage = Storage(
36+
sources: FilePackageCollectionsSourcesStorage(),
37+
collections: SQLitePackageCollectionsStorage()
38+
)
3839

39-
let collectionProviders = [Model.CollectionSourceType.json: JSONPackageCollectionProvider(diagnosticsEngine: diagnosticsEngine)]
40+
let collectionProviders = [Model.CollectionSourceType.json: JSONPackageCollectionProvider()]
4041

41-
let metadataProvider = GitHubPackageMetadataProvider(configuration: .init(authTokens: configuration.authTokens),
42-
diagnosticsEngine: diagnosticsEngine)
42+
let metadataProvider = GitHubPackageMetadataProvider(configuration: .init(authTokens: configuration.authTokens))
4343

4444
self.configuration = configuration
45-
self.diagnosticsEngine = diagnosticsEngine
4645
self.storageContainer = (storage, true)
4746
self.collectionProviders = collectionProviders
4847
self.metadataProvider = metadataProvider
@@ -55,7 +54,6 @@ public struct PackageCollections: PackageCollectionsProtocol {
5554
collectionProviders: [Model.CollectionSourceType: PackageCollectionProvider],
5655
metadataProvider: PackageMetadataProvider) {
5756
self.configuration = configuration
58-
self.diagnosticsEngine = diagnosticsEngine
5957
self.storageContainer = (storage, false)
6058
self.collectionProviders = collectionProviders
6159
self.metadataProvider = metadataProvider
@@ -384,7 +382,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
384382
self.metadataProvider.get(identity: packageSearchResult.package.identity, location: packageSearchResult.package.location) { result in
385383
switch result {
386384
case .failure(let error):
387-
self.diagnosticsEngine?.emit(warning: "Failed fetching information about \(identity) from \(self.metadataProvider.name): \(error)")
385+
ObservabilitySystem.topScope.emit(warning: "Failed fetching information about \(identity) from \(self.metadataProvider.name): \(error)")
388386

389387
let provider: PackageMetadataProviderContext?
390388
switch error {

‎Sources/PackageCollections/Providers/GitHubPackageMetadataProvider.swift

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,18 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
2626
let configuration: Configuration
2727

2828
private let httpClient: HTTPClient
29-
private let diagnosticsEngine: DiagnosticsEngine?
3029
private let decoder: JSONDecoder
3130

3231
private let cache: SQLiteBackedCache<CacheValue>?
3332

34-
init(configuration: Configuration = .init(), httpClient: HTTPClient? = nil, diagnosticsEngine: DiagnosticsEngine? = nil) {
33+
init(configuration: Configuration = .init(), httpClient: HTTPClient? = nil) {
3534
self.configuration = configuration
36-
self.httpClient = httpClient ?? Self.makeDefaultHTTPClient(diagnosticsEngine: diagnosticsEngine)
37-
self.diagnosticsEngine = diagnosticsEngine
35+
self.httpClient = httpClient ?? Self.makeDefaultHTTPClient()
3836
self.decoder = JSONDecoder.makeWithDefaults()
3937
if configuration.cacheTTLInSeconds > 0 {
4038
var cacheConfig = SQLiteBackedCacheConfiguration()
4139
cacheConfig.maxSizeInMegabytes = configuration.cacheSizeInMegabytes
42-
self.cache = SQLiteBackedCache<CacheValue>(tableName: "github_cache", path: configuration.cacheDir.appending(component: "package-metadata.db"), configuration: cacheConfig, diagnosticsEngine: diagnosticsEngine)
40+
self.cache = SQLiteBackedCache<CacheValue>(tableName: "github_cache", path: configuration.cacheDir.appending(component: "package-metadata.db"), configuration: cacheConfig)
4341
} else {
4442
self.cache = nil
4543
}
@@ -85,7 +83,7 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
8583
let apiRemaining = response.headers.get("X-RateLimit-Remaining").first.flatMap(Int.init) ?? -1
8684
switch (response.statusCode, hasAuthorization, apiRemaining) {
8785
case (_, _, 0):
88-
self.diagnosticsEngine?.emit(warning: "Exceeded API limits on \(metadataURL.host ?? metadataURL.absoluteString) (\(apiRemaining)/\(apiLimit)), consider configuring an API token for this service.")
86+
ObservabilitySystem.topScope.emit(warning: "Exceeded API limits on \(metadataURL.host ?? metadataURL.absoluteString) (\(apiRemaining)/\(apiLimit)), consider configuring an API token for this service.")
8987
results[metadataURL] = .failure(Errors.apiLimitsExceeded(metadataURL, apiLimit))
9088
case (401, true, _):
9189
results[metadataURL] = .failure(Errors.invalidAuthToken(metadataURL))
@@ -97,7 +95,7 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
9795
results[metadataURL] = .failure(NotFoundError("\(baseURL)"))
9896
case (200, _, _):
9997
if apiRemaining < self.configuration.apiLimitWarningThreshold {
100-
self.diagnosticsEngine?.emit(warning: "Approaching API limits on \(metadataURL.host ?? metadataURL.absoluteString) (\(apiRemaining)/\(apiLimit)), consider configuring an API token for this service.")
98+
ObservabilitySystem.topScope.emit(warning: "Approaching API limits on \(metadataURL.host ?? metadataURL.absoluteString) (\(apiRemaining)/\(apiLimit)), consider configuring an API token for this service.")
10199
}
102100
// if successful, fan out multiple API calls
103101
[releasesURL, contributorsURL, readmeURL, licenseURL, languagesURL].forEach { url in
@@ -156,7 +154,7 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
156154
do {
157155
try self.cache?.put(key: identity.description, value: CacheValue(package: model, timestamp: DispatchTime.now()), replace: true)
158156
} catch {
159-
self.diagnosticsEngine?.emit(warning: "Failed to save GitHub metadata for package \(identity) to cache: \(error)")
157+
ObservabilitySystem.topScope.emit(warning: "Failed to save GitHub metadata for package \(identity) to cache: \(error)")
160158
}
161159

162160
callback(.success(model))
@@ -218,8 +216,8 @@ struct GitHubPackageMetadataProvider: PackageMetadataProvider {
218216
return .github(host)
219217
}
220218

221-
private static func makeDefaultHTTPClient(diagnosticsEngine: DiagnosticsEngine?) -> HTTPClient {
222-
var client = HTTPClient(diagnosticsEngine: diagnosticsEngine)
219+
private static func makeDefaultHTTPClient() -> HTTPClient {
220+
var client = HTTPClient()
223221
// TODO: make these defaults configurable?
224222
client.configuration.requestTimeout = .seconds(1)
225223
client.configuration.retryStrategy = .exponentialBackoff(maxAttempts: 3, baseDelay: .milliseconds(50))

‎Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
3636
static let defaultCertPolicyKeys: [CertificatePolicyKey] = [.default]
3737

3838
private let configuration: Configuration
39-
private let diagnosticsEngine: DiagnosticsEngine
4039
private let httpClient: HTTPClient
4140
private let decoder: JSONDecoder
4241
private let validator: JSONModel.Validator
@@ -47,18 +46,15 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
4746
httpClient: HTTPClient? = nil,
4847
signatureValidator: PackageCollectionSignatureValidator? = nil,
4948
sourceCertPolicy: PackageCollectionSourceCertificatePolicy = PackageCollectionSourceCertificatePolicy(),
50-
fileSystem: FileSystem = localFileSystem,
51-
diagnosticsEngine: DiagnosticsEngine) {
49+
fileSystem: FileSystem = localFileSystem) {
5250
self.configuration = configuration
53-
self.diagnosticsEngine = diagnosticsEngine
54-
self.httpClient = httpClient ?? Self.makeDefaultHTTPClient(diagnosticsEngine: diagnosticsEngine)
51+
self.httpClient = httpClient ?? Self.makeDefaultHTTPClient()
5552
self.decoder = JSONDecoder.makeWithDefaults()
5653
self.validator = JSONModel.Validator(configuration: configuration.validator)
5754
self.signatureValidator = signatureValidator ?? PackageCollectionSigning(
5855
trustedRootCertsDir: configuration.trustedRootCertsDir ?? fileSystem.swiftPMConfigDirectory.appending(component: "trust-root-certs").asURL,
5956
additionalTrustedRootCerts: sourceCertPolicy.allRootCerts.map { Array($0) },
60-
callbackQueue: .sharedConcurrent,
61-
diagnosticsEngine: diagnosticsEngine
57+
callbackQueue: .sharedConcurrent
6258
)
6359
self.sourceCertPolicy = sourceCertPolicy
6460
}
@@ -168,7 +164,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
168164
fatalError("Expected at least one package collection signature validation failure but got none")
169165
}
170166

171-
self.diagnosticsEngine.emit(warning: "The signature of package collection [\(source)] is invalid: \(error)")
167+
ObservabilitySystem.topScope.emit(warning: "The signature of package collection [\(source)] is invalid: \(error)")
172168
if PackageCollectionSigningError.noTrustedRootCertsConfigured == error as? PackageCollectionSigningError {
173169
callback(.failure(PackageCollectionError.cannotVerifySignature))
174170
} else {
@@ -272,7 +268,7 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
272268
}
273269

274270
if !serializationOkay {
275-
self.diagnosticsEngine.emit(warning: "Some of the information from \(collection.name) could not be deserialized correctly, likely due to invalid format. Contact the collection's author (\(collection.generatedBy?.name ?? "n/a")) to address this issue.")
271+
ObservabilitySystem.topScope.emit(warning: "Some of the information from \(collection.name) could not be deserialized correctly, likely due to invalid format. Contact the collection's author (\(collection.generatedBy?.name ?? "n/a")) to address this issue.")
276272
}
277273

278274
return .success(.init(source: source,
@@ -300,8 +296,8 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
300296
return headers
301297
}
302298

303-
private static func makeDefaultHTTPClient(diagnosticsEngine: DiagnosticsEngine?) -> HTTPClient {
304-
var client = HTTPClient(diagnosticsEngine: diagnosticsEngine)
299+
private static func makeDefaultHTTPClient() -> HTTPClient {
300+
var client = HTTPClient()
305301
// TODO: make these defaults configurable?
306302
client.configuration.requestTimeout = .seconds(5)
307303
client.configuration.retryStrategy = .exponentialBackoff(maxAttempts: 3, baseDelay: .milliseconds(50))

‎Sources/PackageCollections/Storage/FilePackageCollectionsSourcesStorage.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,13 @@ struct FilePackageCollectionsSourcesStorage: PackageCollectionsSourcesStorage {
2020
let fileSystem: FileSystem
2121
let path: AbsolutePath
2222

23-
private let diagnosticsEngine: DiagnosticsEngine?
2423
private let encoder: JSONEncoder
2524
private let decoder: JSONDecoder
2625

27-
init(fileSystem: FileSystem = localFileSystem, path: AbsolutePath? = nil, diagnosticsEngine: DiagnosticsEngine? = nil) {
26+
init(fileSystem: FileSystem = localFileSystem, path: AbsolutePath? = nil) {
2827
self.fileSystem = fileSystem
2928

3029
self.path = path ?? fileSystem.swiftPMConfigDirectory.appending(component: "collections.json")
31-
self.diagnosticsEngine = diagnosticsEngine
3230
self.encoder = JSONEncoder.makeWithDefaults()
3331
self.decoder = JSONDecoder.makeWithDefaults()
3432
}

0 commit comments

Comments
 (0)