Problem / motivation
Loading /admin/config/system/audit-trail/segments on a fresh container crashes with:
Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException: Circular reference detected for service "audit_trail.logger", path: "audit_trail.chain_archiver -> audit_trail.logger -> logger.factory".
Root cause is a service-wiring design smell: AuditTrailLogger wears two hats. It implements LoggerInterface (tagged logger, so the container's RegisterLoggersPass calls addLogger(@audit_trail.logger) on logger.factory), AND it implements AuditTrailInternalWriterInterface (so ChainArchiver, ChainTimestamper et al. inject it as their chain-event emitter). On top of that, it injects @logger.factory for fallback warning emission. The three together form a cycle: logger.factory cannot finish initialization without audit_trail.logger, which cannot finish construction without logger.factory.
Kernel tests do not catch this because the test harness warms logger.factory 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 chain_archiver.
Proposed resolution
A logger should be just a logger. Split into two classes with clearly separated responsibilities:
AuditTrailLogger(refactored): thinLoggerInterfaceentry point. Taggedlogger. Inspects the PSR-3context, forwardschain: TRUEcalls to the internal writer, no-ops the rest. NOlogger.factorydependency. Catastrophic failures fall back toerror_log()because PSR-3 forbids throwing fromlog()and we cannot re-enterlogger.factorywithout re-creating the cycle.AuditTrailInternalWriter(new class): all the existing row-build, canonicalize, HMAC, INSERT, chain-event-mint logic moves here. ImplementsAuditTrailInternalWriterInterface. NOT taggedlogger. Two public entry points:logChained(level, message, context)for the PSR-3 path (called by the logger above) andchainedWrite(chain, action, resource, buckets)for module internals (called byChainArchiver,ChainTimestamper, etc.). On failure: throws a typed exception. Callers handle.
After the split, AuditTrailInternalWriter takes the heavy dependency list (database, secret repository, lock, current_user, datetime.time, config.factory, request_stack, entity_type_manager, state) without @logger.factory. AuditTrailLogger takes only AuditTrailInternalWriter. The service graph becomes a DAG. Cold-resolve of any audit_trail service no longer trips a cycle.
Replacing the five existing loggerFactory warning calls
- Lock-contention drop (AuditTrailLogger:188):
audit_trail.dropped_under_contentionstate counter already tracks this; the supplemental warning is redundant. Drop it. - Chain event mint / secret repository failures (lines 363, 397, 560, 581): the new
AuditTrailInternalWriterthrows typed exceptions on these failure modes.ChainArchiveralready wraps its writes in try/catch and reports through its own paths. The thinAuditTrailLoggerwraps the writer call in try/catch and falls back toerror_log()for the PSR-3 path that cannot throw.
Regression test
A new kernel test that builds a fresh container per service and resolves each write-bearing audit_trail service cold:
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->rebootKernel();
$this->assertNotNull($container->get($id), $id);
}
}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.
Remaining tasks
- Create
src/Writer/AuditTrailInternalWriter.phpwith the bulk of today'sAuditTrailLogger. - Slim
AuditTrailLoggerdown to the entry-point shape. - Update
audit_trail.services.ymlwiring + alias@Drupal\audit_trail\AuditTrailInternalWriterInterfaceto the new service. - Convert the five
loggerFactory->warning(...)callsites to typed exceptions / state counters /error_log()per the table above. - Add the cold-resolve regression test.
- Run the full suite + verify no regression.
Issue fork audit_trail-3591791
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
mably commented