-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add context thread, enqueue all RUM and Logging operations for async processing #169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
57b54d7
d74de51
511f69e
c798ea3
01af161
3ad3c53
dc4db37
77d6641
4fcd192
f840aef
d5ae4ae
e1d8b3a
3f579fb
e743d19
0005f06
dac8a27
b00dd9a
6c12f14
101afbf
37d0946
1dc05e9
77dfaa9
4e679de
e19810f
fcbdf93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2025-Present Datadog, Inc. | ||
|
|
||
| #include "datadog/impl/core/context_thread.hpp" | ||
|
|
||
| namespace datadog::impl { | ||
|
|
||
| void ContextThreadMain( | ||
| const DiagnosticLogger& diagnostic_logger, Queue<std::function<void()>>& queue | ||
| ) { | ||
| diagnostic_logger.Debug("Context thread starting"); | ||
|
|
||
| // Process functions until the queue is stopped and drained (Pop() returns | ||
| // std::nullopt) | ||
| while (auto thunk = queue.Pop()) { | ||
| (*thunk)(); | ||
| } | ||
|
|
||
| diagnostic_logger.Debug("Context thread finished"); | ||
| } | ||
|
|
||
| } // namespace datadog::impl |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2025-Present Datadog, Inc. | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <functional> | ||
|
|
||
| #include "datadog/impl/core/context.hpp" | ||
| #include "datadog/impl/core/queue.hpp" | ||
| #include "datadog/impl/diagnostics.hpp" | ||
|
|
||
| namespace datadog::impl { | ||
|
|
||
| /** | ||
| * Entry point for the context thread. | ||
| * | ||
| * The context thread continually reads functions from the context queue, execute each | ||
| * function serially as it's consumed. This allows features to perform operations | ||
| * asynchronously without blocking their callers. | ||
| * | ||
| * Functions are enqueued via FeatureScope::ExecuteOnContextThread et al. - FeatureScope | ||
| * is responsible for wrapping Feature-provided functions in a thunk that will evaluate | ||
| * the required parameters (CoreContext and EventWriter) from a CoreContextProvider, so | ||
| * the context thread itself is dead-simple: it just executes those thunks. | ||
| * | ||
| * @param diagnostic_logger Interface for logging status/warning messages. | ||
| * @param queue Non-owning reference to the thread-safe queue that we read from; | ||
| * guaranteed to outlive the thread. | ||
| */ | ||
| void ContextThreadMain( | ||
| const DiagnosticLogger& diagnostic_logger, Queue<std::function<void()>>& queue | ||
| ); | ||
|
|
||
| } // namespace datadog::impl |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,28 +12,89 @@ namespace datadog::impl { | |
|
|
||
| FeatureScope::FeatureScope( | ||
| CoreContextProvider& context_provider, | ||
| const EventGeneratedFunc& event_generated_func, | ||
| const DiagnosticLogger& in_diagnostic_logger | ||
| const EventWriter& event_writer, | ||
| const DiagnosticLogger& in_diagnostic_logger, | ||
| FeatureScope::ExecutionMode mode, | ||
| Queue<std::function<void()>>* context_queue | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really wish C++ had a nice way to enforce single ownership with non-owning observers that didn't require |
||
| ) | ||
| : _context_provider(&context_provider), | ||
| _event_generated_func(event_generated_func), | ||
| diagnostic_logger(in_diagnostic_logger) {} | ||
| _event_generated_func(event_writer), | ||
| _mode(mode), | ||
| _context_queue(context_queue), | ||
| diagnostic_logger(in_diagnostic_logger) { | ||
| // Invariant: context_queue must be non-null when mode is OnContextThread | ||
| DATADOG_ASSERT( | ||
| mode != FeatureScope::ExecutionMode::OnContextThread || context_queue != nullptr, | ||
| "context_queue must be non-null when ExecutionMode is OnContextThread" | ||
| ); | ||
| } | ||
|
|
||
| CoreContext FeatureScope::GetContext() const { | ||
| DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); | ||
| return _context_provider->Get(); | ||
| FeatureScope FeatureScope::Create( | ||
| CoreContextProvider& context_provider, | ||
| const EventWriter& event_writer, | ||
| const DiagnosticLogger& diagnostic_logger, | ||
| Queue<std::function<void()>>& context_queue | ||
| ) { | ||
| return FeatureScope( | ||
| context_provider, | ||
| event_writer, | ||
| diagnostic_logger, | ||
| FeatureScope::ExecutionMode::OnContextThread, | ||
| &context_queue | ||
| ); | ||
| } | ||
|
|
||
| FeatureScope FeatureScope::CreateForTesting( | ||
| CoreContextProvider& context_provider, | ||
| const EventWriter& event_writer, | ||
| const DiagnosticLogger& diagnostic_logger | ||
| ) { | ||
| return FeatureScope( | ||
| context_provider, | ||
| event_writer, | ||
| diagnostic_logger, | ||
| FeatureScope::ExecutionMode::Synchronous, | ||
| nullptr | ||
| ); | ||
| } | ||
|
|
||
| void FeatureScope::UpdateContext(const std::function<void(CoreContext&)>& callback) { | ||
| DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); | ||
| _context_provider->Update(callback); | ||
|
|
||
| if (_mode == FeatureScope::ExecutionMode::Synchronous) { | ||
| // Testing mode: execute synchronously on calling thread | ||
| _context_provider->Update(callback); | ||
| return; | ||
| } | ||
|
|
||
| // Production mode: queue for async execution on context thread | ||
| DATADOG_ASSERT( | ||
| _context_queue != nullptr, "context_queue is null in OnContextThread mode" | ||
| ); | ||
| _context_queue->Push([this, callback]() { | ||
| DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); | ||
| _context_provider->Update(callback); | ||
| }); | ||
|
Comment on lines
+74
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a legitimate error in the implementation. As originally written, calling a RUM API function would enqueue a single function to run on the context thread. That function would perform its command-processing work (updating internal state and producing RUM events), then enqueue another function that would mutate the That second function would be enqueued at the end of the queue, causing a legitimate bug. e.g. if you made these three API calls back-to-back:
...then the second log event would be missing the RUM View ID set for This is fixed in dac8a27: we now enqueue our two functions back-to-back, serially, in response to a RUM API call, such that in the example above, |
||
| } | ||
|
|
||
| bool FeatureScope::WriteEvent(Block event, Block event_metadata) const { | ||
| if (_event_generated_func) { | ||
| return _event_generated_func(event, event_metadata); | ||
| void FeatureScope::ExecuteOnContextThread(const ContextThreadFunc& func) { | ||
| if (_mode == FeatureScope::ExecutionMode::Synchronous) { | ||
| // Testing mode: execute synchronously on calling thread | ||
| DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); | ||
| const CoreContext context = _context_provider->Get(); | ||
| func(context, _event_generated_func); | ||
| return; | ||
| } | ||
| return false; | ||
|
|
||
| // Production mode: queue for async execution on context thread | ||
| DATADOG_ASSERT( | ||
| _context_queue != nullptr, "context_queue is null in OnContextThread mode" | ||
| ); | ||
| _context_queue->Push([this, func]() { | ||
| DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); | ||
| const CoreContext context = _context_provider->Get(); | ||
| func(context, _event_generated_func); | ||
|
Comment on lines
+93
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a fair point - fixed in 1dc05e9 by modifying the shutdown order such that the context queue is drained, then all features are stopped, then the storage and upload threads are stopped. As a consequence, it's no longer possible for a feature implementation to do last-minute work (or produce last-minute events) in response to Note that in its current form, the C++ SDK still drains both the context queue and storage queue on shutdown, making |
||
| }); | ||
| } | ||
|
|
||
| } // namespace datadog::impl | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stopping the context queue before invoking
OnCoreStopping()causes stop-time context mutations to be silently dropped, becauseFeatureScope::UpdateContext()enqueues viaPush()and does not handle afalsereturn once the queue is stopped. In this commit,Rum::Stop()still depends onUpdateContext()to clearctx.rum, so after a stop/start cycle stale RUM IDs can remain inCoreContextuntil a later RUM command runs, and early log events may be enriched with incorrect session/view/action IDs.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in fcbdf93 - rather than requiring each feature to reset its own subset of
CoreContextstate onStop()(which can no longer be done because Feature-inducedCoreContextmutations are now asynchronous, and no further async work can be done onStop()), we now just haveCore::Start()preemptively resetCoreContextto a clean starting value.