♻️ split Assembly into MainAssembly and RendererPipeline#133
Conversation
- RendererAssembly owns BRIDGE_CHANNEL/CONFIG_CHANNEL IPC, calls triggerRenderer hook, emits ServerRumEvent{source:RENDERER} directly
- UserActivityTracker now subscribes to ServerRumEvent{source:RENDERER} instead of RawRumEvent
- Remove renderer assembly logic from Assembly
- `Assembly` → `MainAssembly` - `source` field removed from `RawRumEvent` / `RawTelemetryEvent` (always main-process) - `BridgeHandler` deleted
bcaudan
force-pushed
the
bcaudan/renderer-assembly
branch
from
95ec394 to
9757676
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97576760c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
bcaudan
changed the title
Use generateTelemetryErrors (IPC, no button click) so no browser SDK action is buffered before session close. Otherwise, a delayed delivery of the pre-expiry click would accidentally renew the session
…ivity tracking - rename RendererAssembly → RendererPipeline (broader responsibilities) - emit END_USER_ACTIVITY for clicks before the DISCARDED check, so a click after session expiry renews the session even when its timestamp falls outside the closed session window - delete UserActivityTracker (now superseded by RendererPipeline)
bcaudan
force-pushed
the
bcaudan/renderer-assembly
branch
from
9757676 to
9af1dc2
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| * after session inactivity expiry can create a new session even though the event itself | ||
| * would be discarded (its timestamp falls outside the closed session window). | ||
| */ | ||
| export class RendererPipeline { |
There was a problem hiding this comment.
I'm not sure merging BridgeHandler into RendererPipeline is the right call here. We're merging together two classes with very distinct responsibilities, IPC transport and event enrichment. It would be better to keep the concerns separate, with a RendererAssembly to enrich renderer events.
There was a problem hiding this comment.
I had also in mind the coming changes with replay records being handled separately.
So we have:
- IPC message handling
- Some messages being enriched and sent through the event manager
- Some messages being handled by a dedicated class
That is why I have introduced a RendererPipeline instead of just a RendererAssembly, I felt that we needed an orchestrator for all that.
IMO, we could let it grow with the other event types and see which parts we'd want to extract then.
- add `source: EventSource` to RumAssembleParams, TelemetryAssembleParams, SpanAssembleParams - remove triggerRenderer, registerRenderer, RendererHookResult, RendererAssembleParams - switch on source in commonContext.registerRum and ViewContext.registerRum - RendererPipeline calls triggerRum(source: RENDERER) instead of triggerRenderer - update all call sites, specs, and architecture doc
cdn34dd
left a comment
cdn34dd
left a comment
There was a problem hiding this comment.
Looks good, with the current changes integrating the SR PR will be much simpler now.
2 participants
Motivation
Assemblyhandled two unrelated things: renderer events (pre-assembled by the browser SDK, needing a few native attributes injected) and main-process events (assembled from scratch via the full hook chain).All renderer tracks share the same origin (the bridge) and the same minimal enrichment need.
Splitting into
MainAssemblyandRendererPipelinemakes the bridge the single entry point for all renderer events, keeps enrichment logic colocated, and makes adding future renderer tracks (logs, telemetry) straightforward.Changes
MainAssembly, handling main-processRawEvents via the existingtriggerRum/triggerTelemetryhook chain.RendererPipeline, owning the bridge IPC channel and assembling renderer events. EmitsEND_USER_ACTIVITYfor click actions before the session check, so a click after session inactivity expiry renews the session even when its timestamp falls outside the closed session window.triggerRendererhook registered bySessionContext,ViewContext, andcommonContextfor timestamp-based context injection on renderer events.sourcefromRawEventtypes; all raw events are now invariably main-process originated.Test instructions
Checklist