From 11d73cc2b4d65d4ad3f8f0c0b99a409b4d1c361d Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 19 Mar 2025 11:19:06 +0100 Subject: [PATCH 1/3] Make AppMetrics dependency based --- Sources/App/Core/AppMetrics.swift | 29 ++++------ .../Core/Dependencies/AppMetricsClient.swift | 54 +++++++++++++++++++ Sources/App/configure.swift | 2 +- Sources/App/routes.swift | 3 +- Tests/AppTests/AllTests.swift | 5 +- Tests/AppTests/MetricsTests.swift | 1 - 6 files changed, 71 insertions(+), 23 deletions(-) create mode 100644 Sources/App/Core/Dependencies/AppMetricsClient.swift diff --git a/Sources/App/Core/AppMetrics.swift b/Sources/App/Core/AppMetrics.swift index 2c3fb00d1..b97fc234e 100644 --- a/Sources/App/Core/AppMetrics.swift +++ b/Sources/App/Core/AppMetrics.swift @@ -21,20 +21,6 @@ import Vapor enum AppMetrics { - static let initialized = Mutex(false) - - static func bootstrap() { - // prevent tests from boostrapping multiple times - guard !initialized.withLock({ $0 }) else { return } - initialized.withLock { - let client = PrometheusClient() - MetricsSystem.bootstrap(PrometheusMetricsFactory(client: client)) - $0 = true - } - } - - // metrics - static var analyzeCandidatesCount: PromGauge? { gauge("spi_analyze_candidates_count") } @@ -155,13 +141,13 @@ enum AppMetrics { extension AppMetrics { static func counter(_ name: String) -> PromCounter? { - try? MetricsSystem.prometheus() - .createCounter(forType: V.self, named: name) + @Dependency(\.prometheus) var prometheus + return prometheus?.createCounter(forType: V.self, named: name) } static func gauge(_ name: String) -> PromGauge? { - try? MetricsSystem.prometheus() - .createGauge(forType: V.self, named: name) + @Dependency(\.prometheus) var prometheus + return prometheus?.createGauge(forType: V.self, named: name) } } @@ -176,6 +162,11 @@ extension AppMetrics { static func push(client: Client, jobName: String) async throws { @Dependency(\.environment) var environment @Dependency(\.logger) var logger + @Dependency(\.prometheus) var prometheus + + guard let prometheus else { + throw AppError.genericError(nil, "Prometheus client unavailable (nil)") + } guard let pushGatewayUrl = environment.metricsPushGatewayUrl() else { throw AppError.envVariableNotSet("METRICS_PUSHGATEWAY_URL") @@ -183,7 +174,7 @@ extension AppMetrics { let url = URI(string: "\(pushGatewayUrl)/metrics/job/\(jobName)") do { - let metrics: String = try await MetricsSystem.prometheus().collect() + let metrics: String = await prometheus.collect() _ = try await client.post(url) { req in // append "\n" to avoid // text format parsing error in line 4: unexpected end of input stream diff --git a/Sources/App/Core/Dependencies/AppMetricsClient.swift b/Sources/App/Core/Dependencies/AppMetricsClient.swift new file mode 100644 index 000000000..21a6c3759 --- /dev/null +++ b/Sources/App/Core/Dependencies/AppMetricsClient.swift @@ -0,0 +1,54 @@ +// Copyright Dave Verwer, Sven A. Schmidt, and other contributors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import Dependencies +import Metrics +import Synchronization +@preconcurrency import Prometheus + + +enum AppMetricsClient { + private static let initialized = Mutex(false) + + static func bootstrap() { + guard !initialized.withLock({ $0 }) else { return } + initialized.withLock { + let client = PrometheusClient() + MetricsSystem.bootstrap(PrometheusMetricsFactory(client: client)) + $0 = true + } + } +} + + +extension AppMetricsClient: DependencyKey { + static var liveValue: PrometheusClient? { + try? MetricsSystem.prometheus() + } +} + + +extension AppMetricsClient: TestDependencyKey { + static var testValue: PrometheusClient? { + unimplemented("testValue"); return nil + } +} + + +extension DependencyValues { + public var prometheus: PrometheusClient? { + get { self[AppMetricsClient.self] } + set { self[AppMetricsClient.self] = newValue } + } +} diff --git a/Sources/App/configure.swift b/Sources/App/configure.swift index e3b3f0716..18b0ed44e 100644 --- a/Sources/App/configure.swift +++ b/Sources/App/configure.swift @@ -365,7 +365,7 @@ public func configure(_ app: Application, databasePort: Int? = nil) async throws try routes(app) // bootstrap app metrics - AppMetrics.bootstrap() + AppMetricsClient.bootstrap() return host } diff --git a/Sources/App/routes.swift b/Sources/App/routes.swift index dbfb05536..ea29128af 100644 --- a/Sources/App/routes.swift +++ b/Sources/App/routes.swift @@ -211,7 +211,8 @@ func routes(_ app: Application) throws { do { // Metrics app.get("metrics") { req -> String in - try await MetricsSystem.prometheus().collect() + @Dependency(\.prometheus) var prometheus + return await prometheus?.collect() ?? "" } } } diff --git a/Tests/AppTests/AllTests.swift b/Tests/AppTests/AllTests.swift index 6800ebdb2..d92ab582b 100644 --- a/Tests/AppTests/AllTests.swift +++ b/Tests/AppTests/AllTests.swift @@ -16,7 +16,10 @@ import Testing import Dependencies -@Suite(.dependency(\.date.now, .t0)) struct AllTests { } +@Suite( + .dependency(\.date.now, .t0), + .dependency(\.prometheus, .init()) +) struct AllTests { } extension AllTests { diff --git a/Tests/AppTests/MetricsTests.swift b/Tests/AppTests/MetricsTests.swift index 3d81d52f4..6328f9ac3 100644 --- a/Tests/AppTests/MetricsTests.swift +++ b/Tests/AppTests/MetricsTests.swift @@ -164,7 +164,6 @@ extension AllTests.MetricsTests { // validation #expect((AppMetrics.buildTriggerDurationSeconds?.get()) ?? 0 > 0) - print(AppMetrics.buildTriggerDurationSeconds!.get()) } } } From cc255c6b34dcda2942b726a37015d7389fcda066 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 19 Mar 2025 11:40:19 +0100 Subject: [PATCH 2/3] Change dependency to intercept at MetricsSystem level --- Sources/App/Core/AppMetrics.swift | 16 +++----- ...Client.swift => MetricsSystemClient.swift} | 41 +++++++++++++------ Sources/App/configure.swift | 3 +- Sources/App/routes.swift | 4 +- Tests/AppTests/AllTests.swift | 6 ++- 5 files changed, 42 insertions(+), 28 deletions(-) rename Sources/App/Core/Dependencies/{AppMetricsClient.swift => MetricsSystemClient.swift} (53%) diff --git a/Sources/App/Core/AppMetrics.swift b/Sources/App/Core/AppMetrics.swift index b97fc234e..861a46530 100644 --- a/Sources/App/Core/AppMetrics.swift +++ b/Sources/App/Core/AppMetrics.swift @@ -141,13 +141,13 @@ enum AppMetrics { extension AppMetrics { static func counter(_ name: String) -> PromCounter? { - @Dependency(\.prometheus) var prometheus - return prometheus?.createCounter(forType: V.self, named: name) + @Dependency(\.metricsSystem.prometheus) var prometheus + return try? prometheus().createCounter(forType: V.self, named: name) } static func gauge(_ name: String) -> PromGauge? { - @Dependency(\.prometheus) var prometheus - return prometheus?.createGauge(forType: V.self, named: name) + @Dependency(\.metricsSystem.prometheus) var prometheus + return try? prometheus().createGauge(forType: V.self, named: name) } } @@ -162,11 +162,7 @@ extension AppMetrics { static func push(client: Client, jobName: String) async throws { @Dependency(\.environment) var environment @Dependency(\.logger) var logger - @Dependency(\.prometheus) var prometheus - - guard let prometheus else { - throw AppError.genericError(nil, "Prometheus client unavailable (nil)") - } + @Dependency(\.metricsSystem.prometheus) var prometheus guard let pushGatewayUrl = environment.metricsPushGatewayUrl() else { throw AppError.envVariableNotSet("METRICS_PUSHGATEWAY_URL") @@ -174,7 +170,7 @@ extension AppMetrics { let url = URI(string: "\(pushGatewayUrl)/metrics/job/\(jobName)") do { - let metrics: String = await prometheus.collect() + let metrics: String = try await prometheus().collect() _ = try await client.post(url) { req in // append "\n" to avoid // text format parsing error in line 4: unexpected end of input stream diff --git a/Sources/App/Core/Dependencies/AppMetricsClient.swift b/Sources/App/Core/Dependencies/MetricsSystemClient.swift similarity index 53% rename from Sources/App/Core/Dependencies/AppMetricsClient.swift rename to Sources/App/Core/Dependencies/MetricsSystemClient.swift index 21a6c3759..ac64f26da 100644 --- a/Sources/App/Core/Dependencies/AppMetricsClient.swift +++ b/Sources/App/Core/Dependencies/MetricsSystemClient.swift @@ -18,12 +18,17 @@ import Synchronization @preconcurrency import Prometheus -enum AppMetricsClient { +struct MetricsSystemClient { + var prometheus: @Sendable () throws -> PrometheusClient +} + + +extension MetricsSystemClient { private static let initialized = Mutex(false) - static func bootstrap() { - guard !initialized.withLock({ $0 }) else { return } - initialized.withLock { + func bootstrap() { + guard !Self.initialized.withLock({ $0 }) else { return } + Self.initialized.withLock { let client = PrometheusClient() MetricsSystem.bootstrap(PrometheusMetricsFactory(client: client)) $0 = true @@ -32,23 +37,33 @@ enum AppMetricsClient { } -extension AppMetricsClient: DependencyKey { - static var liveValue: PrometheusClient? { - try? MetricsSystem.prometheus() +extension MetricsSystemClient: DependencyKey { + static var liveValue: Self { + .init(prometheus: { try MetricsSystem.prometheus() }) } } -extension AppMetricsClient: TestDependencyKey { - static var testValue: PrometheusClient? { - unimplemented("testValue"); return nil +extension MetricsSystemClient: TestDependencyKey { + static var testValue: Self { + .init(prometheus: { unimplemented("testValue"); return .init() }) } } extension DependencyValues { - public var prometheus: PrometheusClient? { - get { self[AppMetricsClient.self] } - set { self[AppMetricsClient.self] = newValue } + var metricsSystem: MetricsSystemClient { + get { self[MetricsSystemClient.self] } + set { self[MetricsSystemClient.self] = newValue } + } +} + + +#if DEBUG +extension MetricsSystemClient { + static var mock: Self { + let prometheus = PrometheusClient() + return .init(prometheus: { prometheus }) } } +#endif diff --git a/Sources/App/configure.swift b/Sources/App/configure.swift index 18b0ed44e..2d442bb8d 100644 --- a/Sources/App/configure.swift +++ b/Sources/App/configure.swift @@ -365,7 +365,8 @@ public func configure(_ app: Application, databasePort: Int? = nil) async throws try routes(app) // bootstrap app metrics - AppMetricsClient.bootstrap() + @Dependency(\.metricsSystem) var metricsSystem + metricsSystem.bootstrap() return host } diff --git a/Sources/App/routes.swift b/Sources/App/routes.swift index ea29128af..f21c9e372 100644 --- a/Sources/App/routes.swift +++ b/Sources/App/routes.swift @@ -211,8 +211,8 @@ func routes(_ app: Application) throws { do { // Metrics app.get("metrics") { req -> String in - @Dependency(\.prometheus) var prometheus - return await prometheus?.collect() ?? "" + @Dependency(\.metricsSystem.prometheus) var prometheus + return try await prometheus().collect() } } } diff --git a/Tests/AppTests/AllTests.swift b/Tests/AppTests/AllTests.swift index d92ab582b..a7f4c67d0 100644 --- a/Tests/AppTests/AllTests.swift +++ b/Tests/AppTests/AllTests.swift @@ -12,13 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -import Testing +@testable import App + import Dependencies +import Testing @Suite( .dependency(\.date.now, .t0), - .dependency(\.prometheus, .init()) + .dependency(\.metricsSystem, .mock) ) struct AllTests { } From 342e951cbcd0f3942a54da1abc8efcdabb983cc3 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Wed, 19 Mar 2025 12:19:42 +0100 Subject: [PATCH 3/3] Don't need to baseline counts anymore --- Tests/AppTests/MetricsTests.swift | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/Tests/AppTests/MetricsTests.swift b/Tests/AppTests/MetricsTests.swift index 6328f9ac3..5f9e82cd2 100644 --- a/Tests/AppTests/MetricsTests.swift +++ b/Tests/AppTests/MetricsTests.swift @@ -61,18 +61,6 @@ extension AllTests.MetricsTests { @Test func versions_added() async throws { try await withApp { app in // setup - let initialAddedBranch = try #require( - AppMetrics.analyzeVersionsAddedCount?.get(.versionLabels(kind: .branch)) - ) - let initialAddedTag = try #require( - AppMetrics.analyzeVersionsAddedCount?.get(.versionLabels(kind: .tag)) - ) - let initialDeletedBranch = try #require( - AppMetrics.analyzeVersionsDeletedCount?.get(.versionLabels(kind: .branch)) - ) - let initialDeletedTag = try #require( - AppMetrics.analyzeVersionsDeletedCount?.get(.versionLabels(kind: .tag)) - ) let pkg = try await savePackage(on: app.db, "1") let new = [ try Version(package: pkg, reference: .branch("main")), @@ -91,16 +79,16 @@ extension AllTests.MetricsTests { // validation #expect( - AppMetrics.analyzeVersionsAddedCount?.get(.versionLabels(kind: .branch)) == initialAddedBranch + 1 + AppMetrics.analyzeVersionsAddedCount?.get(.versionLabels(kind: .branch)) == 1 ) #expect( - AppMetrics.analyzeVersionsAddedCount?.get(.versionLabels(kind: .tag)) == initialAddedTag + 2 + AppMetrics.analyzeVersionsAddedCount?.get(.versionLabels(kind: .tag)) == 2 ) #expect( - AppMetrics.analyzeVersionsDeletedCount?.get(.versionLabels(kind: .branch)) == initialDeletedBranch + 1 + AppMetrics.analyzeVersionsDeletedCount?.get(.versionLabels(kind: .branch)) == 1 ) #expect( - AppMetrics.analyzeVersionsDeletedCount?.get(.versionLabels(kind: .tag)) == initialDeletedTag + 1 + AppMetrics.analyzeVersionsDeletedCount?.get(.versionLabels(kind: .tag)) == 1 ) } }