Initial propagation headers#1457
Conversation
waltjones
force-pushed
the
waltjones/propagation-headers
branch
from
80e41b9 to
d202180
Compare
shankj3
left a comment
shankj3
left a comment
There was a problem hiding this comment.
I'm still brain-fog here, so I had some questions to understand. Also, where is the sessionId added to the item attributes? Do we just get that for free, since tracing.sessionId is set?
| } | ||
|
|
||
| return function rollbarExpressMiddlewareHandler(req, _res, next) { | ||
| const sessionId = extractSessionId(getBaggageHeader(req)); |
There was a problem hiding this comment.
Do we want to create a sessionId, if one is not present?
There was a problem hiding this comment.
I have a doc forthcoming to discuss that. Whatever we do, it won't go in this PR.
| if (!mergedOptions.options.headers) { | ||
| mergedOptions.options.headers = {}; | ||
| } | ||
| mergedOptions.options.headers.baggage = `rollbar.session.id=${sessionId}`; |
There was a problem hiding this comment.
IIRC, this means we only forward the rollbar.session.id in the baggage header. Is there a reason why wouldn't we forward the entire thing?
There was a problem hiding this comment.
Answered async - duplicates of the same header are concatenated
That's existing code in src/rollbar.js _addItemAttributes. The only change we make in this PR is to also look for it in AsyncLocalStorage. |
waltjones
force-pushed
the
waltjones/propagation-headers
branch
from
55beef7 to
fc8a8bf
Compare
waltjones
force-pushed
the
waltjones/propagation-headers
branch
from
fc8a8bf to
5161ca4
Compare
2 participants
Description of the change
Adds support for the
baggagerequest header, enabling propagation of session id. Additional headers (traceparent,tracestate) and additional baggage keys may be enabled in follow up PRs.Background:
Organized by commit:
session_idattribute to Node (server) payloads, a6669eeType of change
Checklists
Development