LW-3297 Capture parent correlation ID from incoming requests#22
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-provider LW-3365 Add ParentCorrelationIdProvider service class
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-listener LW-3306 Add ParentCorrelationIdListener
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-processor LW-3307 Add ParentCorrelationIdProcessor
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lation-id-services LW-3308 Register parent correlation ID services in DI configuration
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-tests LW-3309 Add unit tests for parent correlation ID components
siarhei-kruk
force-pushed
the
LW-3297-capture-parent-correlation-id
branch
from
0c8d704 to
be8f94f
Compare
| return $record; | ||
| } | ||
|
|
||
| return new \Monolog\LogRecord( |
There was a problem hiding this comment.
Why separate log entry is needed? Why not just append parent correlation to record in \Paysera\LoggingExtraBundle\Service\Processor\CorrelationIdProcessor::__invoke ?
There was a problem hiding this comment.
You're absolutely right! But I deliberately made it this way, to be as much compliant with CorrelationIdProcessor, as possible. (basically, claude just copied CorrelationIdProcessor with minor changes)
| $record->level, | ||
| $record->message, | ||
| $record->context, | ||
| array_merge($record->extra, ['parent_correlation_id' => $parentCorrelationId]), |
There was a problem hiding this comment.
With this approach query in graylog / elasticsearch would be correlation_id = ... OR parent_correlation_id = ...
I'd rather go with introducing totally new value, e.g. request_trace_id or smth, so we'd have two different things:
- filtering by correlation id, which shows logs from single request, filtering logic does not change
- filtering by new property, which would show trace of all requests (backend-for-frontend service A -> B backend service -> C backend dependency), new filtering case for "global" tracing.
In general, having parent_correlation_id kinda achieves this, but naming might be confusing - not exactly clear when to use correlation id and when to use parent correlation id.
Alternatively, if filtering backwards-compatibility does not matter much, lets just set value to correlation id from parent correlation id header, the only downside I see is that current filtering will start revealing logs with correlation id belonging to different app, because correlation id currently has app prefix.
PM for exact examples, don't want to disclose too much publicly
There was a problem hiding this comment.
The idea behind all this to have both correlation_id (current one, without changes) and parent_correlation_id - which basically is current correlation_id in calling application - in the same log record. It's not the global correlation id for the whole chain. It's just allows to build call chain like a linked list of a sort.
siarhei-kruk
force-pushed
the
LW-3297-capture-parent-correlation-id
branch
from
aba29ac to
fc90f4d
Compare
siarhei-kruk
force-pushed
the
LW-3297-capture-parent-correlation-id
branch
from
980b1df to
54d237e
Compare
| return $this->parentCorrelationId; | ||
| } | ||
|
|
||
| public function setParentCorrelationId(?string $parentCorrelationId): void |
There was a problem hiding this comment.
would be better to only accept string in case there is no use-case where currently set value should be reset setParentCorrelationId(string $parentCorrelationId): void
There was a problem hiding this comment.
Listener can set it to null when no header provided. it's normal use-case by design.
There was a problem hiding this comment.
Yes, header value can be null, but it doesn't meant that ParentCorrelationIdProvider::$parentCorrelationId value is not null, hence "reset" scenario. If there is no way that value should be reset, then just accept string and move non-null validation to listener, this is how it's done with DTOs, and this one, from code perspective is a typical DTO, it just has service definition, which makes it a "service", in a sense that it's object managed by singleton pattern,
- FQCN as services IDs - got rid of duplicate constant - code style fix in ParentCorrelationIdProvider
- Add IterationEndListenerTest covering parent correlation ID reset and Sentry flush - Update ParentCorrelationIdProviderTest to use resetParentCorrelationId() instead of nullable setter - Update ParentCorrelationIdListenerTest to verify header absence preserves existing value
2 participants
Implement parent correlation ID capture in
paysera/lib-logging-extra-bundle. When a service receives an HTTP request containing theX-Paysera-Correlation-Idheader (set by an upstream calling), the system captures this value and includes it asparent_correlation_idin all Monolog log records.Changes
ParentCorrelationIdProvider— stores the parent correlation ID for the current requestParentCorrelationIdListener—kernel.requestlistener (priority 255, main request only) that reads theX-Paysera-Correlation-IdheaderParentCorrelationIdProcessor— Monolog processor that addsparent_correlation_idto log recordextrawhen present (Monolog v1/v2/v3)services.xml,listeners.xml,processors.xml