diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index f5e9a0e64..bf7628760 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -732,19 +732,40 @@ static function (ContainerInterface $c) { } ); - // Build the thin openconnector MetricsController subclass (URL - // /api/metrics, route name metrics#index — both unchanged). Admin-only - // posture is engine-owned and re-declared on the subclass method. + // Build the thin openconnector MetricsController (URL /api/metrics, + // route name metrics#index — both unchanged) with the engine delegate + // resolved from OpenRegister's app container, scoped to this app's + // manifest via appName. Admin-only posture is engine-owned and + // re-declared on the openconnector method. $context->registerService( MetricsController::class, static function (ContainerInterface $c) { - // phpcs:ignore CustomSniffs.Nextcloud.NoLegacyServerAccessors.LegacyNamedAccessor -- cross-app DI container lookup; no \OCP\Server equivalent, still used by NC34 core (OCP\AppFramework\App). - $orContainer = \OC::$server->getRegisteredAppContainer('openregister'); + $appManager = $c->get(\OCP\App\IAppManager::class); + + // Mirrors the HealthController guard above. When OpenRegister + // is absent the engine delegate cannot be built, so pass null + // and let MetricsController return a clean 503 instead of a + // bare DI 500 — getRegisteredAppContainer() throws for an app + // that is not registered. Building the delegate references + // OpenRegister classes, so it is only done when OpenRegister + // is enabled. + $delegate = null; + if ($appManager->isInstalled('openregister') === true) { + // phpcs:ignore CustomSniffs.Nextcloud.NoLegacyServerAccessors.LegacyNamedAccessor -- cross-app DI container lookup; no \OCP\Server equivalent, still used by NC34 core (OCP\AppFramework\App). + $orContainer = \OC::$server->getRegisteredAppContainer('openregister'); + $delegate = new \OCA\OpenRegister\AppHost\Controller\GenericMetricsController( + appName: self::APP_ID, + request: $c->get(IRequest::class), + manifestLoader: $orContainer->get(\OCA\OpenRegister\AppHost\Observability\ManifestLoader::class), + engine: $orContainer->get(\OCA\OpenRegister\AppHost\Observability\MetricsEngine::class) + ); + } + return new MetricsController( appName: self::APP_ID, request: $c->get(IRequest::class), - manifestLoader: $orContainer->get(\OCA\OpenRegister\AppHost\Observability\ManifestLoader::class), - engine: $orContainer->get(\OCA\OpenRegister\AppHost\Observability\MetricsEngine::class) + appManager: $appManager, + delegate: $delegate ); } ); @@ -1078,6 +1099,8 @@ private function assertStorageMigrated(): void * @param IBootContext $context Boot context. * * @return void + * + * @spec openspec/specs/apphost-adoption/spec.md */ public function boot(IBootContext $context): void { diff --git a/lib/Controller/MetricsController.php b/lib/Controller/MetricsController.php index 2c63ff6e4..a6873e5c1 100644 --- a/lib/Controller/MetricsController.php +++ b/lib/Controller/MetricsController.php @@ -1,23 +1,35 @@ appManager->isInstalled(self::REQUIRED_APP) === false || $this->delegate === null) { + $response = new TextPlainResponse( + '# metrics unavailable: OpenConnector requires the OpenRegister app — install and enable it.'."\n", + Http::STATUS_SERVICE_UNAVAILABLE + ); + $response->addHeader('Content-Type', self::CONTENT_TYPE); + + return $response; + } + + return $this->delegate->index(); + }//end index() }//end class diff --git a/tests/Unit/Observability/OpenConnectorMetricsProviderTest.php b/tests/Unit/Observability/OpenConnectorMetricsProviderTest.php index 4c99fce01..07e849548 100644 --- a/tests/Unit/Observability/OpenConnectorMetricsProviderTest.php +++ b/tests/Unit/Observability/OpenConnectorMetricsProviderTest.php @@ -69,6 +69,43 @@ function (string $id) use ($objectService, $schemaMapper, $registerMapper) { }//end buildProvider() + /** + * Every provider-backed metric NAME this provider is expected to emit. + * + * One MetricSample per name, regardless of how many points each carries. + * Asserting the name SET rather than a bare count means adding or losing a + * metric fails with a diff that names it, instead of an opaque + * "6 does not match 3" — which is how this assertion silently rotted when + * #1126 revived sources_total / calls_total / synchronization_runs_total. + * + * @var string[] + */ + private const EXPECTED_METRIC_NAMES = [ + 'api_product_errors_total', + 'api_product_latency_seconds', + 'calls_total', + 'circuit_breaker_state', + 'sources_total', + 'synchronization_runs_total', + ]; + + + /** + * Assert the provider emitted exactly the expected set of metric names. + * + * @param array $samples MetricSample objects returned by the provider. + * + * @return void + */ + private function assertSameMetricNames(array $samples): void + { + $names = array_map(static fn (object $s): string => $s->name, $samples); + sort($names); + + $this->assertSame(self::EXPECTED_METRIC_NAMES, $names); + }//end assertSameMetricNames() + + /** * TC-19 — mixed open/closed/never-evaluated sources report 1/0/0. * @@ -94,11 +131,7 @@ public function searchObjects(array $query, bool $_rbac=true, bool $_multitenanc ); $samples = $provider->metrics(); - // api-product-gateway added two more provider-backed gauges - // (api_product_latency_seconds, api_product_errors_total) alongside - // the pre-existing circuit_breaker_state — 3 MetricSample objects - // (one per metric NAME) regardless of how many points each carries. - $this->assertCount(3, $samples); + $this->assertSameMetricNames($samples); $sample = $this->sampleByName($samples, 'circuit_breaker_state'); $this->assertSame('circuit_breaker_state', $sample->name); @@ -138,7 +171,7 @@ public function searchObjects(array $query, bool $_rbac=true, bool $_multitenanc $samples = $provider->metrics(); - $this->assertCount(3, $samples); + $this->assertSameMetricNames($samples); $sample = $this->sampleByName($samples, 'circuit_breaker_state'); $this->assertCount(1, $sample->samples); $this->assertSame(0, $sample->samples[0]['value']); @@ -157,7 +190,7 @@ public function testUnavailableObjectServiceFallsBackToZeroValue(): void $samples = $provider->metrics(); - $this->assertCount(3, $samples); + $this->assertSameMetricNames($samples); $circuitBreaker = $this->sampleByName($samples, 'circuit_breaker_state'); $this->assertSame(0, $circuitBreaker->samples[0]['value']); }//end testUnavailableObjectServiceFallsBackToZeroValue()