>>> [!note] Migrated issue <!-- Drupal.org comment --> <!-- Migrated from issue #3591791. --> Reported by: [mably](https://www.drupal.org/user/3375160) Related to !4 >>> <h3>Problem / motivation</h3> <p>Loading <code>/admin/config/system/audit-trail/segments</code> on a fresh container crashes with:</p> <pre>Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException: Circular reference detected for service "audit_trail.logger", path: "audit_trail.chain_archiver -&gt; audit_trail.logger -&gt; logger.factory".</pre><p>Root cause is a service-wiring design smell: <code>AuditTrailLogger</code> wears two hats. It implements <code>LoggerInterface</code> (tagged <code>logger</code>, so the container's <code>RegisterLoggersPass</code> calls <code>addLogger(@audit_trail.logger)</code> on <code>logger.factory</code>), AND it implements <code>AuditTrailInternalWriterInterface</code> (so <code>ChainArchiver</code>, <code>ChainTimestamper</code> et al. inject it as their chain-event emitter). On top of that, it injects <code>@logger.factory</code> for fallback warning emission. The three together form a cycle: <code>logger.factory</code> cannot finish initialization without <code>audit_trail.logger</code>, which cannot finish construction without <code>logger.factory</code>.</p> <p>Kernel tests do not catch this because the test harness warms <code>logger.factory</code> before any audit_trail service is requested. Production hits the cycle when a controller (the segments admin page) is the first cold thing to resolve <code>chain_archiver</code>.</p> <h3>Proposed resolution</h3> <p>A logger should be just a logger. Split into two classes with clearly separated responsibilities:</p> <ul> <li><strong><code>AuditTrailLogger</code></strong> (refactored): thin <code>LoggerInterface</code> entry point. Tagged <code>logger</code>. Inspects the PSR-3 <code>context</code>, forwards <code>chain: TRUE</code> calls to the internal writer, no-ops the rest. NO <code>logger.factory</code> dependency. Catastrophic failures fall back to <code>error_log()</code> because PSR-3 forbids throwing from <code>log()</code> and we cannot re-enter <code>logger.factory</code> without re-creating the cycle.</li> <li><strong><code>AuditTrailInternalWriter</code></strong> (new class): all the existing row-build, canonicalize, HMAC, INSERT, chain-event-mint logic moves here. Implements <code>AuditTrailInternalWriterInterface</code>. NOT tagged <code>logger</code>. Two public entry points: <code>logChained(level, message, context)</code> for the PSR-3 path (called by the logger above) and <code>chainedWrite(chain, action, resource, buckets)</code> for module internals (called by <code>ChainArchiver</code>, <code>ChainTimestamper</code>, etc.). On failure: throws a typed exception. Callers handle.</li> </ul> <p>After the split, <code>AuditTrailInternalWriter</code> takes the heavy dependency list (database, secret repository, lock, current_user, datetime.time, config.factory, request_stack, entity_type_manager, state) without <code>@logger.factory</code>. <code>AuditTrailLogger</code> takes only <code>AuditTrailInternalWriter</code>. The service graph becomes a DAG. Cold-resolve of any audit_trail service no longer trips a cycle.</p> <h3>Replacing the five existing loggerFactory warning calls</h3> <ul> <li><strong>Lock-contention drop (AuditTrailLogger:188)</strong>: <code>audit_trail.dropped_under_contention</code> state counter already tracks this; the supplemental warning is redundant. Drop it.</li> <li><strong>Chain event mint / secret repository failures (lines 363, 397, 560, 581)</strong>: the new <code>AuditTrailInternalWriter</code> throws typed exceptions on these failure modes. <code>ChainArchiver</code> already wraps its writes in try/catch and reports through its own paths. The thin <code>AuditTrailLogger</code> wraps the writer call in try/catch and falls back to <code>error_log()</code> for the PSR-3 path that cannot throw.</li> </ul> <h3>Regression test</h3> <p>A new kernel test that builds a fresh container per service and resolves each write-bearing audit_trail service cold:</p> <pre>public function testColdResolveCriticalServices(): void { foreach ([ 'audit_trail.logger', 'audit_trail.internal_writer', 'audit_trail.chain_archiver', // ...every service a controller injects ] as $id) { $container = $this-&gt;rebootKernel(); $this-&gt;assertNotNull($container-&gt;get($id), $id); } }</pre><p>This test would have failed against the pre-split code. It will pass after the refactor and fail again if anyone re-introduces a cycle.</p> <h3>Remaining tasks</h3> <ul> <li>Create <code>src/Writer/AuditTrailInternalWriter.php</code> with the bulk of today's <code>AuditTrailLogger</code>.</li> <li>Slim <code>AuditTrailLogger</code> down to the entry-point shape.</li> <li>Update <code>audit_trail.services.yml</code> wiring + alias <code>@Drupal\audit_trail\AuditTrailInternalWriterInterface</code> to the new service.</li> <li>Convert the five <code>loggerFactory-&gt;warning(...)</code> callsites to typed exceptions / state counters / <code>error_log()</code> per the table above.</li> <li>Add the cold-resolve regression test.</li> <li>Run the full suite + verify no regression.</li> </ul>