feat: Add FeatureMessage, MessageBus, and a messaging thread#180
Merged
Conversation
This pattern is consistent with how the SDK currently manages all Core-owned background threads: for the sake of clarity, each dedicated background thread gets its own header (e.g. `messaging_thread.hpp`) and its own consistently-named entry point (e.g. `MessagingThreadMain`), and the Core explicitly manages the lifetime of each thread, rather than adding layers of indirection to thread startup/shutdown logic by encapsulating them in other classes. In other words, data (e.g. `MessageBus`) is kept entirely separate from the Core-owned `std::thread` that runs the thread's logic (e.g. `MessagingThreadMain`). If we change this pattern, we should do so consistently for all threads' implementations.
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ 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". |
fuzzybinary
approved these changes
Mar 19, 2026
| private: | ||
| friend void MessagingThreadMain(const DiagnosticLogger&, MessageBus&); | ||
|
|
||
| Queue<FeatureMessage> _queue; |
Member
There was a problem hiding this comment.
I probably should have said this back when this was first created but Queue seems like too simple a name for everything this does....
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
refs: RUM-12206
This PR adds a
MessageBusto theCore, along with a new dedicated background thread: the messaging thread.Rationale
For the typical work done by features (e.g. reading from
CoreContextand generating events), the existing context thread is sufficient: a feature enqueues its event-generation function to be run on the context thread, and that function will be invoked with an immutable, thread-safe copy of theCoreContext. This ensures that when a feature has decided to produce an event, it always sees a consistent, up-to-date snapshot of theCoreContext.However, we still need a non-lazy mechanism of propagating
CoreContextchanges to features that need to eagerly perform work in response to those changes. This same mechanism can be used for other state changes that need to proactively signal other features. TheMessageBusis designed with two use cases in mind:The
CrashReportingfeature needs to respond to changes in RUM context etc., conveying the new values to its crash handler implementation, such that they can be a.) eagerly flushed to disk alongside the crash report file (inprocess), or b.) stored as annotations to be read by an external process in the result of a crash (crashpad).The
Rumfeature needs to respond to critical errors recorded viaLogging(when configured to do so), automatically recording a RUM Error in response to the log call.This PR does not attempt to implement these feature-specific changes; it is concerned only with the defining the net-new types and functions required by the Core to support messaging.
Scope of this PR
RUM-12206 represents the Core SDK changes required to support core-to-feature and feature-to-feature messaging, not including any feature-specific changes.
The implementation of RUM-12206 is split into three phases:
Core; updatingFeaturewith the ability to register itself with the message bus and respond to messages (enabling theCrashReportinguse case described above)Featurewith the ability to send messages on the message bus (enabling theLogging->Rumuse case described above) - this may be a good point to decide on whether features should explicitly subscribe on a per-message-type basis to limit broadcast overhead.This PR represents the completion of Phase 1.
Changes in this PR
FeatureMessage, astd::variantthat currently just consists ofContextChangedMessageMessageBus, which wraps aQueueofFeatureMessageand a list of registered handlers, represented asstd::functioncallbacksMessagingThreadMain, which consumes from the queue, invoking all registered handlers for each message consumedMessageBusMessagingThreadMain