Skip to content

Auto-inject Logger into LoggerAwareInterface services#23203

Open
diedexx wants to merge 2 commits intotrunkfrom
logger-injection
Open

Auto-inject Logger into LoggerAwareInterface services#23203
diedexx wants to merge 2 commits intotrunkfrom
logger-injection

Conversation

@diedexx
Copy link
Copy Markdown
Member

@diedexx diedexx commented Apr 28, 2026

Context

We want to encourage richer logging across the codebase, but threading the Logger service through every constructor (and updating every test double) is a lot of friction for what is usually an opt-in concern. PSR-3 already standardizes this with Psr\Log\LoggerAwareInterface + LoggerAwareTrait, so this PR teaches our DI container to wire it up automatically.

By default the injected logger is our own Yoast\WP\SEO\Loggers\Logger, which wraps a NullLogger unless a consumer overrides it through the wpseo_logger filter — so opting in costs nothing at runtime until someone actually plugs in a real logger.

We don't use this interface anywhere yet, with one exception: the open PR for the MyYoast OAuth client uses it. Since logger-awareness is part of PSR-3 and is the de facto standard, this seems like a good convention to adopt going forward.

Summary

This PR can be summarized in the following changelog entry:

  • Adds a Logger_Aware_Pass compiler pass so any service implementing Psr\Log\LoggerAwareInterface automatically receives the Logger via setLogger(), with no constructor wiring required.

Relevant technical choices

  • Implemented as a Symfony DI compiler pass (config/dependency-injection/logger-aware-pass.php) in line with the existing Loader_Pass, Interface_Injection_Pass and Inject_From_Registry_Pass. Registered in Container_Compiler::compile() after Inject_From_Registry_Pass.
  • The pass iterates $container->getDefinitions(), skips abstract definitions, and calls addMethodCall( 'setLogger', [ new Reference( Logger::class ) ] ) on every definition whose class implements LoggerAwareInterface.
  • If a definition already has an explicit setLogger method call configured elsewhere, the pass leaves it alone (continue 2). This keeps manual overrides in services.php authoritative.
  • LoggerAwareInterface is referenced via the prefixed YoastSEO_Vendor\Psr\Log\LoggerAwareInterface because PSR-Log is scoped at build time, while the Symfony DI imports stay un-prefixed (the dump rewrite in Container_Compiler swaps those at dump time).
  • Consumers opt in by implements LoggerAwareInterface + use LoggerAwareTrait; — that's the entire change required at the call site.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR has no user-facing impact on its own — it only adds plumbing that takes effect when a service opts in. To acceptance-test that the plumbing is wired up correctly:

  1. Make sure you're on a fresh checkout of this branch and run composer install.
  2. Run composer compile-di to rebuild src/generated/container.php.
  3. Activate Yoast SEO on a clean WordPress site and visit a few pages (front-end, post editor, SEO settings).
  4. Verify that the site behaves identically to trunk — no errors, no warnings, no regressions in the editor or settings screens.
  5. Run composer test (PHP unit tests) and confirm everything still passes.
  6. Dev only: test that loggers are injected
    1. Make any DI-powered service implement the LoggerAwareInterface (use LoggerAwareTrait for quick implementation).
    2. Run composer compile-di.
    3. Verify that setLogger is called in src/generated/container.php with the instance of our existing Logger class.
    4. Verify that the site behaves identically to trunk — no errors, no warnings, no regressions in the editor or settings screens.

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

This is an infrastructure-only change with no UI surface, so the matrix above isn't really applicable. A smoke test on the latest WordPress + a current PHP (7.4 / 8.x) is sufficient.

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

QA can test this PR by activating Yoast SEO on a clean WordPress install and confirming there is no functional change versus the previous release.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

  • The compiled DI container (src/generated/container.php). Every service in the container is examined by the new pass at compile time. At runtime, only services implementing LoggerAwareInterface are affected — currently none in this repository — so the runtime blast radius is effectively zero.

Other environments

  • This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.
  • This PR also affects Yoast SEO for Google Docs. I have added a changelog entry starting with [yoast-doc-extension], added test instructions for Yoast SEO for Google Docs and attached the Google Docs Add-on label to this PR.

Documentation

  • I have written documentation for this change. The new compiler pass carries a class-level docblock explaining its purpose and how to opt in.

Quality assurance

  • I have tested this code to the best of my abilities.
  • During testing, I had activated all plugins that Yoast SEO provides integrations for.
  • I have added unit tests to verify the code works as intended.
  • If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
  • I have written this PR in accordance with my team's definition of done.
  • I have checked that the base branch is correctly set.
  • I have run grunt build:images and commited the results, if my PR introduces new images or SVGs.

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label.
  • I have added my hours to the WBSO document.

Adds a Logger_Aware_Pass compiler pass that automatically calls
setLogger on every registered service whose class implements
Psr\Log\LoggerAwareInterface. Developers can now opt into logging by
implementing the interface and using LoggerAwareTrait, instead of
threading the Logger through the constructor.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@diedexx diedexx added the changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog label Apr 28, 2026
@diedexx diedexx requested a review from Copilot April 28, 2026 09:22
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 28, 2026

Coverage Report for CI Build 24

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage decreased (-0.6%) to 52.831%

Details

  • Coverage decreased (-0.6%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 64136
Covered Lines: 33818
Line Coverage: 52.73%
Relevant Branches: 16093
Covered Branches: 8568
Branch Coverage: 53.24%
Branches in Coverage %: Yes
Coverage Strength: 46901.45 hits per line

💛 - Coveralls

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds DI-container plumbing to automatically inject the plugin’s Yoast\WP\SEO\Loggers\Logger into any service that opts into PSR-3 logger awareness via LoggerAwareInterface, avoiding constructor wiring.

Changes:

  • Introduces a new Symfony DI compiler pass (Logger_Aware_Pass) that adds a setLogger() method call to eligible service definitions.
  • Skips injection when a service definition already has an explicit setLogger method call configured.
  • Registers the new compiler pass in Container_Compiler::compile() after Inject_From_Registry_Pass.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
config/dependency-injection/logger-aware-pass.php Adds the new compiler pass that detects LoggerAwareInterface implementations and injects the Logger via setLogger().
config/dependency-injection/container-compiler.php Registers Logger_Aware_Pass in the container compilation pipeline.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread config/dependency-injection/logger-aware-pass.php Outdated
Comment thread config/dependency-injection/logger-aware-pass.php Outdated
- Update docblock to reference the prefixed YoastSEO_Vendor namespace,
  matching what the pass actually checks.
- Normalize the manual setLogger method-call check with strtolower so
  case-variant configurations (e.g. setlogger) still suppress
  auto-injection, since PHP method names are case-insensitive.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants