INV-3970: log the Trace ID (trace_id) via TraceIdProcessor

[siarhei-kruk]

What

Adds a Monolog TraceIdProcessor that records a request-spanning Trace ID
in the trace_id log field, fed by a host-supplied provider seam
(TraceIdProviderInterface::getTraceId(): ?string).

Why

The Trace ID names an entire request across every service it touches. It is
distinct from Paysera-Correlation-Id, which names a single Hop (it is
regenerated per service, and the inbound value is consumed as the parent link).
So a separate field/mechanism is required to correlate one request's log lines
across services.

Changes

• src/Service/TraceIdProviderInterface.php — provider seam (getTraceId(): ?string).
• src/Service/Processor/TraceIdProcessor.php — Monolog processor mirroring
  CorrelationIdProcessor 1:1, including the Monolog v2/v3 (class_exists('Monolog\LogRecord'))
  dual-path. Writes the configured field (default trace_id); skips it when the
  provider is absent or returns null.
• src/DependencyInjection/{Configuration,PayseraLoggingExtraExtension}.php — new
  config keys trace_id_field (default trace_id) and trace_id_provider.
• src/Resources/config/services/processors.xml — registers the processor with the
  monolog.processor tag; the provider argument uses on-invalid="null".

Backward compatibility

Purely additive. The existing Correlation ID (Hop) classes are untouched. When no
trace_id_provider is configured the processor no-ops (on-invalid="null" +
nullable constructor), so existing consumers are unaffected and the container still
compiles.

Tests

Full suite green: 26 tests, 1170 assertions (includes a new TraceIdProcessorTest
covering value-present, null-skip, and configurable-field-name cases, and the existing
functional container-compilation test).

[gemini-code-assist]

Code Review

This pull request introduces a TraceIdProcessor to append a trace ID to Monolog log records, supporting both Monolog v1/v2 and v3+. It includes a TraceIdProviderInterface, configuration updates to register the processor, and corresponding unit tests. The review feedback suggests improving the Monolog v3 implementation by using the immutable with() method on LogRecord instead of manual instantiation, and removing typed properties from the test class to maintain compatibility with PHP versions older than 7.4.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[mSprunskas]

Probably should not be configurable

[siarhei-kruk]

fixed

[mSprunskas]

Config example in README would not hurt.

 1. A typo in trace_id_provider silently disables trace IDs — no error (High)
  
  Location: src/Resources/config/services/processors.xml:39 + src/DependencyInjection/PayseraLoggingExtraExtension.php:26-28

  on-invalid="null" is carrying two unrelated jobs. It's genuinely needed for the "no provider configured" case, but it also swallows a misconfigured one. setAlias() never checks that its target exists, so trace_id_provider: app.typoed_provider creates an
  alias to a non-existent service; ResolveReferencesToAliasesPass rewrites the reference to that dead id while preserving NULL_ON_INVALID_REFERENCE, and ResolveInvalidReferencesPass then quietly resolves it to null.

  I confirmed this by compiling a real ContainerBuilder against the vendored Symfony DI 4.4:

  CASE 1 (no config):                compiled OK
  CASE 2 (valid config):             compiled OK
  CASE 3 (TYPO in trace_id_provider): compiled OK  <-- silent!
    → injected provider: NULL

  Impact: an operator who misspells the service id gets a green boot, a green cache warmup, and logs that simply never contain trace_id. The failure only surfaces much later, during an incident, when someone tries to trace a request and finds the field
  missing. The same silent-null path also swallows trace_id_provider: ''.

  Fix: make "unconfigured" explicit rather than overloading the invalid-reference behaviour, so a typo becomes a hard compile error:

  // PayseraLoggingExtraExtension::load()
  if ($config['trace_id_provider'] !== null) {
      $container->setAlias(TraceIdProviderInterface::class, $config['trace_id_provider']);
  } else {
      $container->removeDefinition(TraceIdProcessor::class);
  }
  and drop on-invalid="null" from the XML argument. Note a $container->has(...) guard inside load() is not a reliable substitute — extensions run during MergeExtensionConfigurationPass, so a provider registered by another bundle's extension may not exist yet
  at that point. Adding ->cannotBeEmpty() to the config node would cover the empty-string case with a proper validation message. 

  2. trace_id is the first field discarded from oversize stdout records (Medium)

  Location: src/Service/Formatter/StdoutRecordEncoder.php:55-57, :104, :125

  The processor puts trace_id into extra. The stdout encoder hoists correlation_id out of extra to the top level and deliberately protects it — the truncation ladder drops the whole extra array at line 104 but doesn't drop correlation_id until line 125, as a
  last resort. trace_id gets neither treatment: it stays nested under extra, so on any record that exceeds the 32766-byte cap it's discarded early, several steps before correlation_id.

  Impact: oversize records are overwhelmingly exception dumps and stack traces — exactly the records where a request-spanning trace ID is most valuable. The field vanishes precisely when you need it. It's also inconsistent with the top-level placement of
  correlation_id that the README documents, which will complicate VictoriaLogs queries (extra.trace_id vs correlation_id). The Graylog path is unaffected — GelfMessageFormatter emits extra keys unprefixed, so trace_id lands as a normal additional there.

  Fix: hoist trace_id alongside correlation_id and give it the same late-drop position in the ladder. The comment at lines 123-124 ("correlation_id is the only one an oversize value can reach") would need updating to match.

  3. The default no-provider path is untested (Low)

  Location: tests/Unit/Service/Processor/TraceIdProcessorTest.php

  Both tests inject a mock. Nothing covers new TraceIdProcessor(null) — which is the out-of-the-box wiring for every consumer that doesn't set trace_id_provider, and the exact behaviour the CHANGELOG advertises ("The field is skipped when no provider is
  configured"). I verified manually that this path works correctly today, so this is a coverage gap rather than a live defect; it's worth a test because it's the branch most likely to regress unnoticed.

[mSprunskas]

  Issue 1 — Missing test: TraceIdProcessor constructed with null provider

  File: tests/Unit/Service/Processor/TraceIdProcessorTest.php
  Severity: Low

  The DI config (processors.xml:39) uses on-invalid="null", meaning when no trace_id_provider is configured, null is injected as the constructor argument. The processor handles this correctly (TraceIdProcessor.php:23 — $this->traceIdProvider !== null ? ...),
  but there is no test for new TraceIdProcessor(null). The existing testDoesNotAddKeyWhenNull (line 30) tests a mock provider that returns null — a different code path from a null provider itself.

  Suggested fix: Add a test case that constructs the processor with null and asserts the record passes through unchanged:

  public function testDoesNotAddKeyWhenNoProvider(): void
  {
      $processor = new TraceIdProcessor(null);

      $record = $this->invokeProcessor($processor);

      $this->assertArrayNotHasKey('trace_id', $this->getAllExtra($record));
  }

  ---
  Issue 2 — StdoutRecordEncoder is unaware of trace_id

  File: src/Service/Formatter/StdoutRecordEncoder.php:54-80
  Severity: Low (design concern)

  The StdoutRecordEncoder hoists correlation_id from extra to a top-level JSON field (lines 55-58, 79) and gives it special treatment during oversize truncation (lines 123-125). The new trace_id stays buried inside extra. During truncation, when extra is
  dropped (line 104), trace_id is silently lost while correlation_id survives one additional truncation stage.

  If trace_id is meant to be an important observability signal in VictoriaLogs, it should receive the same hoisting treatment as correlation_id. If it is intentionally lower-priority, no change is needed — but this asymmetry is worth a deliberate decision.

  ---
  Issue 3 — Test boilerplate: Monolog v1/v3 helpers duplicated per test class
  
  File: tests/Unit/Service/Processor/TraceIdProcessorTest.php:43-91
  Severity: Low (maintainability)

  The invokeProcessor, getExtra, and getAllExtra private methods (~50 lines) duplicate the Monolog v1/v3 branching logic that every processor test needs. If similar helpers already exist in other processor test classes (CorrelationIdProcessorTest,
  ParentCorrelationIdProcessorTest), they should be extracted to a shared test trait or base class to avoid repeating the same compatibility scaffolding in each test file.

[mSprunskas]

  1. trace_id is never promoted to a Sentry tag, unlike correlation_id

  - Issue Type: Backwards Compatibility / Logic Consistency — new field not propagated to all code paths
  - Severity: Medium — the only finding with user-facing behavioural impact
  - Location: src/Service/Processor/SentryContextProcessor.php:17-19 (Monolog v3 branch) and :45-47 (Monolog v1/v2 branch)

  Problem: SentryContextProcessor special-cases extra.correlation_id and copies it into context.tags.correlation_id, which is what makes it a searchable, filterable Sentry tag. The new trace_id has no equivalent branch. It reaches Sentry only via the generic
  merge on line 16/44 ($record['context']['extra'] = … + $record['extra'] + …), i.e. as an event extra.

  This is precisely the propagation red flag in the review protocol: one path (correlation_id) applies the promotion logic, and the sibling field added for the same purpose bypasses it.

  Impact: README.md:190-193 instructs users — directly beneath the documented Sentry→Graylog workflow — "To follow one request across services instead of within one process, search by trace_id". In Graylog that works (GelfMessageFormatter is constructed with
  a null extraPrefix at formatters.xml:11-14, so trace_id arrives as a plain unprefixed additional; the functional test confirms it). In Sentry it does not: you cannot search, filter or group by trace_id, you must open an individual event and read its
  extras. Ordering is not the blocker — TraceIdProcessor is a logger-level processor while SentryContextProcessor is handler-scoped (<tag name="monolog.processor" handler="sentry"/>), so extra.trace_id is already populated when it runs.

  Fix: mirror the correlation-id block in both conditional branches:

  Summary — most critical first
  }
  If tag promotion is deliberately out of scope, correct the README instead to state that trace_id is searchable in Graylog/VictoriaLogs but only visible per-event in Sentry.
  
  ---
  2. trace_id_provider is validated with trim() but stored untrimmed
  
  - Issue Type: Robustness / validation-consumption mismatch 
  - Severity: Low-Medium
  - Location: src/DependencyInjection/Configuration.php:28-30, consumed at src/DependencyInjection/PayseraLoggingExtraExtension.php:27-28           
  
  Problem:
  ->ifTrue(static function ($value): bool {
      return $value !== null && trim((string) $value) === '';
  })
  The validator uses trim() to decide what counts as blank, but the extension passes the raw value straight to setAlias(). The validation and the stored value therefore disagree about what the configured id is.
  
  Impact: trace_id_provider: ' app.trace_id_provider ' — trivially produced by YAML quoting or a copy-paste — passes validation, then fails container compilation with ServiceNotFoundException: You have requested a non-existent service " app.trace_id_provider
  ", a message whose actual cause (a leading space) is nearly invisible.
  
  Fix: normalise rather than only reject — add ->beforeNormalization()->ifString()->then(static fn ($v) => trim($v))->end() ahead of the validate block; or drop trim() from the predicate so validation and storage agree on plain-empty semantics.
  
  ---
  3. Duplicate hoist tests, in a file that already introduced the provider for the sibling pair                                                     
  
  - Issue Type: Test boilerplate / internal inconsistency
  - Severity: Low 
  - Location: tests/Unit/Service/Formatter/StdoutJsonFormatterTest.php:98-124
  
  Problem: testHoistsCorrelationIdFromExtraToTopLevel and testHoistsTraceIdFromExtraToTopLevel have byte-identical 8-line bodies differing only in field name and sample value. The same change already added hoistedIdProvider() (lines 268-275) to collapse 
  exactly this correlation_id/trace_id pair for the drop test — but left the hoist pair duplicated. One file, one pair of fields, two opposite idioms.

  Fix:
  /**
   * @dataProvider hoistedIdProvider
   */
  public function testHoistsIdFromExtraToTopLevel(string $field): void
  {
      $decoded = $this->decode(['extra' => [$field => 'hoisted-value', 'memory_peak' => '2 MB']]);

      $this->assertSame('hoisted-value', $decoded[$field]);

      $extra = $decoded['extra'];
      $this->assertIsArray($extra);
      $this->assertArrayNotHasKey($field, $extra);
      $this->assertSame('2 MB', $extra['memory_peak']);
  }
  Net −14 lines, and hoistedIdProvider becomes the single registration point for a future third hoisted id. If the realistic correlation-id sample value (app-target2-integration6a171447acf112.97444679) is worth preserving, widen the provider to [$field,
  $sampleValue].

  ---
  4. ConfigurationTest — three boilerplate methods that are really one table

  - Issue Type: Test boilerplate / internal inconsistency
  - Severity: Low
  - Location: tests/Unit/DependencyInjection/ConfigurationTest.php:13-38

  Problem: testTraceIdProviderDefaultsToNull, testKeepsConfiguredTraceIdProvider and testAllowsExplicitNullTraceIdProvider are the same three lines each (build input → process() → assert value), and two of them assert literally the same thing (assertNull).
  Same idiom split as #3: this file already uses blankTraceIdProviderProvider for the rejection cases.

  Fix:
  /**
   * @dataProvider traceIdProviderProvider
   *
   * @param array<string, mixed> $input
   */
  public function testResolvesTraceIdProvider(array $input, ?string $expected): void
  {
      $config = $this->process(['application_name' => 'app-something'] + $input);

      $this->assertSame($expected, $config['trace_id_provider']);
  }

  /**
   * @return array<string, array{array<string, mixed>, string|null}>
   */
  public static function traceIdProviderProvider(): array
  {
      return [
          'omitted defaults to null' => [[], null],
          'explicit null stays null' => [['trace_id_provider' => null], null],
          'configured id is kept' => [['trace_id_provider' => 'app.trace_id_provider'], 'app.trace_id_provider'],
      ];
  }
  Net −12 lines.

  ---
  5. TraceIdProcessorTest — repeated arrange block and an unnecessary parameter

  - Issue Type: Test boilerplate / divergence from sibling test
  - Severity: Low
  - Location: tests/Unit/Service/Processor/TraceIdProcessorTest.php:23-61

  Problem: All three tests repeat the same two arrange lines, and invokeProcessor() takes the processor back as a parameter even though its collaborator is already a field:
  $this->provider->method('getTraceId')->willReturn(…);
  $processor = new TraceIdProcessor($this->provider);
  The sibling ParentCorrelationIdProcessorTest builds $this->processor in setUp() and has a zero-arg invokeProcessor(). There is no ordering constraint forcing the split — the mock is an object reference, so willReturn() may be configured after the SUT is
  constructed.

  Note: this is not a data-provider candidate; the three tests assert genuinely different behaviours. Only the arrange duplication should be removed.

  Fix:
  protected function setUp(): void
  {
      $this->provider = $this->createMock(TraceIdProviderInterface::class);
      $this->processor = new TraceIdProcessor($this->provider);
  }

  private function invokeProcessor()
  {
      return ($this->processor)($this->buildLogRecord(['extra' => ['existing' => 'kept']]));
  }
  Each test drops to a single arrange line, and the two processor tests become structurally identical — which matters, because they are meant to be read as a pair.

  ---
  6. The byte-cap rationale is duplicated across four locations

  - Issue Type: Redundant / overly verbose comments
  - Severity: Low
  - Locations: tests/Unit/Service/Formatter/StdoutJsonFormatterTest.php:251-252; src/Service/Formatter/StdoutRecordEncoder.php:130-132; README.md:173-175; CHANGELOG.md:10

  Problem: One design decision — "the hoisted ids are dropped last so oversize records stay traceable" — is now written out four times, three of them as prose. They will drift.

  Fixes, in priority order:
  - StdoutJsonFormatterTest.php:251-252 — delete outright. It restates the implementation comment verbatim rather than saying anything about the test, and the test name testDropsHoistedIdsWhenOneAloneExceedsTheByteCap already states the asserted behaviour.
  This is the clearest redundancy in the changelist.
  - StdoutRecordEncoder.php:132 — trim the third sentence. The first two sentences are load-bearing (they explain why only these two fields can be oversize at runtime). "They go last so a request stays traceable on every record that can still carry them" is
  motivation the README already carries. The pre-change comment was two lines; it grew to three while adding only one field name.
  - CHANGELOG.md:8,10 — optional. Both 3.5.0 bullets restate the README nearly sentence-for-sentence. Keeping the changelog factual (what changed, how to opt in, BC impact) and letting the README own the why would remove the third and fourth copies. Flagged
  as a suggestion only: the existing 3.4.1 entry is equally essay-length, so this is established house style.

  ---
  7. Functional fixtures are 54-line verbatim copies of basic.yml

  - Issue Type: Maintainability
  - Severity: Low
  - Location: tests/Functional/Fixtures/config/cases/trace_id.yml, tests/Functional/Fixtures/config/cases/trace_id_misspelled_provider.yml

  Problem: Both files reproduce the entire monolog: and sentry: blocks from basic.yml character-for-character, differing from it — and from each other — only in the trailing services: / trace_id_provider: lines. There are now three copies of the same handler
  graph.

  Impact: any future change to the fixture handler chain (as happened in the recent stdout/VictoriaLogs work) must be applied three times; a missed copy surfaces as a confusing, case-specific failure.

  Fix: factor the shared handler/sentry config into an imported fragment, or imports: basic.yml and append only the delta — bundle extension config merges across loaded files, so adding paysera_logging_extra.trace_id_provider in the case file is sufficient.

  ---
  8. Dead service definition in the misspelled-provider fixture

  - Issue Type: Dead configuration / obscured test intent
  - Severity: Low
  - Location: tests/Functional/Fixtures/config/cases/trace_id_misspelled_provider.yml:49-51

  Problem: the file defines test_trace_id_provider while the config deliberately points at test_trace_id_provider_typo. Nothing references the definition; it is removed as unused during compilation and contributes nothing to the assertion.

  Impact: a reader must diff against trace_id.yml to see that the point is the typo. The stray definition suggests the test distinguishes "service exists but id mismatched" from "id does not exist at all" — the container behaves identically either way.

  Fix: delete the services: block (the test still passes and reads more clearly), or keep it with a comment stating that the correctly-named service exists precisely to prove the alias target, not the class, is what fails.

  ---
  9. FunctionalTraceIdTest does not shut down the kernel or reset the container

  - Issue Type: Test hygiene / divergence from established pattern
  - Severity: Low
  - Location: tests/Functional/FunctionalTraceIdTest.php:22-31, versus tests/Functional/FunctionalTestCase.php:41-50

  Problem: the established teardown in this suite shuts the kernel down and resets the container before removing the cache dir. The new test only removes the cache dir. Three kernels are booted in one process — two of them fully, with Monolog handlers, a
  Sentry client and an in-memory SQLite connection — and none are shut down or reset.

  Not extending FunctionalTestCase is defensible here: its tearDown() calls $this->kernel->getContainer() unguarded, which would fatal after the intentionally-failing testFailsToCompileWhenProviderIsMisspelled boot. But the fix for that is a null guard, not
  dropping the shutdown entirely.

  Impact: no current failure (verified — the suite is green on both Monolog majors), but it leaks handler and connection state across test methods and diverges from the pattern every other functional test in the directory follows. That is the kind of drift
  that later surfaces as an order-dependent flake.

  Fix: add $this->kernel->shutdown() plus the container reset(), guarded for the case where boot threw before a container existed.

  ---
  10. No test asserts that trace_id reaches Sentry

  - Issue Type: Test coverage gap
  - Severity: Low (root cause of issue #1 escaping review)
  - Location: tests/Functional/FunctionalTraceIdTest.php

  Problem: coverage is Graylog-only (getAllAdditionals()). FunctionalHandlersTest::testCorrelationId asserts correlation_id on both the Sentry event tags and the Graylog additionals; the trace-id equivalent asserts only the Graylog half. The infrastructure
  to close the gap (sentry_transport, sentry_client, getSingleSentryEvent()) already exists and would have caught issue #1.

  Fix: add a Sentry-side assertion alongside the Graylog one, matching whatever resolution is chosen for #1.

[mSprunskas]

1. Oversize hoisted id destroys message and collaterally drops the other id

  Type: Correctness — shrink-ordering bug (pre-existing path, materially widened by this change)
  Location: src/Service/Formatter/StdoutRecordEncoder.php:118-133

  encodeWithinByteLimit() computes the truncation budget against a JSON blob that still contains the oversize hoisted id, truncates message by that overflow, and only then drops correlation_id/trace_id. I reproduced this against the real class:

  input : message 'small message', correlation_id 'corr-abc-123', trace_id str_repeat('t', 40000)
  output: 151 bytes — keys: timestamp, application_name, channel, level, level_name, message, truncated
          message === ''      <-- wiped
          correlation_id      <-- gone, despite being 12 bytes
          trace_id            <-- gone

  Impact:
  - A single oversize trace_id wipes an otherwise-fine message to the empty string, in a 151-byte line that had ~32.6 KB of headroom. The log record becomes useless.
  - correlation_id is now dropped as collateral. Before this change correlation_id could only be lost when it was oversize; now an unrelated field kills it, which contradicts the stated intent in README/CHANGELOG ("drops it only as a last resort", "so they
  survive the extra drop").
  - The provider is host-supplied (TraceIdProviderInterface), so the bundle has no control over the value's length — this is reachable input, not a theoretical one.

  Fix: drop the unbounded hoisted fields before the message truncation step, and drop them individually with a re-check between:

  foreach (['trace_id', 'correlation_id'] as $key) {
      if (isset($fields[$key])) {
          unset($fields[$key]);
          $json = $this->toJson($fields);
          if (strlen($json) <= self::MAX_JSON_BYTE_COUNT) {
              return $json;
          }
      }
  }
  // only now compute $overflow and truncate message

  ---
  2. The new test codifies the broken behaviour instead of catching it

  Type: Test coverage
  Location: tests/Unit/Service/Formatter/StdoutJsonFormatterTest.php:234-249 (testDropsHoistedIdsWhenOneAloneExceedsTheByteCap)

  The test feeds a 40 000-byte id plus a 1 000-byte message and asserts only assertArrayNotHasKey($field, $decoded) and that the line fits the cap. Both assertions pass while message is silently emptied and the other id is silently discarded — exactly the
  defect in #1. The test as written would not fail if the bug were fixed either, so it provides no regression protection in either direction.

  Fix: add assertions that message survives intact and that the non-oversize id is retained, e.g. for trace_id oversize assert $decoded['correlation_id'] === 'corr-1' and $decoded['message'] still starts with the original content.

  ---
  3. Non-string trace_id_provider values escape validation and fail with an opaque DI error
  
  Type: Input validation / error quality
  Location: src/DependencyInjection/Configuration.php:25-39 → src/DependencyInjection/PayseraLoggingExtraExtension.php:27-31

  beforeNormalization()->ifString() only trims strings, and the validate() guard only rejects the empty string. A YAML boolean or integer passes both:

  paysera_logging_extra:
      trace_id_provider: true     # scalarNode accepts it; validate() sees (string) true === '1' -> valid

  The value then reaches $container->setAlias(TraceIdProviderInterface::class, true), which fails with InvalidArgumentException: $id must be a string, or an Alias object (Symfony 4.4/5) or a raw TypeError (Symfony 6/7) deep in the container builder — instead
  of the clear InvalidConfigurationException the node was clearly designed to produce. trace_id_provider: false is caught (casts to '') while true is not, which is arbitrary.

  Fix: widen the guard to reject any non-string, non-null value:

  ->ifTrue(static function ($value): bool {
      return $value !== null && (!is_string($value) || trim($value) === '');
  })
  ->thenInvalid('The trace_id_provider must be a non-empty service id, got %s.')

  (The trim() in the closure also makes the beforeNormalization block optional rather than load-bearing.)

  ---
  4. FunctionalTraceIdTest bypasses FunctionalTestCase and re-implements its infrastructure
  
  Type: Maintainability — duplication
  Location: tests/Functional/FunctionalTraceIdTest.php:22-136

  Every other functional test extends FunctionalTestCase, which already provides setUpContainer() and the kernel/cache/container-reset tearDown(). This class extends plain TestCase and hand-rolls all of it, plus re-implements Sentry and Graylog accessors
  that already exist in FunctionalHandlersTest (getSentryEvents(), getGraylogMessages()).

  The only justification given in the comment is that testFailsToCompileWhenProviderIsMisspelled boots a kernel that never produces a container — but that is a tearDown() override, not a reason to abandon the base class. Concretely duplicated: kernel
  construction, cache-dir removal before/after, ResettableContainerInterface/ResetInterface reset, $sentryClient->flush() + transport read, mainHandler->close() + flushPublishedMessages().

  Fix: extend FunctionalTestCase, use setUpContainer($testCase), and override tearDown() with the null-container guard. Better still, lift the two accessors into the base class so FunctionalHandlersTest and this test share them.

  ---
  5. Dead use of a class removed in Symfony 5.0
  
          $json = $this->toJson($fields);
          if (strlen($json) <= self::MAX_JSON_BYTE_COUNT) {
              return $json;
          }
      }
  }
  // only now compute $overflow and truncate message

  ---
  6. Empty-string trace id is recorded and promoted to a Sentry tag

  Type: Consistency
  Location: src/Service/Processor/TraceIdProcessor.php:23-25, 56-58; src/Service/Processor/SentryContextProcessor.php:20-22, 51-53

  The processor guards on === null only. A provider returning '' (an easy outcome for a header-reading implementation such as $request->headers->get('X-Trace-Id') ?? '') writes extra.trace_id = '', which then survives StdoutRecordEncoder::filterEmptyFields()
  (it removes only null and []), lands in Graylog as an empty additional, and passes isset() in SentryContextProcessor to become an empty Sentry tag.

  This is inconsistent with the configuration layer, which goes out of its way to reject a blank trace_id_provider. Empty tags are noise in Sentry's tag index and an empty trace_id is worse than an absent one, since it looks like a real value when filtering.

  Fix: treat blank as absent — if ($traceId === null || $traceId === '') { return $record; }. Optionally document the ''-is-not-a-trace-id contract in the TraceIdProviderInterface docblock, which currently has none.

[mSprunskas]

  Issue 1: IterationEndListener constructor — backwards-incompatible signature change in a minor version

  File: src/Listener/IterationEndListener.php:25-29
  Type: Backwards Compatibility

  The new required TraceIdProvider $traceIdProvider parameter is inserted before the optional ?ClientInterface $sentryClient. Any consumer that previously called:

  new IterationEndListener($corrIdProvider, $parentCorrIdProvider, $sentryClient);

  now silently passes the Sentry client as the 3rd argument ($traceIdProvider), causing a TypeError at runtime. This is versioned as 3.5.0 (minor), which under semver must be backwards-compatible.

  Impact: Any service configuration or test outside this bundle that constructs IterationEndListener with 3 positional arguments breaks.

  Fix: Either make $traceIdProvider the last parameter with a default (?TraceIdProvider $traceIdProvider = null), or bump the major version.

  ---
  Issue 2: testEmitsCanonicalFieldOrder does not include parent_corr_id in the test data
  
  File: tests/Unit/Service/Formatter/StdoutJsonFormatterTest.php:41-66
  Type: Test coverage gap

  The field-order assertion test provides only correlation_id and trace_id in extra, skipping parent_corr_id. Since the encoder now hoists three IDs in the order correlation_id → parent_corr_id → trace_id, the test does not verify the complete canonical
  order when all three are present. A future reordering of those fields would go undetected.

  Impact: Low — the production code is correct, but the test doesn't protect the full field order contract.

  Fix: Add 'parent_corr_id' => 'parent-1' to the test's extra and include 'parent_corr_id' between 'correlation_id' and 'trace_id' in the expected key list.