Skip to content

Conversation

@f3l1x
Copy link
Member

@f3l1x f3l1x commented Nov 25, 2025

Add Tracy panel integration to visualize Elasticsearch requests during development:

  • QueryLogger: stores query information (method, endpoint, body, response, duration)
  • LoggingHttpClient: PSR-18 client wrapper that intercepts and logs HTTP requests
  • ElasticsearchPanel: Tracy bar panel showing query count, time, and details
  • ClientFactory: creates Elasticsearch client with optional logging

Features:

  • Display number of queries and total execution time in Tracy bar
  • Show HTTP method, endpoint, status code, and duration for each query
  • Expandable request body and response data
  • Backtrace showing where each query originated
  • Configuration display with sensitive data masked

Enable with debug: true in config. Requires tracy/tracy package.

Summary by CodeRabbit

  • New Features

    • Added Tracy debugger integration for Elasticsearch debugging in development mode
    • Query debugging panel displays query count, execution time, detailed metrics, and request/response information
  • Documentation

    • Added Tracy panel configuration guide to README
  • Chores

    • Added Tracy debugger as a development dependency

✏️ Tip: You can customize this high-level summary in your review settings.

Add Tracy panel integration to visualize Elasticsearch requests during development:

- QueryLogger: stores query information (method, endpoint, body, response, duration)
- LoggingHttpClient: PSR-18 client wrapper that intercepts and logs HTTP requests
- ElasticsearchPanel: Tracy bar panel showing query count, time, and details
- ClientFactory: creates Elasticsearch client with optional logging

Features:
- Display number of queries and total execution time in Tracy bar
- Show HTTP method, endpoint, status code, and duration for each query
- Expandable request body and response data
- Backtrace showing where each query originated
- Configuration display with sensitive data masked

Enable with `debug: true` in config. Requires tracy/tracy package.
@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

Walkthrough

Adds Tracy debugger integration to the Elasticsearch extension. Introduces factory and logging classes, a Tracy panel for query metrics, templates for visualization, and configuration support to enable debug-mode query logging via the Tracy Bar.

Changes

Cohort / File(s) Summary
Documentation & Dependencies
docs/README.md, composer.json
Added Tracy Panel documentation section to README with configuration example and installation note. Added tracy/tracy ^2.10 to require-dev and suggest dependencies.
DI Extension
src/DI/ElasticsearchExtension.php
Enhanced extension to conditionally register QueryLogger and ElasticsearchPanel when debug mode is enabled and Tracy Bar exists. Routes client creation through ClientFactory with logging support; falls back to ClientBuilder when debug is disabled. Added debug boolean configuration schema.
Tracy Integration Core
src/Tracy/ClientFactory.php, src/Tracy/LoggingHttpClient.php, src/Tracy/QueryLogger.php, src/Tracy/ElasticsearchPanel.php
Added ClientFactory for creating Elasticsearch clients with optional logging. Added LoggingHttpClient (PSR-18 wrapper) that captures request/response details and backtrace. Added QueryLogger to collect and aggregate query metrics. Added ElasticsearchPanel implementing Tracy\IBarPanel to render query statistics and sanitize sensitive configuration.
Panel Templates
src/Tracy/templates/tab.phtml, src/Tracy/templates/panel.phtml
Added tab template displaying query count and total time with conditional SVG indicators. Added panel template rendering detailed query table with method/endpoint, request/response bodies (collapsible), backtrace, status codes, and timing information.

Sequence Diagram(s)

sequenceDiagram
    participant Ext as ElasticsearchExtension
    participant QLogger as QueryLogger
    participant LClient as LoggingHttpClient
    participant Inner as Elastic\Client
    participant Tracy as Tracy Panel
    
    Ext->>Ext: Debug enabled?
    alt Debug On
        Ext->>QLogger: Create instance
        Ext->>LClient: Create with QueryLogger
        Ext->>Ext: Register ElasticsearchPanel with Tracy
        Ext->>Inner: Create via ClientFactory
    else Debug Off
        Ext->>Inner: Create via ClientBuilder
    end
    
    LClient->>LClient: Capture request details
    LClient->>Inner: sendRequest()
    Inner-->>LClient: Response
    LClient->>QLogger: logQuery(method, endpoint, body, response, ...)
    QLogger->>QLogger: Accumulate totalTime
    
    Tracy->>QLogger: getQueries(), getQueryCount()
    Tracy->>Tracy: Render metrics & query table
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • src/Tracy/LoggingHttpClient.php: Verify backtrace filtering logic correctly excludes internal frames and handles edge cases
  • src/Tracy/ElasticsearchPanel.php: Confirm maskSensitiveConfig properly handles nested array structures for both scalar and complex authentication values
  • src/DI/ElasticsearchExtension.php: Review conditional logic for debug mode and Tracy availability; ensure fallback path maintains backward compatibility
  • src/Tracy/templates/panel.phtml: Validate JSON pretty-printing safety and escaping for malicious response bodies

Poem

🐰 A panel springs to life so bright,
Queries logged in Tracy's light,
With HTTP steps and timings true,
Debug mode hops right into view!
From Elasticsearch queries we see through,
Making tracing quite the thing to do! 🎯

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Implement Tracy debugging panel' accurately describes the main change in the pull request, which adds comprehensive Tracy debugger integration for Elasticsearch query visualization.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch claude/implement-tracy-panel-014rtsyKytF5jfcY1N6ZQkYB

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
src/Tracy/ElasticsearchPanel.php (1)

35-45: PHPMD “unused local variable” warnings are expected here due to template usage

The locals $queriesNum, $totalTime, $queries, and $config are intentionally set for tab.phtml / panel.phtml and therefore appear unused to PHPMD. Functionally this is fine, but if PHPMD is part of CI you may want to suppress this rule for these methods to keep the signal clean, e.g.:

/**
 * Renders HTML code for custom tab.
 *
 * @SuppressWarnings(PHPMD.UnusedLocalVariable)
 */
public function getTab(): string
{
    // ...
}

(and similarly for getPanel()).

Also applies to: 52-66

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7dc8d8a and 9cfb95f.

📒 Files selected for processing (9)
  • .docs/README.md (2 hunks)
  • composer.json (1 hunks)
  • src/DI/ElasticsearchExtension.php (2 hunks)
  • src/Tracy/ClientFactory.php (1 hunks)
  • src/Tracy/ElasticsearchPanel.php (1 hunks)
  • src/Tracy/LoggingHttpClient.php (1 hunks)
  • src/Tracy/QueryLogger.php (1 hunks)
  • src/Tracy/templates/panel.phtml (1 hunks)
  • src/Tracy/templates/tab.phtml (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/Tracy/ClientFactory.php (2)
src/Tracy/QueryLogger.php (1)
  • QueryLogger (8-69)
src/Tracy/LoggingHttpClient.php (1)
  • LoggingHttpClient (12-124)
src/Tracy/LoggingHttpClient.php (2)
src/Tracy/ElasticsearchPanel.php (1)
  • __construct (16-20)
src/Tracy/QueryLogger.php (2)
  • QueryLogger (8-69)
  • logQuery (20-43)
src/Tracy/ElasticsearchPanel.php (2)
src/Tracy/LoggingHttpClient.php (1)
  • __construct (15-20)
src/Tracy/QueryLogger.php (4)
  • QueryLogger (8-69)
  • getQueryCount (53-56)
  • getTotalTime (58-61)
  • getQueries (48-51)
src/DI/ElasticsearchExtension.php (3)
src/Tracy/ClientFactory.php (1)
  • ClientFactory (12-62)
src/Tracy/ElasticsearchPanel.php (1)
  • ElasticsearchPanel (10-89)
src/Tracy/QueryLogger.php (1)
  • QueryLogger (8-69)
🪛 markdownlint-cli2 (0.18.1)
.docs/README.md

43-43: Hard tabs
Column: 1

(MD010, no-hard-tabs)


44-44: Hard tabs
Column: 1

(MD010, no-hard-tabs)

🪛 PHPMD (2.15.0)
src/Tracy/ElasticsearchPanel.php

37-37: Avoid unused local variables such as '$queriesNum'. (undefined)

(UnusedLocalVariable)


39-39: Avoid unused local variables such as '$totalTime'. (undefined)

(UnusedLocalVariable)


54-54: Avoid unused local variables such as '$queriesNum'. (undefined)

(UnusedLocalVariable)


56-56: Avoid unused local variables such as '$totalTime'. (undefined)

(UnusedLocalVariable)


58-58: Avoid unused local variables such as '$queries'. (undefined)

(UnusedLocalVariable)


60-60: Avoid unused local variables such as '$config'. (undefined)

(UnusedLocalVariable)

🔇 Additional comments (9)
composer.json (1)

29-34: LGTM!

The Tracy dependency is correctly added to require-dev with an appropriate version constraint, and the suggest entry clearly communicates its purpose for debug mode integration.

src/Tracy/QueryLogger.php (1)

8-69: LGTM!

The QueryLogger implementation is clean and straightforward. The comprehensive PHPDoc type annotations ensure type safety, and the logic for accumulating queries and total time is correct.

src/Tracy/templates/tab.phtml (1)

1-29: LGTM!

The tab template correctly displays the Elasticsearch icon and metrics. The conditional rendering based on query count is appropriate, and the formatting is clear and consistent with Tracy's UI patterns.

src/Tracy/ClientFactory.php (1)

12-62: LGTM!

The ClientFactory correctly builds an Elasticsearch client with optional logging support. The configuration handling is clean, and the conditional wrapping of the HTTP client with LoggingHttpClient when a logger is provided follows a sound design pattern.

src/Tracy/templates/panel.phtml (1)

1-136: LGTM!

The panel template is well-structured and comprehensive. It properly escapes all output using Helpers::escapeHtml(), handles edge cases with appropriate fallbacks for json_encode(), and provides a clear, organized view of Elasticsearch queries with collapsible sections for detailed data. The styling is consistent with Tracy's design patterns.

src/DI/ElasticsearchExtension.php (3)

41-41: LGTM!

The defensive check class_exists(Bar::class) ensures Tracy integration is only enabled when Tracy is actually available, preventing errors if the optional dependency is not installed.


66-73: I'll help you verify the HTTP client discovery compatibility concern in the Elasticsearch extension. Let me start by examining the full context of the file and understanding both code paths.
<function_calls>

shell

First, let's read the full file to understand the context around lines 66-80

wc -l src/DI/ElasticsearchExtension.php

Then read the relevant section

sed -n '60,85p' src/DI/ElasticsearchExtension.php


</function_calls>


76-80: Based on my verification of the code and official Elasticsearch PHP documentation:

ClientBuilder::fromConfig() supports all required configuration options in version 8.11.0.

The investigation confirms:

  1. ClientFactory::create() (debug mode) manually handles: hosts, retries, sslVerification, apiKey, basicAuthentication
  2. ClientBuilder::fromConfig() (non-debug mode) accepts the same configuration array keys
  3. Both paths receive the identical filtered $clientConfig (lines 44-45)
  4. Elasticsearch PHP 8.11.0 documentation confirms fromConfig() maps array keys to setter methods for all these options

The original concern about configuration parity between debug and non-debug modes is unfounded—both paths handle the full configuration identically.

src/Tracy/LoggingHttpClient.php (1)

109-114: The review comment is incorrect. The 'function' key is always present in debug_backtrace entries.

PHP's debug_backtrace() function guarantees that the 'function' key is present in every frame. While 'file' and 'line' keys may be absent for internal C-level calls, when 'file' is present (as verified by the isset($trace['file']) check at line 108), the call originates from user-land code, and the 'function' key is guaranteed to exist. The code at line 113 is safe and requires no defensive null coalescing operator.

Comment on lines +43 to +44
hosts: [localhost]
debug: true
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Replace hard tabs with spaces.

Markdownlint is flagging the use of hard tabs in the NEON configuration example.

Apply this diff to fix the indentation:

 elasticsearch:
-	hosts: [localhost]
-	debug: true
+  hosts: [localhost]
+  debug: true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
hosts: [localhost]
debug: true
elasticsearch:
hosts: [localhost]
debug: true
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

43-43: Hard tabs
Column: 1

(MD010, no-hard-tabs)


44-44: Hard tabs
Column: 1

(MD010, no-hard-tabs)

🤖 Prompt for AI Agents
In .docs/README.md around lines 43 to 44, the NEON configuration example uses
hard tabs for indentation which Markdownlint flags; replace any tab characters
with spaces (use consistent spacing — e.g., two spaces per indent) for both
lines ("hosts: [localhost]" and "debug: true") so the snippet contains only
spaces and conforms to Markdownlint rules.

Comment on lines +74 to +87
private function maskSensitiveConfig(array $config): array
{
$sensitiveKeys = ['apiKey', 'basicAuthentication'];

foreach ($sensitiveKeys as $key) {
if (isset($config[$key])) {
$config[$key] = is_array($config[$key])
? array_map(fn () => '***', $config[$key])
: '***';
}
}

return $config;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Script executed:

#!/bin/bash

# First, verify the file exists and examine the exact code
cat -n src/Tracy/ElasticsearchPanel.php | head -100

Repository: contributte/elasticsearch


<shell_result>
Repository: contributte/elasticsearch
Exit code: 0

stdout:

     1	<?php declare(strict_types = 1);
     2	
     3	namespace Contributte\Elasticsearch\Tracy;
     4	
     5	use Tracy\IBarPanel;
     6	
     7	/**
     8	 * Tracy debugger bar panel for Elasticsearch queries.
     9	 */
    10	class ElasticsearchPanel implements IBarPanel
    11	{
    12	
    13		/** @var array<string, mixed> */
    14		private array $config = [];
    15	
    16		public function __construct(
    17			private QueryLogger $logger,
    18		)
    19		{
    20		}
    21	
    22		/**
    23		 * @param array<string, mixed> $config
    24		 */
    25		public function setConfig(array $config): void
    26		{
    27			$this->config = $config;
    28		}
    29	
    30		/**
    31		 * Renders HTML code for custom tab.
    32		 */
    33		public function getTab(): string
    34		{
    35			// Variables used in template
    36			// phpcs:ignore SlevomatCodingStandard.Variables.UnusedVariable.UnusedVariable
    37			$queriesNum = $this->logger->getQueryCount();
    38			// phpcs:ignore SlevomatCodingStandard.Variables.UnusedVariable.UnusedVariable
    39			$totalTime = $this->logger->getTotalTime();
    40	
    41			ob_start();
    42			require __DIR__ . '/templates/tab.phtml';
    43	
    44			return (string) ob_get_clean();
    45		}
    46	
    47		/**
    48		 * Renders HTML code for custom panel.
    49		 */
    50		public function getPanel(): string
    51		{
    52			// Variables used in template
    53			// phpcs:ignore SlevomatCodingStandard.Variables.UnusedVariable.UnusedVariable
    54			$queriesNum = $this->logger->getQueryCount();
    55			// phpcs:ignore SlevomatCodingStandard.Variables.UnusedVariable.UnusedVariable
    56			$totalTime = $this->logger->getTotalTime();
    57			// phpcs:ignore SlevomatCodingStandard.Variables.UnusedVariable.UnusedVariable
    58			$queries = $this->logger->getQueries();
    59			// phpcs:ignore SlevomatCodingStandard.Variables.UnusedVariable.UnusedVariable
    60			$config = $this->maskSensitiveConfig($this->config);
    61	
    62			ob_start();
    63			require __DIR__ . '/templates/panel.phtml';
    64	
    65			return (string) ob_get_clean();
    66		}
    67	
    68		/**
    69		 * Mask sensitive configuration values.
    70		 *
    71		 * @param array<string, mixed> $config
    72		 * @return array<string, mixed>
    73		 */
    74		private function maskSensitiveConfig(array $config): array
    75		{
    76			$sensitiveKeys = ['apiKey', 'basicAuthentication'];
    77	
    78			foreach ($sensitiveKeys as $key) {
    79				if (isset($config[$key])) {
    80					$config[$key] = is_array($config[$key])
    81						? array_map(fn () => '***', $config[$key])
    82						: '***';
    83				}
    84			}
    85	
    86			return $config;
    87		}
    88	
    89	}

</shell_result>


Script executed:

#!/bin/bash

# Check composer.json for PHP version constraints
cat composer.json | head -50

Repository: contributte/elasticsearch


<shell_result>
Repository: contributte/elasticsearch
Exit code: 0

stdout:

{
  "name": "contributte/elasticsearch",
  "description": "Tiny Elasticsearch integration into Nette Framework",
  "keywords": [
    "contributte",
    "nette",
    "elasticsearch",
    "search",
    "elastic",
    "es"
  ],
  "type": "library",
  "license": "MIT",
  "homepage": "https://github.com/contributte/elasticsearch",
  "authors": [
    {
      "name": "Milan Felix Šulc",
      "homepage": "https://f3l1x.io"
    }
  ],
  "require": {
    "php": ">=8.1",
    "elasticsearch/elasticsearch": "^8.11.0",
    "nette/di": "^3.1.8"
  },
  "require-dev": {
    "contributte/qa": "^0.4",
    "contributte/tester": "^0.4",
    "contributte/phpstan": "^0.1",
    "tracy/tracy": "^2.10"
  },
  "suggest": {
    "tracy/tracy": "For Tracy debugger panel integration (debug mode)"
  },
  "conflict": {
    "psr/http-message": "<2.0"
  },
  "autoload": {
    "psr-4": {
      "Contributte\\Elasticsearch\\": "src/"
    }
  },
  "autoload-dev": {
    "psr-4": {
      "Tests\\Cases\\": "tests/cases"
    }
  },
  "minimum-stability": "dev",
  "prefer-stable": true,
  "config": {

</shell_result>


Script executed:

#!/bin/bash

# Search for tests of maskSensitiveConfig
find . -name "*.php" -type f | xargs rg -l "maskSensitiveConfig|basicAuthentication" | grep -v vendor

# Also check if there are any tests at all
find tests -name "*.php" -type f 2>/dev/null | head -20

Repository: contributte/elasticsearch


<shell_result>
Repository: contributte/elasticsearch
Exit code: 0

stdout:

./src/Tracy/ElasticsearchPanel.php
./src/Tracy/ClientFactory.php
./src/DI/ElasticsearchExtension.php
tests/bootstrap.php

</shell_result>


Script executed:

#!/bin/bash

# Check ClientFactory.php for basicAuthentication usage
echo "=== ClientFactory.php ==="
cat -n src/Tracy/ClientFactory.php

echo -e "\n=== ElasticsearchExtension.php ==="
cat -n src/DI/ElasticsearchExtension.php | head -150

Repository: contributte/elasticsearch


<shell_result>
Repository: contributte/elasticsearch
Exit code: 0

stdout:

=== ClientFactory.php ===
     1	<?php declare(strict_types = 1);
     2	
     3	namespace Contributte\Elasticsearch\Tracy;
     4	
     5	use Elastic\Elasticsearch\Client;
     6	use Elastic\Elasticsearch\ClientBuilder;
     7	use Psr\Http\Client\ClientInterface;
     8	
     9	/**
    10	 * Factory for creating Elasticsearch client with optional query logging.
    11	 */
    12	class ClientFactory
    13	{
    14	
    15		/**
    16		 * @param array{hosts?: array<string>, retries?: int, sslVerification?: bool, apiKey?: array<string>, basicAuthentication?: array<string>} $config
    17		 */
    18		public static function create(
    19			array $config,
    20			?ClientInterface $httpClient = null,
    21			?QueryLogger $logger = null,
    22		): Client
    23		{
    24			$builder = ClientBuilder::create();
    25	
    26			// Set hosts
    27			if (isset($config['hosts'])) {
    28				$builder->setHosts($config['hosts']);
    29			}
    30	
    31			// Set retries
    32			if (isset($config['retries'])) {
    33				$builder->setRetries($config['retries']);
    34			}
    35	
    36			// Set SSL verification
    37			if (isset($config['sslVerification'])) {
    38				$builder->setSSLVerification($config['sslVerification']);
    39			}
    40	
    41			// Set API key
    42			if (isset($config['apiKey'])) {
    43				$builder->setApiKey(...$config['apiKey']);
    44			}
    45	
    46			// Set basic authentication
    47			if (isset($config['basicAuthentication'])) {
    48				$builder->setBasicAuthentication(...$config['basicAuthentication']);
    49			}
    50	
    51			// Set custom HTTP client with logging wrapper if provided
    52			if ($httpClient !== null && $logger !== null) {
    53				$loggingClient = new LoggingHttpClient($httpClient, $logger);
    54				$builder->setHttpClient($loggingClient);
    55			} elseif ($httpClient !== null) {
    56				$builder->setHttpClient($httpClient);
    57			}
    58	
    59			return $builder->build();
    60		}
    61	
    62	}

=== ElasticsearchExtension.php ===
     1	<?php declare(strict_types = 1);
     2	
     3	namespace Contributte\Elasticsearch\DI;
     4	
     5	use Contributte\Elasticsearch\Tracy\ClientFactory;
     6	use Contributte\Elasticsearch\Tracy\ElasticsearchPanel;
     7	use Contributte\Elasticsearch\Tracy\QueryLogger;
     8	use Elastic\Elasticsearch\Client;
     9	use Elastic\Elasticsearch\ClientBuilder;
    10	use Nette\DI\CompilerExtension;
    11	use Nette\DI\Definitions\ServiceDefinition;
    12	use Nette\DI\Definitions\Statement;
    13	use Nette\Schema\Expect;
    14	use Nette\Schema\Schema;
    15	use stdClass;
    16	use Tracy\Bar;
    17	
    18	/**
    19	 * @method stdClass getConfig()
    20	 */
    21	class ElasticsearchExtension extends CompilerExtension
    22	{
    23	
    24		public function getConfigSchema(): Schema
    25		{
    26			return Expect::structure([
    27				'hosts' => Expect::arrayOf(Expect::string())->required()->min(1),
    28				'retries' => Expect::int(1),
    29				'sslVerification' => Expect::bool(true),
    30				'apiKey' => Expect::anyOf(Expect::arrayOf(Expect::string())->min(1)->max(2), null),
    31				'basicAuthentication' => Expect::anyOf(Expect::arrayOf(Expect::string())->min(2)->max(2), null),
    32				'debug' => Expect::bool(false),
    33			]);
    34		}
    35	
    36		public function beforeCompile(): void
    37		{
    38			$config = $this->getConfig();
    39			$builder = $this->getContainerBuilder();
    40	
    41			$debug = $config->debug && class_exists(Bar::class);
    42	
    43			// Filter out null values and remove debug from config passed to client
    44			$clientConfig = array_filter((array) $config, fn ($value) => $value !== null);
    45			unset($clientConfig['debug']);
    46	
    47			if ($debug) {
    48				// Register QueryLogger
    49				$builder->addDefinition($this->prefix('queryLogger'))
    50					->setType(QueryLogger::class);
    51	
    52				// Register ElasticsearchPanel
    53				$panel = $builder->addDefinition($this->prefix('panel'))
    54					->setType(ElasticsearchPanel::class)
    55					->setArguments([new Statement('@' . $this->prefix('queryLogger'))]);
    56	
    57				$panel->addSetup('setConfig', [$clientConfig]);
    58	
    59				// Register panel with Tracy Bar
    60				if ($builder->hasDefinition('tracy.bar')) {
    61					$tracyBar = $builder->getDefinition('tracy.bar');
    62					assert($tracyBar instanceof ServiceDefinition);
    63					$tracyBar->addSetup('addPanel', [new Statement('@' . $this->prefix('panel'))]);
    64				}
    65	
    66				// Create client with logging using ClientFactory
    67				$builder->addDefinition($this->prefix('client'))
    68					->setType(Client::class)
    69					->setFactory([ClientFactory::class, 'create'], [
    70						$clientConfig,
    71						new Statement('Http\Discovery\Psr18ClientDiscovery::find'),
    72						new Statement('@' . $this->prefix('queryLogger')),
    73					]);
    74			} else {
    75				// Create client without logging
    76				$builder->addDefinition($this->prefix('client'))
    77					->setType(Client::class)
    78					->setFactory([ClientBuilder::class, 'fromConfig'])
    79					->setArguments([$clientConfig]);
    80			}
    81		}
    82	
    83	}

</shell_result>


The review comment is accurate—this is a critical runtime error that will definitely break the Tracy panel

The code at line 81 uses array_map(fn () => '***', $config[$key]) with a zero-parameter callback. Since:

  • PHP 8.1+ is required (per composer.json)
  • basicAuthentication is always configured as array<string> (per ElasticsearchExtension.php line 31)
  • This array is passed to maskSensitiveConfig() at line 60 in getPanel()
  • array_map() will invoke the callback with each element, but the callback accepts zero parameters

This will throw ArgumentCountError at runtime whenever the Tracy panel renders with debug mode enabled and basicAuthentication is configured.

The suggested fix is correct:

$config[$key] = is_array($config[$key])
    ? array_map(static fn (mixed $_): string => '***', $config[$key])
    : '***';
🤖 Prompt for AI Agents
In src/Tracy/ElasticsearchPanel.php around lines 74 to 87, the anonymous
callback passed to array_map accepts zero parameters which causes an
ArgumentCountError at runtime when basicAuthentication is an array; update the
array_map callback to accept a single typed parameter and return a string (e.g.
static fn (mixed $_): string => '***') so it matches array_map's expected
signature and prevents the runtime error.

Comment on lines +37 to +38
$body = (string) $request->getBody();
$request->getBody()->rewind();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Handle potential non-rewindable streams.

Calling rewind() on a PSR-7 stream can fail if the stream is not seekable. While most PSR-18 implementations use seekable streams, this could throw an exception with certain stream types.

Consider checking seekability before rewinding:

 // Get request body
 $body = (string) $request->getBody();
-$request->getBody()->rewind();
+$requestBody = $request->getBody();
+if ($requestBody->isSeekable()) {
+  $requestBody->rewind();
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$body = (string) $request->getBody();
$request->getBody()->rewind();
$body = (string) $request->getBody();
$requestBody = $request->getBody();
if ($requestBody->isSeekable()) {
$requestBody->rewind();
}
🤖 Prompt for AI Agents
In src/Tracy/LoggingHttpClient.php around lines 37-38, the code unconditionally
calls rewind() on the PSR-7 stream which can fail for non-seekable streams;
change the logic to first check $stream = $request->getBody(); if
($stream->isSeekable()) { $stream->rewind(); } else { create a new seekable
stream from the stringified body (or otherwise buffer the body content) and use
that for logging so you never call rewind() on non-seekable streams; ensure you
preserve the original request body behavior when replacing or buffering.

Comment on lines +50 to +51
$responseBody = (string) $response->getBody();
$response->getBody()->rewind();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Handle potential non-rewindable streams.

Similar to the request body, calling rewind() on the response body stream can fail if the stream is not seekable.

Consider checking seekability before rewinding:

 // Get response body
 $responseBody = (string) $response->getBody();
-$response->getBody()->rewind();
+$responseBodyStream = $response->getBody();
+if ($responseBodyStream->isSeekable()) {
+  $responseBodyStream->rewind();
+}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/Tracy/LoggingHttpClient.php around lines 50-51, the code unconditionally
calls rewind() on the response body which can fail for non-seekable streams;
change the logic to check if the response body is seekable (or catch exceptions)
before calling rewind() — e.g. call $response->getBody()->isSeekable() and only
invoke rewind() when true, otherwise skip rewind (or safely handle the
exception) so non-seekable streams don’t trigger errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants