Skip to content

THRIFT-6009: Add PSR-3 logger support and runtime deprecation warnings for PHP transports#3490

Open
sveneld wants to merge 1 commit into
apache:masterfrom
sveneld:THRIFT-6009
Open

THRIFT-6009: Add PSR-3 logger support and runtime deprecation warnings for PHP transports#3490
sveneld wants to merge 1 commit into
apache:masterfrom
sveneld:THRIFT-6009

Conversation

@sveneld
Copy link
Copy Markdown
Contributor

@sveneld sveneld commented May 15, 2026

Resolves THRIFT-6009.

The PHP socket transports (TSocket, TSSLSocket, TSocketPool) previously accepted only callable|string|null for $debugHandler, defaulting to 'error_log', invoked via call_user_func(). There was no way to wire in a PSR-3 logger directly and the only @deprecated API in the PHP library (TSSLServerSocket::getSSLHost()) emitted nothing at runtime.

What this PR does

  • Adds psr/log: ^1.0 || ^2.0 || ^3.0 to composer.json. Thrift is a consumer of LoggerInterface, so the multi-major constraint is safe.
  • Widens $debugHandler on the three transports to Psr\Log\LoggerInterface|callable|string|null.
  • Routes diagnostics through a single protected log(string $level, string $message) helper on TSocket:
    • LoggerInterface — always invoked; the logger is responsible for level filtering. Per-call-site LogLevel constants are used: DEBUG for the retry-interval-elapsed message, WARNING for marking a host down, ERROR for connection failures and all-hosts-down.
    • legacy callable / string — gated by setDebug() for BC; constructor emits E_USER_DEPRECATED.
    • no handler + setDebug(true) — falls back to PHP error_log(), preserving master's stderr output.
  • setDebug() is kept (still gates the legacy callable path) but marked @deprecated and emits E_USER_DEPRECATED. It has no effect on the PSR-3 path.
  • TSSLServerSocket::getSSLHost() now emits E_USER_DEPRECATED at runtime.
  • test/php/TestClient.php demonstrates the new API by injecting a tiny StderrLogger extends AbstractLogger.

Test coverage

  • LoggerInterface dispatch with per-call-site levels (TSocket, TSSLSocket, TSocketPool).
  • Constructor deprecation triggers for string- and Closure-form $debugHandler.
  • setDebug() deprecation regression guard.
  • TSSLServerSocket::getSSLHost() runtime deprecation.
  • Null / LoggerInterface paths assert no deprecation (regression guards).
  • New Test\Thrift\Unit\Lib\UserDeprecationCapture trait shared between tests.

646 unit tests pass; PHPStan (level 6) clean; phpcs clean; cross-test (java, go,rs,cpp,py,php,nodejs,nodets) passes.

  • Did you create an Apache Jira ticket? — THRIFT-6009
  • Pull request title follows THRIFT-NNNN: …?
  • Single squashed commit?
  • Avoided breaking changes? — runtime behaviour preserved for the common cases (no handler + no setDebug; legacy callable + setDebug); legacy paths are runtime-deprecated rather than removed.
  • [skip ci] — N/A.

@mergeable mergeable Bot added the php label May 15, 2026
@sveneld sveneld marked this pull request as draft May 16, 2026 08:50
Client: php

* Require psr/log ^1 || ^2 || ^3. TSocket/TSSLSocket/TSocketPool
  constructors accept a Psr\Log\LoggerInterface as $debugHandler.
  The PSR-3 path is always invoked and the logger filters by level
  (per-call-site DEBUG / WARNING / ERROR in TSocketPool::open()).
* Default with no handler is silent (NullLogger). When the caller
  enables setDebug(true) without supplying a handler, log() falls
  back to PHP error_log() — preserves master's stderr output.
* Legacy callable / string $debugHandler is preserved for BC;
  constructor emits E_USER_DEPRECATED when used. setDebug() still
  gates that legacy path and is itself @deprecated.
* TSSLServerSocket::getSSLHost() now emits E_USER_DEPRECATED.
* TestClient.php demonstrates the new API via a small PSR-3
  StderrLogger.
* New Test\Thrift\Unit\Lib\UserDeprecationCapture trait factors
  shared E_USER_DEPRECATED capture out of the unit tests.

Generated-by: Claude Opus 4.7
@sveneld sveneld marked this pull request as ready for review May 16, 2026 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant