fix(metrics): avoid leaking scrape connections

Run metrics scrape queries through a cloned force-sync Manticore client so the shared async client does not retain one internal 127.0.0.1:9312 socket per /metrics request.

Related issue #686

Author: sanikolaev

src/Plugin/Metrics/Handler.php

@@ -52,7 +52,9 @@ public function run(): Task {
 			$definitions = $decoded;
 			$store = new MetricStore($definitions);
 			$context = new MetricsScrapeContext();
-			$settings = $client->getSettings();
+			$metricsClient = clone $client;
+			$metricsClient->setForceSync(true);
+			$settings = $metricsClient->getSettings();
 			$context->settings = [
 				'searchd.data_dir' => (string)($settings->searchdDataDir ?? ''),
 				'searchd.binlog_path' => (string)($settings->searchdBinlogPath ?? ''),
@@ -71,7 +73,7 @@ public function run(): Task {
 
 			foreach ($collectors as $collector) {
 				try {
-					$collector->collect($client, $store, $context);
+					$collector->collect($metricsClient, $store, $context);
 				} catch (ManticoreSearchClientError | ManticoreSearchResponseError $e) {
 					Buddy::warning('Metrics: collector failed (' . $collector::class . '): ' . $e->getMessage());
 

test/Plugin/Metrics/MetricsContentTypeTest.php

@@ -9,13 +9,16 @@
  program; if you did not, you can find it at http://www.gnu.org/
  */
 
+use Manticoresearch\Buddy\Base\Plugin\Metrics\Handler;
 use Manticoresearch\Buddy\Base\Plugin\Metrics\Payload;
 use Manticoresearch\Buddy\Core\ManticoreSearch\Endpoint;
 use Manticoresearch\Buddy\Core\ManticoreSearch\RequestFormat;
 use Manticoresearch\Buddy\Core\Network\Request;
 use Manticoresearch\Buddy\Core\Task\TaskResult;
 use Manticoresearch\Buddy\Core\Tool\Buddy;
+use Manticoresearch\BuddyTest\Plugin\Metrics\RecordingMetricsClient;
 use PHPUnit\Framework\TestCase;
+use Swoole\Event;
 
 /**
  * Unit test for Metrics plugin content type verification
@@ -112,6 +115,37 @@ public function testPayloadFromRequest(): void {
 	 *
 	 * @return void
 	 */
+	public function testMetricsHandlerUsesCloseOnResponseClientForInternalScrapeQueries(): void {
+		RecordingMetricsClient::reset();
+
+		$payload = new Payload();
+		$handler = new Handler($payload);
+		$client = new RecordingMetricsClient('original');
+		$handler->setManticoreClient($client);
+
+		$failure = null;
+		go(
+			static function () use ($handler, &$failure): void {
+				try {
+					$task = $handler->run();
+					$task->wait(true);
+				} catch (Throwable $e) {
+					$failure = $e;
+				}
+			}
+		);
+		Event::wait();
+
+		if ($failure instanceof Throwable) {
+			throw $failure;
+		}
+
+		$this->assertSame(0, $client->sendRequestCount, 'The shared async client must not handle metrics scrape SQL');
+		$this->assertInstanceOf(RecordingMetricsClient::class, RecordingMetricsClient::$lastClone);
+		$this->assertTrue(RecordingMetricsClient::$lastClone->forceSyncWasEnabled);
+		$this->assertGreaterThan(0, RecordingMetricsClient::$lastClone->sendRequestCount);
+	}
+
 	public function testMetricsEndpointEnumValue(): void {
 		$this->assertEquals('metrics', Endpoint::Metrics->value);
 	}

test/src/Plugin/Metrics/RecordingMetricsClient.php

@@ -0,0 +1,92 @@
+<?php declare(strict_types=1);
+
+/*
+ Copyright (c) 2024, Manticore Software LTD (https://manticoresearch.com)
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License version 3 or any later
+ version. You should have received a copy of the GPL license along with this
+ program; if you did not, you can find it at http://www.gnu.org/
+ */
+
+namespace Manticoresearch\BuddyTest\Plugin\Metrics;
+
+use Manticoresearch\Buddy\Core\ManticoreSearch\Client;
+use Manticoresearch\Buddy\Core\ManticoreSearch\Response;
+use Manticoresearch\Buddy\Core\ManticoreSearch\Settings;
+
+final class RecordingMetricsClient extends Client {
+
+	public static ?self $lastClone = null;
+	public int $sendRequestCount = 0;
+	public bool $forceSyncWasEnabled = false;
+
+	/** @var array<int, string> */
+	public array $queries = [];
+
+	public function __construct(public string $name) {
+	}
+
+	public static function reset(): void {
+		self::$lastClone = null;
+	}
+
+	public function __clone() {
+		$this->name = 'clone';
+		$this->sendRequestCount = 0;
+		$this->queries = [];
+		self::$lastClone = $this;
+	}
+
+	public function setForceSync(bool $value = true): static {
+		$this->forceSyncWasEnabled = $value;
+		return parent::setForceSync($value);
+	}
+
+	public function getSettings(): Settings {
+		$settings = new Settings();
+		$settings->searchdDataDir = '';
+		$settings->searchdBinlogPath = '';
+		$settings->searchdLog = '';
+		return $settings;
+	}
+
+	public function sendRequest(
+		string $request,
+		?string $path = null,
+		bool $disableAgentHeader = false,
+		string $requestMethod = 'POST',
+	): Response {
+		unset($path, $disableAgentHeader, $requestMethod);
+
+		++$this->sendRequestCount;
+		$this->queries[] = $request;
+
+		return Response::fromBody(json_encode([$this->responseFor($request)], JSON_THROW_ON_ERROR));
+	}
+
+	/**
+	 * @return array{data: array<int, array<string, mixed>>, error: string, warning: string, total: int}
+	 */
+	private function responseFor(string $request): array {
+		$data = match ($request) {
+			'SHOW THREADS' => [
+				['Name' => 'work_0', 'This/prev job time' => '0us', 'Info' => ''],
+			],
+			'SHOW STATUS' => [
+				['Counter' => 'uptime', 'Value' => '1'],
+				['Counter' => 'connections', 'Value' => '1'],
+				['Counter' => 'version', 'Value' => 'Manticore 0.0.0 test'],
+			],
+			'SHOW TABLES' => [],
+			default => [],
+		};
+
+		return [
+			'data' => $data,
+			'error' => '',
+			'warning' => '',
+			'total' => sizeof($data),
+		];
+	}
+}