From 57b54d776fa210df36c64c406ef8a1bfa83e9337 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Thu, 5 Mar 2026 17:41:27 -0500 Subject: [PATCH 01/25] feat: Define an entry point for the context thread --- src/CMakeLists.txt | 1 + src/datadog/impl/core/context_thread.cpp | 27 ++++++++++++++++ src/datadog/impl/core/context_thread.hpp | 39 ++++++++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 src/datadog/impl/core/context_thread.cpp create mode 100644 src/datadog/impl/core/context_thread.hpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 74c55845..5334b790 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -24,6 +24,7 @@ add_library(dd_native ${DD_LIBRARY_TYPE} datadog/cpp/version.cpp datadog/impl/attribute/cow.cpp datadog/impl/attribute/merge.cpp + datadog/impl/core/context_thread.cpp datadog/impl/core/context.cpp datadog/impl/core/core.cpp datadog/impl/core/feature_read.cpp diff --git a/src/datadog/impl/core/context_thread.cpp b/src/datadog/impl/core/context_thread.cpp new file mode 100644 index 00000000..d8060b0f --- /dev/null +++ b/src/datadog/impl/core/context_thread.cpp @@ -0,0 +1,27 @@ +// 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>& queue, + CoreContextProvider& /* context_provider */ +) { + diagnostic_logger.Debug("Context thread starting"); + + // Process closures until the queue is stopped and drained (Pop() returns + // std::nullopt) + while (auto closure = queue.Pop()) { + (*closure)(); + } + + diagnostic_logger.Debug("Context thread finished"); +} + +} // namespace datadog::impl diff --git a/src/datadog/impl/core/context_thread.hpp b/src/datadog/impl/core/context_thread.hpp new file mode 100644 index 00000000..6a38318f --- /dev/null +++ b/src/datadog/impl/core/context_thread.hpp @@ -0,0 +1,39 @@ +// 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 + +#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 processes closures submitted by features, providing each + * closure with a snapshot of the CoreContext at execution time. This allows + * features to perform operations asynchronously without blocking their callers. + * + * The thread runs until the queue is stopped and drained, processing closures in + * FIFO order. + * + * @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. + * @param context_provider Non-owning reference to the CoreContextProvider that + * supplies context snapshots; guaranteed to outlive the thread. + */ +void ContextThreadMain( + const DiagnosticLogger& diagnostic_logger, + Queue>& queue, + CoreContextProvider& context_provider +); + +} // namespace datadog::impl From d74de5143afc20d2baac663ee783fd72efa31dcc Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Thu, 5 Mar 2026 17:49:30 -0500 Subject: [PATCH 02/25] feat: Update Core to own a context queue and run a context thread --- src/datadog/impl/core/core.cpp | 60 +++++++++++++++++++++++++++++++++- src/datadog/impl/core/core.hpp | 11 +++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/datadog/impl/core/core.cpp b/src/datadog/impl/core/core.cpp index 18932ed8..1fcbf5ed 100644 --- a/src/datadog/impl/core/core.cpp +++ b/src/datadog/impl/core/core.cpp @@ -7,11 +7,14 @@ #include "datadog/impl/core/core.hpp" #include +#include #include #include +#include #include #include "datadog/impl/assert.hpp" +#include "datadog/impl/core/context_thread.hpp" #include "datadog/impl/core/storage_thread.hpp" #include "datadog/impl/core/types.hpp" #include "datadog/impl/core/version.hpp" @@ -314,6 +317,22 @@ bool Core::Start() { std::ref(_features) ); + // Initialize a thread-safe queue for closures that will execute on the context + // thread + DATADOG_ASSERT(!_context_queue, "_context_queue already exists on Start()"); + _context_queue = std::make_unique>>(); + + // Start the context thread that will execute closures submitted by features + DATADOG_ASSERT(!_context_thread, "_context_thread already exists on Start()"); + _context_thread = std::thread( + ContextThreadMain, + std::ref(_diagnostic_logger), + std::ref(*_context_queue), + std::ref(*_context_provider) + ); + + // Initialize an upload scheduler to manage the timing of upload cycles for each + // feature DATADOG_ASSERT(!_upload_scheduler, "_upload_scheduler already exists on Start()"); _upload_scheduler = std::make_unique(*_subsystems.clock); @@ -378,7 +397,12 @@ void Core::Stop() { feature.impl->OnCoreStopping(); } - // If we were previously started, the storage and upload threads should be running + // If we were previously started, all background threads should be running + DATADOG_ASSERT(_context_queue, "_context_queue is invalid on Stop"); + DATADOG_ASSERT( + _context_thread && _context_thread->joinable(), + "_context_thread is non-joinable on Stop" + ); DATADOG_ASSERT(_storage_queue, "_storage_queue is invalid on Stop"); DATADOG_ASSERT( _storage_thread && _storage_thread->joinable(), @@ -390,6 +414,17 @@ void Core::Stop() { "_upload_thread is non-joinable on Stop" ); + // Stop the context queue, then block until the context thread drains the queue and + // exits. This ensures all pending feature operations complete before we stop the + // storage thread. + _context_queue->Stop(); + if (_context_thread) { + _diagnostic_logger.Debug("Joining on context thread"); + _context_thread->join(); + } + _context_thread.reset(); + _context_queue.reset(); + // Stop all queue processing, then block until the consumer thread drains the queue // and exits, at which point it's safe to release the queue _storage_queue->Stop(); @@ -492,4 +527,27 @@ std::string_view Core::GetApplicationVersion() const { return _context_provider->GetHttpContext().application_version; } +void Core::FlushContextQueue() { + DATADOG_ASSERT( + _state == CoreState::Started, "FlushContextQueue called while Core not running" + ); + DATADOG_ASSERT(_context_queue, "_context_queue is null on FlushContextQueue"); + + // Use a condition variable to wait until the sentinel closure executes + std::mutex mutex; + std::condition_variable cv; + bool sentinel_executed = false; + + // Queue a sentinel closure that signals completion + _context_queue->Push([&]() { + std::lock_guard lock(mutex); + sentinel_executed = true; + cv.notify_one(); + }); + + // Wait until the sentinel has been executed by the context thread + std::unique_lock lock(mutex); + cv.wait(lock, [&] { return sentinel_executed; }); +} + } // namespace datadog::impl diff --git a/src/datadog/impl/core/core.hpp b/src/datadog/impl/core/core.hpp index ac13b19f..81c9c7ff 100644 --- a/src/datadog/impl/core/core.hpp +++ b/src/datadog/impl/core/core.hpp @@ -365,6 +365,14 @@ class Core { */ void Stop(); + /** + * Blocks until all queued closures on the context thread have been processed. + * + * This is intended for testing, to ensure deterministic execution of asynchronous + * operations. Must only be called when the core is running. + */ + void FlushContextQueue(); + private: bool EnqueueStorageWrite(FeatureId feature_id, Block event, Block event_metadata); @@ -386,6 +394,9 @@ class Core { std::unique_ptr _storage_queue; std::optional _storage_thread; + std::unique_ptr>> _context_queue; + std::optional _context_thread; + std::unique_ptr _upload_scheduler; std::optional _upload_thread; From 511f69e533176376ee5d0e0b15af3d3a3a0067aa Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Thu, 5 Mar 2026 18:09:09 -0500 Subject: [PATCH 03/25] feat: Update FeatureScope to allow deferred execution on context thread --- src/datadog/impl/core/context_thread.cpp | 6 ++-- src/datadog/impl/core/context_thread.hpp | 6 ++-- src/datadog/impl/core/core.cpp | 17 ++++++----- src/datadog/impl/core/core.hpp | 2 +- src/datadog/impl/core/feature_scope.cpp | 23 ++++++++++++++- src/datadog/impl/core/feature_scope.hpp | 36 +++++++++++++++++++++++- 6 files changed, 74 insertions(+), 16 deletions(-) diff --git a/src/datadog/impl/core/context_thread.cpp b/src/datadog/impl/core/context_thread.cpp index d8060b0f..89aaa169 100644 --- a/src/datadog/impl/core/context_thread.cpp +++ b/src/datadog/impl/core/context_thread.cpp @@ -15,10 +15,10 @@ void ContextThreadMain( ) { diagnostic_logger.Debug("Context thread starting"); - // Process closures until the queue is stopped and drained (Pop() returns + // Process functions until the queue is stopped and drained (Pop() returns // std::nullopt) - while (auto closure = queue.Pop()) { - (*closure)(); + while (auto thunk = queue.Pop()) { + (*thunk)(); } diagnostic_logger.Debug("Context thread finished"); diff --git a/src/datadog/impl/core/context_thread.hpp b/src/datadog/impl/core/context_thread.hpp index 6a38318f..c5a44835 100644 --- a/src/datadog/impl/core/context_thread.hpp +++ b/src/datadog/impl/core/context_thread.hpp @@ -17,11 +17,11 @@ namespace datadog::impl { /** * Entry point for the context thread. * - * The context thread processes closures submitted by features, providing each - * closure with a snapshot of the CoreContext at execution time. This allows + * The context thread processes functions submitted by features, providing each + * function with a snapshot of the CoreContext at execution time. This allows * features to perform operations asynchronously without blocking their callers. * - * The thread runs until the queue is stopped and drained, processing closures in + * The thread runs until the queue is stopped and drained, processing functions in * FIFO order. * * @param diagnostic_logger Interface for logging status/warning messages. diff --git a/src/datadog/impl/core/core.cpp b/src/datadog/impl/core/core.cpp index 1fcbf5ed..86876898 100644 --- a/src/datadog/impl/core/core.cpp +++ b/src/datadog/impl/core/core.cpp @@ -317,12 +317,12 @@ bool Core::Start() { std::ref(_features) ); - // Initialize a thread-safe queue for closures that will execute on the context + // Initialize a thread-safe queue for functions that will execute on the context // thread DATADOG_ASSERT(!_context_queue, "_context_queue already exists on Start()"); _context_queue = std::make_unique>>(); - // Start the context thread that will execute closures submitted by features + // Start the context thread that will execute functions submitted by features DATADOG_ASSERT(!_context_thread, "_context_thread already exists on Start()"); _context_thread = std::thread( ContextThreadMain, @@ -378,9 +378,12 @@ bool Core::Start() { [this, id](Block event, Block event_metadata) -> bool { return EnqueueStorageWrite(id, event, event_metadata); }; - feature.impl->OnCoreStarted( - FeatureScope(*_context_provider, event_generated_func, _diagnostic_logger) - ); + feature.impl->OnCoreStarted(FeatureScope( + *_context_provider, + event_generated_func, + _diagnostic_logger, + _context_queue.get() + )); } return true; } @@ -533,12 +536,12 @@ void Core::FlushContextQueue() { ); DATADOG_ASSERT(_context_queue, "_context_queue is null on FlushContextQueue"); - // Use a condition variable to wait until the sentinel closure executes + // Use a condition variable to wait until the sentinel function executes std::mutex mutex; std::condition_variable cv; bool sentinel_executed = false; - // Queue a sentinel closure that signals completion + // Queue a sentinel function that signals completion _context_queue->Push([&]() { std::lock_guard lock(mutex); sentinel_executed = true; diff --git a/src/datadog/impl/core/core.hpp b/src/datadog/impl/core/core.hpp index 81c9c7ff..3535275d 100644 --- a/src/datadog/impl/core/core.hpp +++ b/src/datadog/impl/core/core.hpp @@ -366,7 +366,7 @@ class Core { void Stop(); /** - * Blocks until all queued closures on the context thread have been processed. + * Blocks until all queued functions on the context thread have been processed. * * This is intended for testing, to ensure deterministic execution of asynchronous * operations. Must only be called when the core is running. diff --git a/src/datadog/impl/core/feature_scope.cpp b/src/datadog/impl/core/feature_scope.cpp index b5bde9a3..ca64399e 100644 --- a/src/datadog/impl/core/feature_scope.cpp +++ b/src/datadog/impl/core/feature_scope.cpp @@ -13,10 +13,12 @@ namespace datadog::impl { FeatureScope::FeatureScope( CoreContextProvider& context_provider, const EventGeneratedFunc& event_generated_func, - const DiagnosticLogger& in_diagnostic_logger + const DiagnosticLogger& in_diagnostic_logger, + Queue>* context_queue ) : _context_provider(&context_provider), _event_generated_func(event_generated_func), + _context_queue(context_queue), diagnostic_logger(in_diagnostic_logger) {} CoreContext FeatureScope::GetContext() const { @@ -36,4 +38,23 @@ bool FeatureScope::WriteEvent(Block event, Block event_metadata) const { return false; } +void FeatureScope::ExecuteOnContextThread(const ContextThreadFunc& func) { + // If no context queue is available (testing mode), execute synchronously + if (_context_queue == nullptr) { + DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); + const CoreContext context = _context_provider->Get(); + func(context, _event_generated_func); + return; + } + + // Queue a thunk that will execute on the context thread + _context_queue->Push([this, func]() { + DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); + // Get a CoreContext snapshot at execution time + const CoreContext context = _context_provider->Get(); + // Invoke the user's function with the context and event writer + func(context, _event_generated_func); + }); +} + } // namespace datadog::impl diff --git a/src/datadog/impl/core/feature_scope.hpp b/src/datadog/impl/core/feature_scope.hpp index 2b0722e8..9b1da3f1 100644 --- a/src/datadog/impl/core/feature_scope.hpp +++ b/src/datadog/impl/core/feature_scope.hpp @@ -10,6 +10,7 @@ #include "datadog/impl/core/block.hpp" #include "datadog/impl/core/context.hpp" +#include "datadog/impl/core/queue.hpp" #include "datadog/impl/diagnostics.hpp" namespace datadog::impl { @@ -27,6 +28,20 @@ namespace datadog::impl { */ using EventGeneratedFunc = std::function; +/** + * Callback that writes an event to storage from the context thread. + * + * Returns whether the event was successfully enqueued. + */ +using EventWriter = std::function; + +/** + * Function executed on the context thread with access to a CoreContext snapshot and an + * EventWriter for generating events. + */ +using ContextThreadFunc = + std::function; + /** * Interface provided to each feature in order to give that feature access to core SDK * functionality. @@ -35,6 +50,7 @@ class FeatureScope { private: CoreContextProvider* _context_provider; EventGeneratedFunc _event_generated_func; + Queue>* _context_queue; public: DiagnosticLogger diagnostic_logger; @@ -42,11 +58,16 @@ class FeatureScope { /** * Initializes a FeatureScope for a single feature, given the state necessary to * connect it to the Core. + * + * @param context_queue Optional pointer to the context queue. If null, operations + * will execute synchronously (used in tests). If non-null, operations will be + * queued for asynchronous execution on the context thread. */ explicit FeatureScope( CoreContextProvider& context_provider, const EventGeneratedFunc& event_generated_func, - const DiagnosticLogger& in_diagnostic_logger + const DiagnosticLogger& in_diagnostic_logger, + Queue>* context_queue = nullptr ); // Noncopyable, but movable (so ownership can be transferred to Feature from Core) @@ -101,6 +122,19 @@ class FeatureScope { * CoreContext, it may be worthwhile to use async dispatch (see above). */ bool WriteEvent(Block event, Block event_metadata) const; + + /** + * Executes a function on the context thread, providing it with a CoreContext snapshot + * and an EventWriter for generating events. + * + * If no context queue is configured (testing mode), executes the function + * synchronously on the calling thread. + * + * The function receives: + * - A const reference to a CoreContext snapshot (taken at execution time) + * - An EventWriter callback that can be used to generate events + */ + void ExecuteOnContextThread(const ContextThreadFunc& func); }; } // namespace datadog::impl From c798ea3caf8a4d12f3c0adcb69fa03ca4d813605 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Fri, 6 Mar 2026 13:14:58 -0500 Subject: [PATCH 04/25] feat: Update RUM implementation to use context thread --- .../features/rum/containers/resource_map.cpp | 7 +- .../features/rum/containers/resource_map.hpp | 5 +- .../features/rum/containers/view_array.cpp | 6 +- .../features/rum/containers/view_array.hpp | 4 +- src/datadog/impl/features/rum/rum.cpp | 90 +++--- src/datadog/impl/features/rum/rum.hpp | 2 +- src/datadog/impl/features/rum/scope.cpp | 13 - src/datadog/impl/features/rum/scope.hpp | 39 +-- .../impl/features/rum/scopes/action.cpp | 16 +- .../impl/features/rum/scopes/action.hpp | 13 +- .../impl/features/rum/scopes/application.cpp | 10 +- .../impl/features/rum/scopes/application.hpp | 6 +- .../features/rum/scopes/event_enrichment.hpp | 18 +- .../impl/features/rum/scopes/resource.cpp | 27 +- .../impl/features/rum/scopes/resource.hpp | 15 +- .../impl/features/rum/scopes/session.cpp | 43 ++- .../impl/features/rum/scopes/session.hpp | 24 +- src/datadog/impl/features/rum/scopes/view.cpp | 74 +++-- src/datadog/impl/features/rum/scopes/view.hpp | 38 ++- .../rum/containers/resource_map_test.cpp | 16 +- .../rum/containers/view_array_test.cpp | 29 +- .../impl/features/rum/scopes/action_test.cpp | 87 ++++-- .../features/rum/scopes/application_test.cpp | 163 ++++++++--- .../features/rum/scopes/resource_test.cpp | 15 +- .../impl/features/rum/scopes/session_test.cpp | 275 +++++++++++++----- tests/impl/features/rum/scopes/view_test.cpp | 90 ++++-- 26 files changed, 798 insertions(+), 327 deletions(-) diff --git a/src/datadog/impl/features/rum/containers/resource_map.cpp b/src/datadog/impl/features/rum/containers/resource_map.cpp index d55b315f..6b40cc4b 100644 --- a/src/datadog/impl/features/rum/containers/resource_map.cpp +++ b/src/datadog/impl/features/rum/containers/resource_map.cpp @@ -42,7 +42,10 @@ RumResourceScope& RumResourceMap::Add( } RumResourceScope::Result RumResourceMap::Forward( - const std::string& resource_key, const RumCommand& command + const std::string& resource_key, + const RumCommand& command, + const CoreContext& context, + const EventWriter& writer ) { // Prepare a result value, defaulting to 'no event' RumResourceScope::Result scope_result = RumResourceScope::Result::SentNoEvent; @@ -58,7 +61,7 @@ RumResourceScope::Result RumResourceMap::Forward( // We have a matching resource: propagate the command to that scope (and that scope // only), removing it from the map if it should be closed as a result - const RumScopeResult result = found->second.Process(command); + const RumScopeResult result = found->second.Process(command, context, writer); if (result == RumScopeResult::Close) { // If the scope is now closed, record its result (i.e. what kind of event it sent) // before destroying it diff --git a/src/datadog/impl/features/rum/containers/resource_map.hpp b/src/datadog/impl/features/rum/containers/resource_map.hpp index 9baf368b..836f2454 100644 --- a/src/datadog/impl/features/rum/containers/resource_map.hpp +++ b/src/datadog/impl/features/rum/containers/resource_map.hpp @@ -59,7 +59,10 @@ struct RumResourceMap { * resource event, and error event, or no event. */ RumResourceScope::Result Forward( - const std::string& resource_key, const RumCommand& command + const std::string& resource_key, + const RumCommand& command, + const CoreContext& context, + const EventWriter& writer ); /** diff --git a/src/datadog/impl/features/rum/containers/view_array.cpp b/src/datadog/impl/features/rum/containers/view_array.cpp index 2d9a7198..fca863da 100644 --- a/src/datadog/impl/features/rum/containers/view_array.cpp +++ b/src/datadog/impl/features/rum/containers/view_array.cpp @@ -80,7 +80,9 @@ RumViewScope& RumViewArray::Push( ); } -void RumViewArray::Propagate(const RumCommand& command) { +void RumViewArray::Propagate( + const RumCommand& command, const CoreContext& context, const EventWriter& writer +) { // Iterate over all view scopes in the order in which they're stored, without respect // to creation time etc. for (auto& item : items) { @@ -88,7 +90,7 @@ void RumViewArray::Propagate(const RumCommand& command) { if (item) { // Process the command, and clear the slot (destroying the RumViewScope) if the // scope should be closed as a result - RumScopeResult result = item->Process(command); + RumScopeResult result = item->Process(command, context, writer); if (result == RumScopeResult::Close) { item.reset(); } diff --git a/src/datadog/impl/features/rum/containers/view_array.hpp b/src/datadog/impl/features/rum/containers/view_array.hpp index 83b595ea..f7c4530c 100644 --- a/src/datadog/impl/features/rum/containers/view_array.hpp +++ b/src/datadog/impl/features/rum/containers/view_array.hpp @@ -60,7 +60,9 @@ struct RumViewArray { * Iteration order is not defined: no assumptions should be made about the order in * which sibling view scopes will process the command. */ - void Propagate(const RumCommand& command); + void Propagate( + const RumCommand& command, const CoreContext& context, const EventWriter& writer + ); /** * Returns a reference to the first active view scope found in the array, or diff --git a/src/datadog/impl/features/rum/rum.cpp b/src/datadog/impl/features/rum/rum.cpp index d795fbe5..39e8305f 100644 --- a/src/datadog/impl/features/rum/rum.cpp +++ b/src/datadog/impl/features/rum/rum.cpp @@ -50,17 +50,13 @@ std::optional Rum::UploadThread_PrepareReport( } void Rum::Start() { - // Inject a reference to our FeatureScope interface into the RumScopeDependencies that - // will be provided to all scopes, so they can generate events etc. - if (_scope) { - _deps.OnStart(*_scope); - } else { - DATADOG_ASSERT(false, "Rum has no valid FeatureScope on Start"); - } + // Fully reinitialize RUM application state to clear all sessions/views/etc. from + // previous runs + // Fully reinitialize RUM application state + _application = RumApplicationScope(_deps); - // Dispatch an SDKInit command, which should kick off initialization of our first - // session when handled by the application scope - Dispatch(RumCommand::SDKInit(GetBaseCommandParams())); + // Dispatch SDKInit to start first session + DispatchAsync(RumCommand::SDKInit(GetBaseCommandParams())); }; void Rum::Stop() { @@ -70,12 +66,9 @@ void Rum::Stop() { _scope->UpdateContext([](CoreContext& ctx) { ctx.rum.reset(); }); } - // Fully reinitialize RUM application state to clear all sessions/views/etc. - _application = RumApplicationScope(_deps); - - // Clear the FeatureScope reference in RumScopeDependencies: scopes should no longer - // generate events - _deps.OnStop(); + // Note: _application will be destroyed when Rum is destroyed (after Core joins + // the context thread). In-flight lambdas can safely access _application as long as + // weak_ptr.lock() succeeds. } void Rum::AddAttribute(std::string_view name, const Attribute& value) { @@ -90,50 +83,52 @@ void Rum::RemoveAttribute(std::string_view name) { void Rum::StopSession() { // Dispatch a StopSession command, which the session scope should handle - Dispatch(RumCommand::StopSession(GetBaseCommandParams())); + DispatchAsync(RumCommand::StopSession(GetBaseCommandParams())); } void Rum::StartView( std::string_view key, std::string_view name, const Attribute& attributes ) { // Dispatch a StartView command to be handled by the active session - Dispatch(RumCommand::StartView(GetBaseCommandParams(attributes), key, name)); + DispatchAsync(RumCommand::StartView(GetBaseCommandParams(attributes), key, name)); } void Rum::AddViewAttribute(std::string_view name, const Attribute& value) { // TODO(RUM-11363): Log a warning if there's no active view to receive the command? - Dispatch(RumCommand::AddViewAttribute(GetBaseCommandParams(), name, value)); + DispatchAsync(RumCommand::AddViewAttribute(GetBaseCommandParams(), name, value)); } void Rum::RemoveViewAttribute(std::string_view name) { - Dispatch(RumCommand::RemoveViewAttribute(GetBaseCommandParams(), name)); + DispatchAsync(RumCommand::RemoveViewAttribute(GetBaseCommandParams(), name)); } void Rum::StopView(std::string_view key, const Attribute& attributes) { // Dispatch a StopView command - Dispatch(RumCommand::StopView(GetBaseCommandParams(attributes), key)); + DispatchAsync(RumCommand::StopView(GetBaseCommandParams(attributes), key)); } void Rum::AddAction( RumActionType type, std::string_view name, const Attribute& attributes ) { - Dispatch(RumCommand::AddAction(GetBaseCommandParams(attributes), type, name)); + DispatchAsync(RumCommand::AddAction(GetBaseCommandParams(attributes), type, name)); } void Rum::StartAction( RumActionType type, std::string_view name, const Attribute& attributes ) { - Dispatch(RumCommand::StartAction(GetBaseCommandParams(attributes), type, name)); + DispatchAsync(RumCommand::StartAction(GetBaseCommandParams(attributes), type, name)); } void Rum::StopAction(std::string_view new_name, const Attribute& attributes) { - Dispatch(RumCommand::StopAction(GetBaseCommandParams(attributes), new_name)); + DispatchAsync(RumCommand::StopAction(GetBaseCommandParams(attributes), new_name)); } void Rum::StartResource( std::string_view key, const RumRequestDetails& request, const Attribute& attributes ) { - Dispatch(RumCommand::StartResource(GetBaseCommandParams(attributes), key, request)); + DispatchAsync( + RumCommand::StartResource(GetBaseCommandParams(attributes), key, request) + ); } void Rum::StopResource( @@ -142,7 +137,7 @@ void Rum::StopResource( const std::optional& error, const Attribute& attributes ) { - Dispatch( + DispatchAsync( RumCommand::StopResource(GetBaseCommandParams(attributes), key, response, error) ); } @@ -150,7 +145,7 @@ void Rum::StopResource( void Rum::AddError( RumErrorSource source, const RumErrorDetails& error, const Attribute& attributes ) { - Dispatch(RumCommand::AddError(GetBaseCommandParams(attributes), source, error)); + DispatchAsync(RumCommand::AddError(GetBaseCommandParams(attributes), source, error)); } void Rum::StartFeatureOperation( @@ -158,7 +153,7 @@ void Rum::StartFeatureOperation( std::optional operation_key, const Attribute& attributes ) { - Dispatch( + DispatchAsync( RumCommand::StartFeatureOperation( GetBaseCommandParams(attributes), name, operation_key ) @@ -171,7 +166,7 @@ void Rum::StopFeatureOperation( std::optional failure_reason, const Attribute& attributes ) { - Dispatch( + DispatchAsync( RumCommand::StopFeatureOperation( GetBaseCommandParams(attributes), name, operation_key, failure_reason ) @@ -190,20 +185,37 @@ RumCommandParams Rum::GetBaseCommandParams(const Attribute& attributes) const { return RumCommandParams(issued_at, global_attributes, attributes); } -void Rum::Dispatch(const RumCommand& command) { - // Refrain from dispatching any commands if we don't have a valid FeatureScope: this - // means that the SDK has not yet started or has previously shut down +void Rum::DispatchAsync(const RumCommand& command) { if (!_scope) { return; } - // Let the root RumApplicationScope handle the command: each scope will propagate - // commands to child scope(s) at their discretion - _application.Process(command); - - // After every command, update our RumFeatureContext, which makes current RUM state - // available to other features within the SDK - UpdateFeatureContext(); + // Store reference to scope to satisfy linter's optional access check + FeatureScope& scope = *_scope; + + // Capture weak_ptr to self for fail-safe shutdown detection. Matches iOS SDK + // pattern where closures capture weak references and fail gracefully when + // features are deallocated during shutdown. + auto weak_rum = std::weak_ptr(std::static_pointer_cast(shared_from_this())); + + scope.ExecuteOnContextThread( + [weak_rum, cmd = command](const CoreContext& context, const EventWriter& writer) { + // Single-level check: Is Rum object still alive? + auto rum = weak_rum.lock(); + if (!rum) { + // Rum destroyed during shutdown, exit gracefully + return; + } + + // Safe to proceed - processing uses only deps, context, writer, and + // _application (all valid as long as Rum is alive) + rum->_application.Process(cmd, context, writer); + + // After every command, update our RumFeatureContext, which makes current RUM + // state available to other features within the SDK + rum->UpdateFeatureContext(); + } + ); } void Rum::UpdateFeatureContext() { diff --git a/src/datadog/impl/features/rum/rum.hpp b/src/datadog/impl/features/rum/rum.hpp index 4d84afc0..aa636eba 100644 --- a/src/datadog/impl/features/rum/rum.hpp +++ b/src/datadog/impl/features/rum/rum.hpp @@ -159,7 +159,7 @@ class Rum final : public Feature { const Attribute& attributes = Attribute() ) const; - void Dispatch(const RumCommand& command); + void DispatchAsync(const RumCommand& command); void UpdateFeatureContext(); void UpdateApplicationSnapshot(); diff --git a/src/datadog/impl/features/rum/scope.cpp b/src/datadog/impl/features/rum/scope.cpp index ea14c7b9..4604ced5 100644 --- a/src/datadog/impl/features/rum/scope.cpp +++ b/src/datadog/impl/features/rum/scope.cpp @@ -19,25 +19,12 @@ RumScopeDependencies::RumScopeDependencies( ) : application_id(config.application_id), clock(in_clock), - scope(nullptr), _sampling_rate_unit(config.session_sample_rate / 100.0f), _sampling_rng(std::random_device{}()), _sampling_distribution(0.0f, 1.0f) { _encode_buffer.reserve(8192); } -void RumScopeDependencies::OnStart(FeatureScope& in_scope) { - DATADOG_ASSERT(!scope, "RUM deps has valid scope pointer on SDK start"); - diagnostic_logger = in_scope.diagnostic_logger; - scope = &in_scope; -} - -void RumScopeDependencies::OnStop() { - DATADOG_ASSERT(scope, "RUM deps has no valid scope pointer on SDK start"); - diagnostic_logger = DiagnosticLogger{}; - scope = nullptr; -} - bool RumScopeDependencies::ShouldSampleSession() const { std::unique_lock write_lock(mutex); diff --git a/src/datadog/impl/features/rum/scope.hpp b/src/datadog/impl/features/rum/scope.hpp index 76dd1128..9c348a7b 100644 --- a/src/datadog/impl/features/rum/scope.hpp +++ b/src/datadog/impl/features/rum/scope.hpp @@ -37,12 +37,6 @@ struct RumScopeDependencies { DiagnosticLogger diagnostic_logger; const platform::IClock& clock; - // Reference to the Feature's interface to the core, used for accessing context, - // generating events, etc. FeatureScopes aren't created until Core::Start(), and they - // only last until Core::Stop(), so this value is only guaranteed to be non-NULL while - // the SDK is running. - FeatureScope* scope; - private: // Synchronization for internal state mutable std::shared_mutex mutex; @@ -60,16 +54,6 @@ struct RumScopeDependencies { const RumConfig& config, const platform::IClock& in_clock ); - /** - * Injects required dependencies that are only initialized upon SDK start. - */ - void OnStart(FeatureScope& in_scope); - - /** - * Clears any dependencies that were injected on SDK start. - */ - void OnStop(); - public: /** * Makes a single sampling decision for a newly-created session. If the result is @@ -79,16 +63,21 @@ struct RumScopeDependencies { */ bool ShouldSampleSession() const; + /** + * Encodes a RUM event to JSON without writing it. The caller uses the returned + * string_view with the EventWriter callback to actually write the event. + * + * @param event RUM event to encode + * @return JSON-encoded event as a string_view referencing the internal encode buffer. + * Valid until the next call to EncodeEvent(). + */ template - void ProduceEvent(const T& event) const { - if (scope) { - std::unique_lock write_lock(mutex); - EncodeJson(_encode_buffer, event); - std::string_view data( - reinterpret_cast(_encode_buffer.data()), _encode_buffer.size() - ); - scope->WriteEvent(data, {}); - } + std::string_view EncodeEvent(const T& event) const { + std::unique_lock write_lock(mutex); + EncodeJson(_encode_buffer, event); + return std::string_view( + reinterpret_cast(_encode_buffer.data()), _encode_buffer.size() + ); } }; diff --git a/src/datadog/impl/features/rum/scopes/action.cpp b/src/datadog/impl/features/rum/scopes/action.cpp index d7354b8e..b8d6bc67 100644 --- a/src/datadog/impl/features/rum/scopes/action.cpp +++ b/src/datadog/impl/features/rum/scopes/action.cpp @@ -44,7 +44,9 @@ void RumActionScope::PopulateContext(struct RumContext& out_context) const { out_context.active_action_id = _action_id; } -RumScopeResult RumActionScope::Process(const RumCommand& command) { +RumScopeResult RumActionScope::Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer +) { // Determine whether our expiration time has passed: even if it has, we still remain // active until the last active resource that started under our watch has ended const bool has_expired = command.base.issued_at >= _expires_at; @@ -94,7 +96,7 @@ RumScopeResult RumActionScope::Process(const RumCommand& command) { // completion to that time: this will be reflected in the event payload as a // duration in nanoseconds called `loading_time` const Timestamp completed_at = has_expired ? _expires_at : command.base.issued_at; - SendActionEvent(command, completed_at); + SendActionEvent(command, completed_at, context, writer); return RumScopeResult::Close; } @@ -115,7 +117,10 @@ RumScopeResult RumActionScope::Process(const RumCommand& command) { } void RumActionScope::SendActionEvent( - const RumCommand& command, const Timestamp& completed_at + const RumCommand& command, + const Timestamp& completed_at, + const CoreContext& context_param, + const EventWriter& writer ) { // Resolve references needed to populate required event data const RumScopeDependencies& deps = _deps; @@ -204,9 +209,10 @@ void RumActionScope::SendActionEvent( } // Enrich event with OS and device properties from CoreContext - RumEventEnrichment::PopulateCommonProperties(deps.scope, ev); + RumEventEnrichment::PopulateCommonProperties(context_param, ev); - deps.ProduceEvent(ev); + std::string_view json = deps.EncodeEvent(ev); + writer(Block{json.data(), json.size()}, Block{}); _has_sent_action_event = true; } diff --git a/src/datadog/impl/features/rum/scopes/action.hpp b/src/datadog/impl/features/rum/scopes/action.hpp index b7b4d880..91812b59 100644 --- a/src/datadog/impl/features/rum/scopes/action.hpp +++ b/src/datadog/impl/features/rum/scopes/action.hpp @@ -6,6 +6,8 @@ #pragma once +#include "datadog/impl/core/context.hpp" +#include "datadog/impl/core/feature_scope.hpp" #include "datadog/impl/features/rum/scope.hpp" namespace datadog::impl { @@ -67,13 +69,20 @@ class RumActionScope { void PopulateContext(struct RumContext& out_context) const; // RumScope interface - RumScopeResult Process(const RumCommand& command); + RumScopeResult Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer + ); private: /** * Generates and sends a RUM action event in response to the given command. */ - void SendActionEvent(const RumCommand& command, const Timestamp& completed_at); + void SendActionEvent( + const RumCommand& command, + const Timestamp& completed_at, + const CoreContext& context, + const EventWriter& writer + ); private: std::reference_wrapper _deps; diff --git a/src/datadog/impl/features/rum/scopes/application.cpp b/src/datadog/impl/features/rum/scopes/application.cpp index b77f351a..0e03bac9 100644 --- a/src/datadog/impl/features/rum/scopes/application.cpp +++ b/src/datadog/impl/features/rum/scopes/application.cpp @@ -25,7 +25,9 @@ void RumApplicationScope::PopulateContext(RumContext& out_context) const { out_context.application_id = deps.application_id; } -RumScopeResult RumApplicationScope::Process(const RumCommand& command) { +RumScopeResult RumApplicationScope::Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer +) { // On SDK init, create an initial session if (command.Is()) { // Open a brand new session scope @@ -66,7 +68,8 @@ RumScopeResult RumApplicationScope::Process(const RumCommand& command) { if (_active_session) { // Allow the session to process the command, and potentially propagate it to child // views etc. - const RumScopeResult session_result = _active_session->Process(command); + const RumScopeResult session_result = + _active_session->Process(command, context, writer); // If the session scope was closed in response to the command, update our state and // refresh the session if necessary @@ -92,7 +95,8 @@ RumScopeResult RumApplicationScope::Process(const RumCommand& command) { // If we've ended up with a new session, propagate the original command to it if (_active_session) { - const RumScopeResult result = _active_session->Process(command); + const RumScopeResult result = + _active_session->Process(command, context, writer); // If we've just created a new session to handle a command, that command should // _not_ close the session: if it does, drop the session as if it never existed diff --git a/src/datadog/impl/features/rum/scopes/application.hpp b/src/datadog/impl/features/rum/scopes/application.hpp index fcc3595d..11b55514 100644 --- a/src/datadog/impl/features/rum/scopes/application.hpp +++ b/src/datadog/impl/features/rum/scopes/application.hpp @@ -10,6 +10,8 @@ #include #include +#include "datadog/impl/core/context.hpp" +#include "datadog/impl/core/feature_scope.hpp" #include "datadog/impl/features/rum/scope.hpp" #include "datadog/impl/features/rum/scopes/session.hpp" @@ -108,7 +110,9 @@ class RumApplicationScope { void PopulateContext(struct RumContext& out_context) const; // RumScope interface - RumScopeResult Process(const RumCommand& command); + RumScopeResult Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer + ); private: /** diff --git a/src/datadog/impl/features/rum/scopes/event_enrichment.hpp b/src/datadog/impl/features/rum/scopes/event_enrichment.hpp index a4e603b7..81c8980f 100644 --- a/src/datadog/impl/features/rum/scopes/event_enrichment.hpp +++ b/src/datadog/impl/features/rum/scopes/event_enrichment.hpp @@ -22,24 +22,16 @@ struct RumEventEnrichment { * Populates a RUM event's `os` and `device` fields with properties retrieved from * CoreContext. * - * @param scope FeatureScope from which to access CoreContext. If null or if - * CoreContext lacks OS/device info, corresponding event fields remain unchanged. + * @param context CoreContext containing OS and device info. If CoreContext lacks + * OS/device info, corresponding event fields remain unchanged. * @param ev RUM event with `OmitIfNoValue os` and * `OmitIfNoValue device` fields, to be modified in-place. */ template - static void PopulateCommonProperties(const FeatureScope* scope, T& ev) { - // Abort if no scope (SDK is not active) - if (!scope) { - return; - } - - // Obtain an immutable, thread-safe copy of the SDK's core context - const CoreContext ctx = scope->GetContext(); - + static void PopulateCommonProperties(const CoreContext& context, T& ev) { // If SystemInfo details are available, populate 'os' and 'device' properties - PopulateOsProperties(ctx, ev); - PopulateDeviceProperties(ctx, ev); + PopulateOsProperties(context, ev); + PopulateDeviceProperties(context, ev); } private: diff --git a/src/datadog/impl/features/rum/scopes/resource.cpp b/src/datadog/impl/features/rum/scopes/resource.cpp index 53506cca..4952f9e6 100644 --- a/src/datadog/impl/features/rum/scopes/resource.cpp +++ b/src/datadog/impl/features/rum/scopes/resource.cpp @@ -42,7 +42,9 @@ RumResourceScope::RumResourceScope( } } -RumScopeResult RumResourceScope::Process(const RumCommand& command) { +RumScopeResult RumResourceScope::Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer +) { // Note that RumViewScope generally only propagates commands that explicitly target a // specific resource, and it will propagate such a command only to the scope // identified by `key` in the command. Do not expect RumResourceScope to receive any @@ -64,13 +66,13 @@ RumScopeResult RumResourceScope::Process(const RumCommand& command) { // 'error' event whose 'error.resource' properties describe this request, then close // the scope (wihthout sending a 'resource' event) if (payload.error) { - SendErrorEvent(command.base, payload, *payload.error); + SendErrorEvent(command.base, payload, *payload.error, context, writer); return RumScopeResult::Close; } // Otherwise, StopResource was called, indicating that the application got a valid // response: send a 'resource' event and close the scope - SendResourceEvent(command.base, payload); + SendResourceEvent(command.base, payload, context, writer); return RumScopeResult::Close; } @@ -80,7 +82,10 @@ RumScopeResult RumResourceScope::Process(const RumCommand& command) { } void RumResourceScope::SendResourceEvent( - const RumCommandParams& base, const RumStopResourcePayload& payload + const RumCommandParams& base, + const RumStopResourcePayload& payload, + const CoreContext& context_param, + const EventWriter& writer ) { // A resource scope should only ever send a single event DATADOG_ASSERT( @@ -153,16 +158,19 @@ void RumResourceScope::SendResourceEvent( } // Enrich event with OS and device properties from CoreContext - RumEventEnrichment::PopulateCommonProperties(deps.scope, ev); + RumEventEnrichment::PopulateCommonProperties(context_param, ev); - deps.ProduceEvent(ev); + std::string_view json = deps.EncodeEvent(ev); + writer(Block{json.data(), json.size()}, Block{}); _result = Result::SentResourceEvent; } void RumResourceScope::SendErrorEvent( const RumCommandParams& base, const RumStopResourcePayload& payload, - const RumErrorDetails& error + const RumErrorDetails& error, + const CoreContext& context_param, + const EventWriter& writer ) { // A resource scope should only ever send a single event DATADOG_ASSERT( @@ -224,9 +232,10 @@ void RumResourceScope::SendErrorEvent( } // Enrich event with OS and device properties from CoreContext - RumEventEnrichment::PopulateCommonProperties(deps.scope, ev); + RumEventEnrichment::PopulateCommonProperties(context_param, ev); - deps.ProduceEvent(ev); + std::string_view json = deps.EncodeEvent(ev); + writer(Block{json.data(), json.size()}, Block{}); _result = Result::SentErrorEvent; } diff --git a/src/datadog/impl/features/rum/scopes/resource.hpp b/src/datadog/impl/features/rum/scopes/resource.hpp index fa31763c..fda57bc4 100644 --- a/src/datadog/impl/features/rum/scopes/resource.hpp +++ b/src/datadog/impl/features/rum/scopes/resource.hpp @@ -8,6 +8,8 @@ #include +#include "datadog/impl/core/context.hpp" +#include "datadog/impl/core/feature_scope.hpp" #include "datadog/impl/features/rum/scope.hpp" namespace datadog::impl { @@ -65,7 +67,9 @@ class RumResourceScope { RumResourceScope& operator=(RumResourceScope&&) = default; // RumScope interface - RumScopeResult Process(const RumCommand& command); + RumScopeResult Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer + ); private: /** @@ -73,7 +77,10 @@ class RumResourceScope { * of a resource (i.e. a StopResource call). */ void SendResourceEvent( - const RumCommandParams& base, const RumStopResourcePayload& payload + const RumCommandParams& base, + const RumStopResourcePayload& payload, + const CoreContext& context, + const EventWriter& writer ); /** @@ -84,7 +91,9 @@ class RumResourceScope { void SendErrorEvent( const RumCommandParams& base, const RumStopResourcePayload& payload, - const RumErrorDetails& error + const RumErrorDetails& error, + const CoreContext& context, + const EventWriter& writer ); static UUID ResolveActiveActionId(const RumViewScope& parent); diff --git a/src/datadog/impl/features/rum/scopes/session.cpp b/src/datadog/impl/features/rum/scopes/session.cpp index b2d08540..f49a5c24 100644 --- a/src/datadog/impl/features/rum/scopes/session.cpp +++ b/src/datadog/impl/features/rum/scopes/session.cpp @@ -52,7 +52,9 @@ void RumSessionScope::PopulateContext(RumContext& out_context) const { out_context.session_precondition = _precondition; } -RumScopeResult RumSessionScope::Process(const RumCommand& command) { +RumScopeResult RumSessionScope::Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer +) { // -- Determine if the session needs to end, and early-out if so // If the session already ended in response to a prior command, we should not be @@ -71,7 +73,7 @@ RumScopeResult RumSessionScope::Process(const RumCommand& command) { // Before returning control to the application scope, record some state to // facilitate view transfer, and (only if explicitly stopped) send final view events _end_reason = should_close; - OnClose(command, *_end_reason); + OnClose(command, *_end_reason, context, writer); return RumScopeResult::Close; } @@ -113,7 +115,7 @@ RumScopeResult RumSessionScope::Process(const RumCommand& command) { // same basic details as the view that was active when our previous session ended: // this will be a no-op if we already have an active view or have previously created // any views in this session - AttemptViewTransfer(command, *_active_view_from_predecessor); + AttemptViewTransfer(command, *_active_view_from_predecessor, context, writer); // Clear last view state, regardless of whether we successfully created a new view, // so we don't run these checks again @@ -182,7 +184,9 @@ RumScopeResult RumSessionScope::Process(const RumCommand& command) { payload.name, RumVitalStepType::Start, payload.operation_key, - std::nullopt + std::nullopt, + context, + writer ); // Vital events are session-scoped: do not propagate to views @@ -235,7 +239,9 @@ RumScopeResult RumSessionScope::Process(const RumCommand& command) { payload.name, RumVitalStepType::End, payload.operation_key, - vital_failure_reason + vital_failure_reason, + context, + writer ); // Vital events are session-scoped: do not propagate to views @@ -248,7 +254,7 @@ RumScopeResult RumSessionScope::Process(const RumCommand& command) { // TODO(RUM-11247): In case of off-view commands, create Background view if warranted // Propagate the command to any and all child view scopes - _view_scopes.Propagate(command); + _view_scopes.Propagate(command, context, writer); // All of the circumstances that cause a session to end are handled up-front in // ShouldCloseRatherThanProcessing(): no other command types should result in the @@ -291,7 +297,12 @@ RumSessionScope::ShouldCloseRatherThanProcessing(const RumCommand& command) cons return std::nullopt; } -void RumSessionScope::OnClose(const RumCommand& command, EndReason end_reason) { +void RumSessionScope::OnClose( + const RumCommand& command, + EndReason end_reason, + const CoreContext& context, + const EventWriter& writer +) { // If we have an active view, cache its essential details in _active_view_on_close so // they can be conveyed to the next session we might create DATADOG_ASSERT(!_active_view_on_close, "last active view already cached on close"); @@ -311,7 +322,7 @@ void RumSessionScope::OnClose(const RumCommand& command, EndReason end_reason) { // note that we _don't_ send final view events after session expiration if (end_reason == EndReason::Stopped) { DATADOG_ASSERT(command.Is(), "stopped by non-StopSession"); - _view_scopes.Propagate(command); + _view_scopes.Propagate(command, context, writer); } } @@ -320,7 +331,9 @@ void RumSessionScope::SendVitalEvent( std::string_view name, RumVitalStepType step_type, std::optional operation_key, - std::optional failure_reason + std::optional failure_reason, + const CoreContext& context, + const EventWriter& writer ) { const RumScopeDependencies& deps = _deps; @@ -378,14 +391,18 @@ void RumSessionScope::SendVitalEvent( } // Enrich event with OS and device properties from CoreContext - RumEventEnrichment::PopulateCommonProperties(deps.scope, ev); + RumEventEnrichment::PopulateCommonProperties(context, ev); // Serialize and write the event - deps.ProduceEvent(ev); + std::string_view json = deps.EncodeEvent(ev); + writer(Block{json.data(), json.size()}, Block{}); } void RumSessionScope::AttemptViewTransfer( - const RumCommand& command, const RumSessionScope::ViewDetails& prev_view + const RumCommand& command, + const RumSessionScope::ViewDetails& prev_view, + const CoreContext& context, + const EventWriter& writer ) { // If this session is the initial session, or if it's not sampled, then // RumApplicationScope should not have provided it with any last_active_view details @@ -421,7 +438,7 @@ void RumSessionScope::AttemptViewTransfer( RumCommandParams base = command.base; base.attributes = prev_view.attributes; const RumCommand cmd = RumCommand::StartView(std::move(base), view_key, view_name); - _view_scopes.Propagate(cmd); + _view_scopes.Propagate(cmd, context, writer); } } // namespace datadog::impl diff --git a/src/datadog/impl/features/rum/scopes/session.hpp b/src/datadog/impl/features/rum/scopes/session.hpp index a98f9f87..c06ec243 100644 --- a/src/datadog/impl/features/rum/scopes/session.hpp +++ b/src/datadog/impl/features/rum/scopes/session.hpp @@ -14,6 +14,8 @@ #include "datadog/uuid.hpp" +#include "datadog/impl/core/context.hpp" +#include "datadog/impl/core/feature_scope.hpp" #include "datadog/impl/core/feature_types/rum.hpp" #include "datadog/impl/features/rum/containers/view_array.hpp" #include "datadog/impl/features/rum/scope.hpp" @@ -138,7 +140,9 @@ class RumSessionScope { void PopulateContext(struct RumContext& out_context) const; // RumScope interface - RumScopeResult Process(const RumCommand& command); + RumScopeResult Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer + ); private: /** @@ -155,7 +159,12 @@ class RumSessionScope { * `command` will _not_ be propagated to child scopes via the normal code path in * `Process()`, so `OnClose()` has a chance to do so explicitly if desired. */ - void OnClose(const RumCommand& command, EndReason end_reason); + void OnClose( + const RumCommand& command, + EndReason end_reason, + const CoreContext& context, + const EventWriter& writer + ); /** * Attempts to create a new active view scope that's initialized from the details of @@ -163,7 +172,12 @@ class RumSessionScope { * correspond with the timestamp of the given command, but its essential details will * be preserved from the previous scope. */ - void AttemptViewTransfer(const RumCommand& command, const ViewDetails& prev_view); + void AttemptViewTransfer( + const RumCommand& command, + const ViewDetails& prev_view, + const CoreContext& context, + const EventWriter& writer + ); /** * Constructs and emits a RUM vital event in the context of the current session. @@ -175,7 +189,9 @@ class RumSessionScope { std::string_view name, RumVitalStepType step_type, std::optional operation_key, - std::optional failure_reason + std::optional failure_reason, + const CoreContext& context, + const EventWriter& writer ); private: diff --git a/src/datadog/impl/features/rum/scopes/view.cpp b/src/datadog/impl/features/rum/scopes/view.cpp index 1d549118..0ee036c3 100644 --- a/src/datadog/impl/features/rum/scopes/view.cpp +++ b/src/datadog/impl/features/rum/scopes/view.cpp @@ -48,12 +48,15 @@ void RumViewScope::PopulateContext(struct RumContext& out_context) const { out_context.active_view_name = _name; } -RumScopeResult RumViewScope::Process(const RumCommand& command) { +RumScopeResult RumViewScope::Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer +) { // If we have a child action scope, allow it to process this command, and clear it if // the scope is closed as a result bool has_incremented_action_count = false; if (_active_action_scope) { - const RumScopeResult action_result = _active_action_scope->Process(command); + const RumScopeResult action_result = + _active_action_scope->Process(command, context, writer); if (action_result == RumScopeResult::Close) { if (_active_action_scope->HasSentActionEvent()) { _num_actions_completed++; @@ -64,7 +67,7 @@ RumScopeResult RumViewScope::Process(const RumCommand& command) { } // Process the command, updating our internal state as needed - ViewEventType event_type = HandleCommand(command); + ViewEventType event_type = HandleCommand(command, context, writer); // Determine whether this command targets a specific resource, and forward it to that // resource if applicable @@ -72,7 +75,9 @@ RumScopeResult RumViewScope::Process(const RumCommand& command) { if (!target_resource_key.empty()) { // If processing of the command results in a 'resource' or 'error' event being sent, // increment the appropriate count - auto res = _resource_scopes.Forward(std::string{target_resource_key}, command); + auto res = _resource_scopes.Forward( + std::string{target_resource_key}, command, context, writer + ); switch (res) { case RumResourceScope::Result::SentNoEvent: break; @@ -102,7 +107,7 @@ RumScopeResult RumViewScope::Process(const RumCommand& command) { // Generate a 'view' event if our state has meaningfully changed since the last event // we sent if (event_type != ViewEventType::None) { - SendViewEvent(command); + SendViewEvent(command, context, writer); } // If the result of this command is that we're no longer the active view and we no @@ -116,7 +121,9 @@ RumScopeResult RumViewScope::Process(const RumCommand& command) { return RumScopeResult::RemainOpen; } -RumViewScope::ViewEventType RumViewScope::HandleCommand(const RumCommand& command) { +RumViewScope::ViewEventType RumViewScope::HandleCommand( + const RumCommand& command, const CoreContext& context, const EventWriter& writer +) { // On `StopSession`, any active views should should be implicitly stopped if (command.Is()) { return HandleStopSession(command.base); @@ -159,7 +166,9 @@ RumViewScope::ViewEventType RumViewScope::HandleCommand(const RumCommand& comman // or for any other action type we should either a.) open a new action scope with // is_continuous false, or b.) drop the action if we already have an active action if (command.Is()) { - return HandleAddAction(command.base, command.As()); + return HandleAddAction( + command.base, command.As(), context, writer + ); } // On `StartAction`, we should either a.) open a new action scope with is_continuous @@ -177,7 +186,9 @@ RumViewScope::ViewEventType RumViewScope::HandleCommand(const RumCommand& comman // On `AddError`, we should immediately send an error event in the context of this // view, updating our view state accordingly if (command.Is()) { - return HandleAddError(command.base, command.As()); + return HandleAddError( + command.base, command.As(), context, writer + ); } return ViewEventType::None; @@ -283,7 +294,10 @@ RumViewScope::ViewEventType RumViewScope::HandleRemoveViewAttribute( } RumViewScope::ViewEventType RumViewScope::HandleAddAction( - const RumCommandParams& base, const RumAddActionPayload& payload + const RumCommandParams& base, + const RumAddActionPayload& payload, + const CoreContext& context, + const EventWriter& writer ) { // If already inactive, we can ignore AddAction if (!_is_active) { @@ -292,7 +306,7 @@ RumViewScope::ViewEventType RumViewScope::HandleAddAction( // A discrete custom action should always be recorded immediately if (payload.type == RumActionType::Custom) { - ProcessDiscreteCustomAction(base, payload.name); + ProcessDiscreteCustomAction(base, payload.name, context, writer); return ViewEventType::Full; } @@ -353,7 +367,10 @@ RumViewScope::ViewEventType RumViewScope::HandleStartResource( } RumViewScope::ViewEventType RumViewScope::HandleAddError( - const RumCommandParams& base, const RumAddErrorPayload& payload + const RumCommandParams& base, + const RumAddErrorPayload& payload, + const CoreContext& context, + const EventWriter& writer ) { // If the view is no longer active, it should report no errors if (!_is_active) { @@ -361,7 +378,7 @@ RumViewScope::ViewEventType RumViewScope::HandleAddError( } // Immediately generate a RUM 'error' event describing the error - SendErrorEvent(base, payload); + SendErrorEvent(base, payload, context, writer); // Our error count has been incremented we must update the state of the view return ViewEventType::Full; @@ -394,7 +411,10 @@ void RumViewScope::BecomeInactive( } void RumViewScope::ProcessDiscreteCustomAction( - const RumCommandParams& base, std::string_view name + const RumCommandParams& base, + std::string_view name, + const CoreContext& context, + const EventWriter& writer ) { const RumScopeDependencies& deps = _deps; const UUID action_id = UUID::Random(); @@ -410,8 +430,9 @@ void RumViewScope::ProcessDiscreteCustomAction( ); RumCommandParams base_copy = base; - const RumScopeResult result = - scope.Process(RumCommand::StopAction(std::move(base_copy), name)); + const RumScopeResult result = scope.Process( + RumCommand::StopAction(std::move(base_copy), name), context, writer + ); DATADOG_ASSERT( result == RumScopeResult::Close, @@ -478,7 +499,9 @@ std::string_view RumViewScope::IdentifyTargetResourceKey(const RumCommand& comma return std::string_view{}; } -void RumViewScope::SendViewEvent(const RumCommand& command) { +void RumViewScope::SendViewEvent( + const RumCommand& command, const CoreContext& context, const EventWriter& writer +) { // Resolve references needed to populate required event data const RumScopeDependencies& deps = _deps; const RumSessionScope& session = _parent; @@ -538,15 +561,19 @@ void RumViewScope::SendViewEvent(const RumCommand& command) { } // Enrich event with OS and device properties from CoreContext - RumEventEnrichment::PopulateCommonProperties(deps.scope, ev); + RumEventEnrichment::PopulateCommonProperties(context, ev); - // Serialize the event to JSON in a shared buffer, then copy that raw event payload - // onto the storage thread - deps.ProduceEvent(ev); + // Serialize the event to JSON in a shared buffer, then write it using the provided + // writer callback + std::string_view json = deps.EncodeEvent(ev); + writer(Block{json.data(), json.size()}, Block{}); } void RumViewScope::SendErrorEvent( - const RumCommandParams& base, const RumAddErrorPayload& payload + const RumCommandParams& base, + const RumAddErrorPayload& payload, + const CoreContext& context, + const EventWriter& writer ) { DATADOG_ASSERT(_is_active, "SendErrorEvent called while view scope is inactive"); @@ -597,9 +624,10 @@ void RumViewScope::SendErrorEvent( } // Enrich event with OS and device properties from CoreContext - RumEventEnrichment::PopulateCommonProperties(deps.scope, ev); + RumEventEnrichment::PopulateCommonProperties(context, ev); - deps.ProduceEvent(ev); + std::string_view json = deps.EncodeEvent(ev); + writer(Block{json.data(), json.size()}, Block{}); _num_errors_reported++; } diff --git a/src/datadog/impl/features/rum/scopes/view.hpp b/src/datadog/impl/features/rum/scopes/view.hpp index d753abb7..de319dc9 100644 --- a/src/datadog/impl/features/rum/scopes/view.hpp +++ b/src/datadog/impl/features/rum/scopes/view.hpp @@ -12,6 +12,8 @@ #include "datadog/uuid.hpp" #include "datadog/impl/attribute/typed_attribute.hpp" +#include "datadog/impl/core/context.hpp" +#include "datadog/impl/core/feature_scope.hpp" #include "datadog/impl/features/rum/containers/resource_map.hpp" #include "datadog/impl/features/rum/scope.hpp" #include "datadog/impl/features/rum/scopes/action.hpp" @@ -144,7 +146,9 @@ class RumViewScope { void PopulateContext(struct RumContext& out_context) const; // RumScope interface - RumScopeResult Process(const RumCommand& command); + RumScopeResult Process( + const RumCommand& command, const CoreContext& context, const EventWriter& writer + ); private: /** @@ -158,7 +162,9 @@ class RumViewScope { * Updates view state in response to the given command, then returns what sort of view * event (if any) we should generate in response. */ - ViewEventType HandleCommand(const RumCommand& command); + ViewEventType HandleCommand( + const RumCommand& command, const CoreContext& context, const EventWriter& writer + ); // Handler functions for individual command types ViewEventType HandleStopSession(const RumCommandParams& base); @@ -175,7 +181,10 @@ class RumViewScope { const RumCommandParams& base, const RumRemoveViewAttributePayload& payload ); ViewEventType HandleAddAction( - const RumCommandParams& base, const RumAddActionPayload& payload + const RumCommandParams& base, + const RumAddActionPayload& payload, + const CoreContext& context, + const EventWriter& writer ); ViewEventType HandleStartAction( const RumCommandParams& base, const RumStartActionPayload& payload @@ -184,7 +193,10 @@ class RumViewScope { const RumCommandParams& base, const RumStartResourcePayload& payload ); ViewEventType HandleAddError( - const RumCommandParams& base, const RumAddErrorPayload& payload + const RumCommandParams& base, + const RumAddErrorPayload& payload, + const CoreContext& context, + const EventWriter& writer ); /** @@ -199,7 +211,12 @@ class RumViewScope { */ void BecomeInactive(const RumCommandParams& base, bool accept_command_attributes); - void ProcessDiscreteCustomAction(const RumCommandParams& base, std::string_view name); + void ProcessDiscreteCustomAction( + const RumCommandParams& base, + std::string_view name, + const CoreContext& context, + const EventWriter& writer + ); void OpenActionScope( const RumCommandParams& base, @@ -215,12 +232,19 @@ class RumViewScope { /** * Generates and sends a RUM view event in response to the given command. */ - void SendViewEvent(const RumCommand& command); + void SendViewEvent( + const RumCommand& command, const CoreContext& context, const EventWriter& writer + ); /** * Generates and sends a RUM error event in response to the given command. */ - void SendErrorEvent(const RumCommandParams& base, const RumAddErrorPayload& payload); + void SendErrorEvent( + const RumCommandParams& base, + const RumAddErrorPayload& payload, + const CoreContext& context, + const EventWriter& writer + ); private: std::reference_wrapper _deps; diff --git a/tests/impl/features/rum/containers/resource_map_test.cpp b/tests/impl/features/rum/containers/resource_map_test.cpp index f1d3b3b4..472d38d3 100644 --- a/tests/impl/features/rum/containers/resource_map_test.cpp +++ b/tests/impl/features/rum/containers/resource_map_test.cpp @@ -16,6 +16,7 @@ #include "datadog/impl/features/rum/scopes/view.hpp" #include "mock/clock.hpp" +#include "mock/system_info.hpp" #include "support/catch.hpp" using namespace datadog; @@ -44,6 +45,14 @@ TEST_CASE("RumResourceMap", "[unit][rum]") { // And a set of prerequisite values required to initialize new RumResourceScopes RumConfig config("a991ca10-4004-4004-4004-beefbeefbeef"); MockClock clock; + MockSystemInfo system_info; + CoreContext context( + CoreConfig{"test-token", "test-service", "test-env"}, + system_info.os_info, + system_info.device_info + ); + EventWriter writer = [](Block, Block) { return true; }; + clock.FreezeAtMilliseconds(1700000000000); RumScopeDependencies deps(config, clock); RumApplicationScope application(deps); @@ -122,7 +131,8 @@ TEST_CASE("RumResourceMap", "[unit][rum]") { SECTION("M forward command to target resource W key matches") { // When we forward a StopResource command for 'bar' using the key 'bar' - auto result = map.Forward("bar", RumCommand::StopResource(base(), "bar")); + auto result = + map.Forward("bar", RumCommand::StopResource(base(), "bar"), context, writer); // Then our command is handled and results in a resource event being sent REQUIRE(result == RumResourceScope::Result::SentResourceEvent); @@ -137,7 +147,9 @@ TEST_CASE("RumResourceMap", "[unit][rum]") { SECTION("M do nothing W command does not match any resource key") { // When we forward a StopResource command for 'nobody' using the key 'nobody' - auto result = map.Forward("nobody", RumCommand::StopResource(base(), "nobody")); + auto result = map.Forward( + "nobody", RumCommand::StopResource(base(), "nobody"), context, writer + ); // Then our command is ignored, resulting in no event REQUIRE(result == RumResourceScope::Result::SentNoEvent); diff --git a/tests/impl/features/rum/containers/view_array_test.cpp b/tests/impl/features/rum/containers/view_array_test.cpp index 1cf38f85..313897a1 100644 --- a/tests/impl/features/rum/containers/view_array_test.cpp +++ b/tests/impl/features/rum/containers/view_array_test.cpp @@ -15,6 +15,7 @@ #include "datadog/impl/features/rum/scopes/view.hpp" #include "mock/clock.hpp" +#include "mock/system_info.hpp" #include "support/catch.hpp" using namespace datadog; @@ -62,6 +63,14 @@ TEST_CASE("RumViewArray", "[unit][rum]") { // And a set of prerequisite values required to initialize new RumViewScopes RumConfig config("a991ca10-4004-4004-4004-beefbeefbeef"); MockClock clock; + MockSystemInfo system_info; + CoreContext context( + CoreConfig{"test-token", "test-service", "test-env"}, + system_info.os_info, + system_info.device_info + ); + EventWriter writer = [](Block, Block) { return true; }; + clock.FreezeAtMilliseconds(1700000000000); RumScopeDependencies deps(config, clock); RumApplicationScope application(deps); @@ -105,7 +114,9 @@ TEST_CASE("RumViewArray", "[unit][rum]") { SECTION("M do nothing W Propagate is called") { // When we propagate a command to our set of zero child scopes - array.Propagate(RumCommand::StartAction(base(), RumActionType::Custom, "foo")); + array.Propagate( + RumCommand::StartAction(base(), RumActionType::Custom, "foo"), context, writer + ); // Then nothing happens REQUIRE(count_items() == 0); @@ -152,7 +163,9 @@ TEST_CASE("RumViewArray", "[unit][rum]") { REQUIRE(array.items[0]->GetActiveAction() == std::nullopt); REQUIRE(array.items[1].has_value()); REQUIRE(array.items[1]->GetActiveAction() == std::nullopt); - array.Propagate(RumCommand::StartAction(base(), RumActionType::Custom, "foo")); + array.Propagate( + RumCommand::StartAction(base(), RumActionType::Custom, "foo"), context, writer + ); // Then our scopes remain alive, and their state is updated in response to the // command @@ -163,7 +176,7 @@ TEST_CASE("RumViewArray", "[unit][rum]") { SECTION("M pass command to all scopes W Propagate is called {both Close}") { // When we propagate a command that will cause both our scopes to close - array.Propagate(RumCommand::StopSession(base())); + array.Propagate(RumCommand::StopSession(base()), context, writer); // Then both scopes are removed from the array REQUIRE(count_items() == 0); @@ -174,7 +187,9 @@ TEST_CASE("RumViewArray", "[unit][rum]") { ) { // When we propagate a command that will cause view-0 to be closed but keep view-1 // open - array.Propagate(RumCommand::StartView(base(), "view-1", "view-1")); + array.Propagate( + RumCommand::StartView(base(), "view-1", "view-1"), context, writer + ); // Then one scope is removed from the array REQUIRE(count_items() == 1); @@ -240,7 +255,9 @@ TEST_CASE("RumViewArray", "[unit][rum]") { SECTION("M pass command to all scopes W Propagate is called {all RemainOpen}") { // When we propagate a command that will update all our scopes and keep them open - array.Propagate(RumCommand::StartAction(base(), RumActionType::Custom, "foo")); + array.Propagate( + RumCommand::StartAction(base(), RumActionType::Custom, "foo"), context, writer + ); // Then all our scopes remain alive REQUIRE(count_items() == array.items.max_size()); @@ -248,7 +265,7 @@ TEST_CASE("RumViewArray", "[unit][rum]") { SECTION("M pass command to all scopes W Propagate is called {all Close}") { // When we propagate a command that will cause all our scopes to close - array.Propagate(RumCommand::StopSession(base())); + array.Propagate(RumCommand::StopSession(base()), context, writer); // Then all scopes are removed from the array REQUIRE(count_items() == 0); diff --git a/tests/impl/features/rum/scopes/action_test.cpp b/tests/impl/features/rum/scopes/action_test.cpp index fa985bbf..3ce426d7 100644 --- a/tests/impl/features/rum/scopes/action_test.cpp +++ b/tests/impl/features/rum/scopes/action_test.cpp @@ -46,6 +46,12 @@ class ActionFixture { RumEventCapture event_capture; public: + CoreContext GetTestContext() { return event_capture.GetFeatureScope().GetContext(); } + EventWriter GetTestWriter() { + return [this](Block event, Block metadata) { + return event_capture.GetFeatureScope().WriteEvent(event, metadata); + }; + } ActionFixture() : config(APPLICATION_ID), deps(config, clock), @@ -86,7 +92,6 @@ class ActionFixture { Attribute() ), event_capture(APPLICATION_ID, SESSION_ID, VIEW_ID) { - deps.scope = &event_capture.GetFeatureScope(); deps.diagnostic_logger = event_capture.GetFeatureScope().diagnostic_logger; clock.FreezeAtMilliseconds(1700000000000); } @@ -97,7 +102,9 @@ class ActionFixture { TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { SECTION("M close and send event W StopSession is processed") { // When we process StopSession - const auto result = scope.Process(RumCommand::StopSession(GetBaseParams())); + const auto result = scope.Process( + RumCommand::StopSession(GetBaseParams()), GetTestContext(), GetTestWriter() + ); // Then our action scope is closed REQUIRE(result == RumScopeResult::Close); @@ -112,8 +119,11 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { SECTION("M close and send event W StartView is processed") { // When we process StartView with _any_ view key, at T+5ms clock.TickMilliseconds(5); - const auto result = - scope.Process(RumCommand::StartView(GetBaseParams(), "some-view", "Some View")); + const auto result = scope.Process( + RumCommand::StartView(GetBaseParams(), "some-view", "Some View"), + GetTestContext(), + GetTestWriter() + ); // Then our action scope is closed REQUIRE(result == RumScopeResult::Close); @@ -129,8 +139,11 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { SECTION("M close and send event W StopView is processed") { // When we process StopView - const auto result = - scope.Process(RumCommand::StopView(GetBaseParams(), "some-view")); + const auto result = scope.Process( + RumCommand::StopView(GetBaseParams(), "some-view"), + GetTestContext(), + GetTestWriter() + ); // Then our action scope is closed REQUIRE(result == RumScopeResult::Close); @@ -148,7 +161,9 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { GetBaseParams(), "some-resource", RumRequestDetails{RumResourceMethod::Get, "http://localhost:5000/foo"} - ) + ), + GetTestContext(), + GetTestWriter() ); // Then our action scope remains open @@ -168,7 +183,9 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { GetBaseParams(), "some-resource", RumRequestDetails{RumResourceMethod::Get, "http://localhost:5000/foo"} - ) + ), + GetTestContext(), + GetTestWriter() ); // Then our action scope is closed, because the command was processed after the @@ -198,7 +215,9 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { GetBaseParams(), "some-resource", RumRequestDetails{RumResourceMethod::Get, "http://localhost:5000/foo"} - ) + ), + GetTestContext(), + GetTestWriter() ); // Then our action scope remains open @@ -206,7 +225,11 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { // Next: When we process StopResource at T+150ms clock.TickMilliseconds(100); - result = scope.Process(RumCommand::StopResource(GetBaseParams(), "some-resource")); + result = scope.Process( + RumCommand::StopResource(GetBaseParams(), "some-resource"), + GetTestContext(), + GetTestWriter() + ); // Then our scope is closed, because the timeout has passed and we no longer have // pending resources @@ -233,26 +256,42 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { // When we start a resource 'foo' before timeout, Then our action scope remains open clock.TickMilliseconds(50); - auto result = scope.Process(RumCommand::StartResource(GetBaseParams(), "foo", req)); + auto result = scope.Process( + RumCommand::StartResource(GetBaseParams(), "foo", req), + GetTestContext(), + GetTestWriter() + ); REQUIRE(result == RumScopeResult::RemainOpen); // When we start a resource 'bar' at any point thereafter, so long as 'foo' is still // pending, Then our action scope remains open clock.TickMilliseconds(100); - result = scope.Process(RumCommand::StartResource(GetBaseParams(), "bar", req)); + result = scope.Process( + RumCommand::StartResource(GetBaseParams(), "bar", req), + GetTestContext(), + GetTestWriter() + ); REQUIRE(result == RumScopeResult::RemainOpen); // When we stop either one of those two resources, Then our action scope still // remains open, because there's still another resource pending clock.TickMilliseconds(350); - result = scope.Process(RumCommand::StopResource(GetBaseParams(), "foo")); + result = scope.Process( + RumCommand::StopResource(GetBaseParams(), "foo"), + GetTestContext(), + GetTestWriter() + ); REQUIRE(result == RumScopeResult::RemainOpen); // When we stop the final resource, Then our action scope finally closes and sends // an event that records 2 resources clock.TickMilliseconds(400); REQUIRE(event_capture.Actions().empty()); - result = scope.Process(RumCommand::StopResource(GetBaseParams(), "bar")); + result = scope.Process( + RumCommand::StopResource(GetBaseParams(), "bar"), + GetTestContext(), + GetTestWriter() + ); REQUIRE(result == RumScopeResult::Close); auto actions = event_capture.Actions(); REQUIRE(actions.size() == 1); @@ -268,7 +307,9 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { GetBaseParams(), "some-resource", RumRequestDetails{RumResourceMethod::Get, "http://localhost:5000/foo"} - ) + ), + GetTestContext(), + GetTestWriter() ); // Then our action scope remains open @@ -279,7 +320,9 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { result = scope.Process( RumCommand::StopResource( GetBaseParams(), "some-resource", RumResponseDetails(), RumErrorDetails() - ) + ), + GetTestContext(), + GetTestWriter() ); // Then our scope remains open, because we haven't exceeded the timeout @@ -288,7 +331,9 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { // Next: When the action scope is closed for any reason and sends its action event clock.TickMilliseconds(3); - result = scope.Process(RumCommand::StopSession(GetBaseParams())); + result = scope.Process( + RumCommand::StopSession(GetBaseParams()), GetTestContext(), GetTestWriter() + ); REQUIRE(result == RumScopeResult::Close); auto actions = event_capture.Actions(); REQUIRE(actions.size() == 1); @@ -309,13 +354,17 @@ TEST_CASE_METHOD(ActionFixture, "RumActionScope::Process", "[unit][rum]") { GetBaseParams(), RumErrorSource::Source, RumErrorDetails{"oops", "Error", "stacktrace"} - ) + ), + GetTestContext(), + GetTestWriter() ); REQUIRE(result == RumScopeResult::RemainOpen); // And we then process StopAction to explicitly end the action 10ms later clock.TickMilliseconds(10); - result = scope.Process(RumCommand::StopAction(GetBaseParams(), "")); + result = scope.Process( + RumCommand::StopAction(GetBaseParams(), ""), GetTestContext(), GetTestWriter() + ); REQUIRE(result == RumScopeResult::Close); auto actions = event_capture.Actions(); REQUIRE(actions.size() == 1); diff --git a/tests/impl/features/rum/scopes/application_test.cpp b/tests/impl/features/rum/scopes/application_test.cpp index 0a903b66..2b3fb21b 100644 --- a/tests/impl/features/rum/scopes/application_test.cpp +++ b/tests/impl/features/rum/scopes/application_test.cpp @@ -16,6 +16,7 @@ #include "datadog/impl/features/rum/context.hpp" #include "mock/clock.hpp" +#include "mock/system_info.hpp" using namespace datadog; using namespace datadog::impl; @@ -25,13 +26,26 @@ class ApplicationFixture { static constexpr const char* APPLICATION_ID = "a991ca10-4004-4004-4004-beefbeefbeef"; MockClock clock; + MockSystemInfo system_info; RumConfig config; RumScopeDependencies deps; RumApplicationScope scope; + CoreContext context; + EventWriter writer; + public: - ApplicationFixture() : config(APPLICATION_ID), deps(config, clock), scope(deps) { + ApplicationFixture() + : config(APPLICATION_ID), + deps(config, clock), + scope(deps), + context( + CoreConfig{"test-token", "test-service", "test-env"}, + system_info.os_info, + system_info.device_info + ), + writer([](Block, Block) { return true; }) { clock.FreezeAtMilliseconds(1700000000000); } @@ -44,7 +58,7 @@ TEST_CASE_METHOD(ApplicationFixture, "RumApplicationScope::Process", "[unit][rum REQUIRE(scope.GetActiveSession() == std::nullopt); // When we process SDKInit - scope.Process(RumCommand::SDKInit(GetBaseParams())); + scope.Process(RumCommand::SDKInit(GetBaseParams()), context, writer); // Then a new session is created auto session_opt = scope.GetActiveSession(); @@ -61,12 +75,12 @@ TEST_CASE_METHOD(ApplicationFixture, "RumApplicationScope::Process", "[unit][rum SECTION("M end existing session W StopSession is processed") { // Given a RumApplicationScope with an active session - scope.Process(RumCommand::SDKInit(GetBaseParams())); + scope.Process(RumCommand::SDKInit(GetBaseParams()), context, writer); REQUIRE(scope.GetActiveSession()); const UUID initial_session_id = (*scope.GetActiveSession()).get().GetSessionID(); // When we process StopSession - scope.Process(RumCommand::StopSession(GetBaseParams())); + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); // Then we no longer have an active session REQUIRE(scope.GetActiveSession() == std::nullopt); @@ -82,17 +96,21 @@ TEST_CASE_METHOD(ApplicationFixture, "RumApplicationScope::Process", "[unit][rum SECTION("M start new session W user interaction is processed after StopSession") { // Given a RumApplicationScope with an active session - scope.Process(RumCommand::SDKInit(GetBaseParams())); + scope.Process(RumCommand::SDKInit(GetBaseParams()), context, writer); REQUIRE(scope.GetActiveSession()); const UUID initial_session_id = (*scope.GetActiveSession()).get().GetSessionID(); // When we process StopSession - scope.Process(RumCommand::StopSession(GetBaseParams())); + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); // And then we subsequently process a command like AddAction // (Note that the '{on session refresh}' tests below validate a more exhaustive // range of command types) - scope.Process(RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo")); + scope.Process( + RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo"), + context, + writer + ); // Then we once again have an active session, and it's distinct from the first REQUIRE(scope.GetActiveSession()); @@ -111,16 +129,16 @@ TEST_CASE_METHOD(ApplicationFixture, "RumApplicationScope::Process", "[unit][rum SECTION("M do nothing W StopSession is processed after StopSession") { // Given a RumApplicationScope that's received StopSession, and therefore has no // active session - scope.Process(RumCommand::SDKInit(GetBaseParams())); + scope.Process(RumCommand::SDKInit(GetBaseParams()), context, writer); REQUIRE(scope.GetActiveSession()); const UUID initial_session_id = (*scope.GetActiveSession()).get().GetSessionID(); - scope.Process(RumCommand::StopSession(GetBaseParams())); + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); REQUIRE(scope.GetActiveSession() == std::nullopt); // When we process an additional StopSession command in our already-stopped session // (Note that the '{on session refresh}' tests below validate a more exhaustive // range of command types) - scope.Process(RumCommand::StopSession(GetBaseParams())); + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); // Then nothing happens: we still have no active session REQUIRE(scope.GetActiveSession() == std::nullopt); @@ -139,14 +157,14 @@ TEST_CASE_METHOD(ApplicationFixture, "RumApplicationScope::Process", "[unit][rum "15m+ of inactivity" ) { // Given a RumApplicationScope with an active session - scope.Process(RumCommand::SDKInit(GetBaseParams())); + scope.Process(RumCommand::SDKInit(GetBaseParams()), context, writer); REQUIRE(scope.GetActiveSession()); const UUID initial_session_id = (*scope.GetActiveSession()).get().GetSessionID(); // When we wait 16 minutes, then process any command that represents user // interaction clock.Tick(std::chrono::minutes(16)); - scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); // Then we have an active session that's distinct from the first one REQUIRE(scope.GetActiveSession()); @@ -161,7 +179,7 @@ TEST_CASE_METHOD(ApplicationFixture, "RumApplicationScope::Process", "[unit][rum SECTION("M end existing session and start new one W session duration is 4h+") { // Given a RumApplicationScope with an active session - scope.Process(RumCommand::SDKInit(GetBaseParams())); + scope.Process(RumCommand::SDKInit(GetBaseParams()), context, writer); REQUIRE(scope.GetActiveSession()); const UUID initial_session_id = (*scope.GetActiveSession()).get().GetSessionID(); @@ -169,7 +187,7 @@ TEST_CASE_METHOD(ApplicationFixture, "RumApplicationScope::Process", "[unit][rum // times so that our last interaction is recorded at 3h58m into the session for (int i = 1; i <= 17; i++) { clock.Tick(std::chrono::minutes(14)); - scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); } // Then as of 3h58m, our original session should still be active @@ -180,7 +198,7 @@ TEST_CASE_METHOD(ApplicationFixture, "RumApplicationScope::Process", "[unit][rum // Next: When we wait three minutes, then try to record a user interaction at 4h01m clock.Tick(std::chrono::minutes(3)); - scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); // Then we have an active session that's distinct from the first one REQUIRE(scope.GetActiveSession()); @@ -204,11 +222,15 @@ class ViewTransferFixture { static constexpr const char* APPLICATION_ID = "a991ca10-4004-4004-4004-beefbeefbeef"; MockClock clock; + MockSystemInfo system_info; RumConfig config; RumScopeDependencies deps; RumApplicationScope scope; + CoreContext context; + EventWriter writer; + // Recorded details of the session and view we started with UUID initial_session_id; UUID initial_view_id; @@ -226,17 +248,28 @@ class ViewTransferFixture { State state{State::Initial}; public: - ViewTransferFixture() : config(APPLICATION_ID), deps(config, clock), scope(deps) { + ViewTransferFixture() + : config(APPLICATION_ID), + deps(config, clock), + scope(deps), + context( + CoreConfig{"test-token", "test-service", "test-env"}, + system_info.os_info, + system_info.device_info + ), + writer([](Block, Block) { return true; }) { clock.FreezeAtMilliseconds(1700000000000); // Issue SDKInit to create an initial session - scope.Process(RumCommand::SDKInit(GetBaseParams())); + scope.Process(RumCommand::SDKInit(GetBaseParams()), context, writer); auto session_opt = scope.GetActiveSession(); REQUIRE(session_opt.has_value()); const RumSessionScope& session = session_opt->get(); // Create a new view with key 'foo' - scope.Process(RumCommand::StartView(GetBaseParams(), "foo", "Foo")); + scope.Process( + RumCommand::StartView(GetBaseParams(), "foo", "Foo"), context, writer + ); auto view_opt = session.GetActiveView(); REQUIRE(view_opt.has_value()); const RumViewScope& view = view_opt->get(); @@ -279,7 +312,9 @@ class ViewTransferFixture { for (int i = 1; i <= 17; i++) { // User interactions recorded at [T+14m, T+28m, ..., T+238m] clock.Tick(std::chrono::minutes(14)); - scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + scope.Process( + RumCommand::StopAction(GetBaseParams(), "foo"), context, writer + ); } // Advance one minute beyond the max session duration: next command will trigger // expiration and refresh @@ -290,7 +325,7 @@ class ViewTransferFixture { // Advance past the start of the session, then dispatch StopSession: on next // command, there will be no active session clock.Tick(std::chrono::minutes(5)); - scope.Process(RumCommand::StopSession(GetBaseParams())); + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); RequireNoActiveSession(); return; } @@ -416,7 +451,7 @@ TEST_CASE_METHOD( )); // When we process a StopSession call in that expired or closed state - scope.Process(RumCommand::StopSession(GetBaseParams())); + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); // Then we are left without an active session, because StopSession was called RequireNoActiveSession(); @@ -452,7 +487,9 @@ TEST_CASE_METHOD( )); // When we trigger session refresh with a StartView command - scope.Process(RumCommand::StartView(GetBaseParams(), "bar", "Bar")); + scope.Process( + RumCommand::StartView(GetBaseParams(), "bar", "Bar"), context, writer + ); // Then a new session is created, and it contains our new view, not a copy of the // one we started with @@ -472,7 +509,7 @@ TEST_CASE_METHOD( auto view_key = GENERATE("foo", "bar"); // When we trigger session refresh with that StopView command - scope.Process(RumCommand::StopView(GetBaseParams(), view_key)); + scope.Process(RumCommand::StopView(GetBaseParams(), view_key), context, writer); // Then the session is refreshed, but it remains without an active view RequireNewSessionWithNoActiveView(); @@ -483,7 +520,7 @@ TEST_CASE_METHOD( EnterState(ViewTransferFixture::State::ExplicitlyStopped); // When we process StopView after the session has been explicitly stopped - scope.Process(RumCommand::StopView(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopView(GetBaseParams(), "foo"), context, writer); // Then the command is ignored and no session refresh occurs RequireNoActiveSession(); @@ -502,7 +539,9 @@ TEST_CASE_METHOD( scope.Process( RumCommand::StartResource( GetBaseParams(), "foo", RumRequestDetails{RumResourceMethod::Get, "/foo"} - ) + ), + context, + writer ); // Then the session is refreshed, and the last active view is recreated in order @@ -518,7 +557,9 @@ TEST_CASE_METHOD( scope.Process( RumCommand::StartResource( GetBaseParams(), "foo", RumRequestDetails{RumResourceMethod::Get, "/foo"} - ) + ), + context, + writer ); // Then the StartResource call is ignored @@ -535,7 +576,7 @@ TEST_CASE_METHOD( )); // When we trigger session refresh with StopResource - scope.Process(RumCommand::StopResource(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopResource(GetBaseParams(), "foo"), context, writer); // Then the session is refreshed, but it remains without an active view RequireNewSessionWithNoActiveView(); @@ -546,7 +587,7 @@ TEST_CASE_METHOD( EnterState(ViewTransferFixture::State::ExplicitlyStopped); // When we process StopResource after the session has been explicitly stopped - scope.Process(RumCommand::StopResource(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopResource(GetBaseParams(), "foo"), context, writer); // Then the command is ignored and no session refresh occurs RequireNoActiveSession(); @@ -563,7 +604,11 @@ TEST_CASE_METHOD( )); // When we trigger session refresh with a AddAction command - scope.Process(RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo")); + scope.Process( + RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo"), + context, + writer + ); // Then the session is refreshed and the last active view is recreated RequireRecreatedView(); @@ -581,7 +626,9 @@ TEST_CASE_METHOD( // When we trigger session refresh with a StartAction command scope.Process( - RumCommand::StartAction(GetBaseParams(), RumActionType::Tap, "foo") + RumCommand::StartAction(GetBaseParams(), RumActionType::Tap, "foo"), + context, + writer ); // Then the session is refreshed and the last active view is recreated @@ -598,7 +645,7 @@ TEST_CASE_METHOD( )); // When we trigger session refresh with StopAction - scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); // Then the session is refreshed, but it remains without an active view RequireNewSessionWithNoActiveView(); @@ -609,7 +656,7 @@ TEST_CASE_METHOD( EnterState(ViewTransferFixture::State::ExplicitlyStopped); // When we process StopAction after the session has been explicitly stopped - scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); // Then the command is ignored and no session refresh occurs RequireNoActiveSession(); @@ -629,12 +676,18 @@ TEST_CASE_METHOD( scope.Process( RumCommand::StartResource( GetBaseParams(), "foo", RumRequestDetails{RumResourceMethod::Get, "/foo"} - ) + ), + context, + writer ); // And then we handle an AddAction command post-refresh clock.Tick(std::chrono::seconds(1)); - scope.Process(RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo")); + scope.Process( + RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo"), + context, + writer + ); // Then the session is refreshed and the last active view is recreated RequireRecreatedView(); @@ -651,11 +704,15 @@ TEST_CASE_METHOD( )); // When we trigger session refresh with a StopResource command - scope.Process(RumCommand::StopResource(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopResource(GetBaseParams(), "foo"), context, writer); // And then we handle an AddAction command post-refresh clock.Tick(std::chrono::seconds(1)); - scope.Process(RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo")); + scope.Process( + RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo"), + context, + writer + ); // Then the session is refreshed and the last active view is recreated RequireRecreatedView(); @@ -673,11 +730,15 @@ TEST_CASE_METHOD( // When we trigger session refresh with a StopView command that targets a // different view than the one that's active - scope.Process(RumCommand::StopView(GetBaseParams(), "bar")); + scope.Process(RumCommand::StopView(GetBaseParams(), "bar"), context, writer); // And then we handle an AddAction command post-refresh clock.Tick(std::chrono::seconds(1)); - scope.Process(RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo")); + scope.Process( + RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo"), + context, + writer + ); // Then the session is refreshed and the last active view is recreated RequireRecreatedView(); @@ -695,11 +756,15 @@ TEST_CASE_METHOD( // When we trigger session refresh with a StopView command that targets the active // view - scope.Process(RumCommand::StopView(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopView(GetBaseParams(), "foo"), context, writer); // And then we handle an AddAction command post-refresh clock.Tick(std::chrono::seconds(1)); - scope.Process(RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo")); + scope.Process( + RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo"), + context, + writer + ); // TODO(RUM-12247): With background tracking enabled, the new session would have // a 'Background' view @@ -717,7 +782,7 @@ TEST_CASE_METHOD( // When we trigger session refresh with a StopView command that targets the active // view (which should clear any cached view-transfer state for that view) - scope.Process(RumCommand::StopView(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopView(GetBaseParams(), "foo"), context, writer); // Then we still have no active session, because StopView does not trigger session // refresh after explicit stop @@ -725,7 +790,11 @@ TEST_CASE_METHOD( // Next: When we handle an AddAction command post-refresh clock.Tick(std::chrono::seconds(1)); - scope.Process(RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo")); + scope.Process( + RumCommand::AddAction(GetBaseParams(), RumActionType::Tap, "foo"), + context, + writer + ); // TODO(RUM-12247): With background tracking enabled, the new session would have // a 'Background' view @@ -746,7 +815,7 @@ TEST_CASE_METHOD( )); // When we trigger session refresh with a StopResource command - scope.Process(RumCommand::StopResource(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopResource(GetBaseParams(), "foo"), context, writer); // Then either we end up with an expired session or we remain in our // post-StopSession state with no active session @@ -757,7 +826,9 @@ TEST_CASE_METHOD( } // Next: When we process StartView to explicitly register a new view - scope.Process(RumCommand::StartView(GetBaseParams(), "bar", "Bar")); + scope.Process( + RumCommand::StartView(GetBaseParams(), "bar", "Bar"), context, writer + ); // Then that view is active in a new session RequireDifferentView("bar", "Bar"); @@ -769,7 +840,9 @@ TEST_CASE_METHOD( // Next: When we process StartAction scope.Process( - RumCommand::StartAction(GetBaseParams(), RumActionType::Tap, "foo") + RumCommand::StartAction(GetBaseParams(), RumActionType::Tap, "foo"), + context, + writer ); // Then we remain in our newly-created view: the last-active view from our diff --git a/tests/impl/features/rum/scopes/resource_test.cpp b/tests/impl/features/rum/scopes/resource_test.cpp index 864c0a83..88bc9737 100644 --- a/tests/impl/features/rum/scopes/resource_test.cpp +++ b/tests/impl/features/rum/scopes/resource_test.cpp @@ -46,6 +46,12 @@ class ResourceFixture { RumEventCapture event_capture; public: + CoreContext GetTestContext() { return event_capture.GetFeatureScope().GetContext(); } + EventWriter GetTestWriter() { + return [this](Block event, Block metadata) { + return event_capture.GetFeatureScope().WriteEvent(event, metadata); + }; + } ResourceFixture() : config(APPLICATION_ID), deps(config, clock), @@ -86,7 +92,6 @@ class ResourceFixture { Attribute() ), event_capture(APPLICATION_ID, SESSION_ID, VIEW_ID) { - deps.scope = &event_capture.GetFeatureScope(); deps.diagnostic_logger = event_capture.GetFeatureScope().diagnostic_logger; clock.FreezeAtMilliseconds(1700000000000); } @@ -109,7 +114,9 @@ TEST_CASE_METHOD(ResourceFixture, "RumResourceScope::Process", "[unit][rum]") { GetBaseParams(), "my-resource-key", RumResponseDetails{403, 16, RumResourceType::Other} - ) + ), + GetTestContext(), + GetTestWriter() ); // Then our resource scope is closed @@ -149,7 +156,9 @@ TEST_CASE_METHOD(ResourceFixture, "RumResourceScope::Process", "[unit][rum]") { "my-resource-key", RumResponseDetails{100}, RumErrorDetails{"oh no", "BadException", "stack\ntrace\n"} - ) + ), + GetTestContext(), + GetTestWriter() ); // Then our resource scope is closed diff --git a/tests/impl/features/rum/scopes/session_test.cpp b/tests/impl/features/rum/scopes/session_test.cpp index 9f573139..8cbaebc2 100644 --- a/tests/impl/features/rum/scopes/session_test.cpp +++ b/tests/impl/features/rum/scopes/session_test.cpp @@ -21,6 +21,7 @@ #include "datadog/impl/platform/system_info.hpp" #include "mock/clock.hpp" +#include "mock/system_info.hpp" #include "support/rum_event_capture.hpp" using namespace datadog; @@ -32,12 +33,16 @@ class SessionFixture { static constexpr const char* SESSION_ID = "5e551017-4114-4114-4114-beeeefbeeeef"; MockClock clock; + MockSystemInfo system_info; RumConfig config; RumScopeDependencies deps; RumApplicationScope parent; RumSessionScope scope; + CoreContext context; + EventWriter writer; + public: SessionFixture(bool is_session_sampled = true) : config(APPLICATION_ID), @@ -54,7 +59,13 @@ class SessionFixture { std::chrono::milliseconds{1700000000000} )}, std::nullopt - ) { + ), + context( + CoreConfig{"test-token", "test-service", "test-env"}, + system_info.os_info, + system_info.device_info + ), + writer([](Block, Block) { return true; }) { clock.FreezeAtMilliseconds(1700000000000); } @@ -77,7 +88,7 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { // When we process any user interaction RumScopeResult result = - scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); // Then the command is processed and the session scope remains open REQUIRE(result == RumScopeResult::RemainOpen); @@ -91,7 +102,7 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { // When we process any user interaction after 7 minutes clock.Tick(std::chrono::minutes(7)); RumScopeResult result = - scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); // Then the scope is still open, as 7m does not exceed our inactivity timeout REQUIRE(result == RumScopeResult::RemainOpen); @@ -99,7 +110,8 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { // Next: When we process any user interaction 14 minutes thereafter clock.Tick(std::chrono::minutes(14)); - result = scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + result = + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); // Then the result is the same, as 14m does not exceed our timeout either, and our // previous command refreshed the last-interaction timestamp @@ -108,7 +120,8 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { // Next: When we wait a full 16 minutes before processing the next user interaction clock.Tick(std::chrono::minutes(16)); - result = scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + result = + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); // Then our session scope is closed and the command is not handled REQUIRE(result == RumScopeResult::Close); @@ -126,14 +139,17 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { clock.Tick(std::chrono::minutes(10)); // Then every such interaction is accepted and keeps the session open - const auto result = scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + const auto result = scope.Process( + RumCommand::StopAction(GetBaseParams(), "foo"), context, writer + ); REQUIRE(result == RumScopeResult::RemainOpen); REQUIRE(scope.GetEndReason() == std::nullopt); } // Next: When we advance time to T+4h01m and process another user interaction clock.Tick(std::chrono::minutes(11)); - const auto result = scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + const auto result = + scope.Process(RumCommand::StopAction(GetBaseParams(), "foo"), context, writer); // Then our session scope is closed and the command is not handled REQUIRE(result == RumScopeResult::Close); @@ -142,7 +158,8 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { SECTION("M close with Stopped W StopSession is processed") { // When we process StopSession - const auto result = scope.Process(RumCommand::StopSession(GetBaseParams())); + const auto result = + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); // Then the scope is explicitly closed REQUIRE(result == RumScopeResult::Close); @@ -154,7 +171,8 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { ) { // When we process StopSession after 15m+ of inactivity clock.Tick(std::chrono::minutes(16)); - const auto result = scope.Process(RumCommand::StopSession(GetBaseParams())); + const auto result = + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); // Then the scope is closed, but the inactivity takes precendence over the explicit // stop call @@ -170,12 +188,15 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { // When we process StopSession at T+4h01m since session start for (int i = 1; i <= 23; i++) { clock.Tick(std::chrono::minutes(10)); - const auto result = scope.Process(RumCommand::StopAction(GetBaseParams(), "foo")); + const auto result = scope.Process( + RumCommand::StopAction(GetBaseParams(), "foo"), context, writer + ); REQUIRE(result == RumScopeResult::RemainOpen); REQUIRE(scope.GetEndReason() == std::nullopt); } clock.Tick(std::chrono::minutes(11)); - const auto result = scope.Process(RumCommand::StopSession(GetBaseParams())); + const auto result = + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); // Then the scope is closed, but the excessive duration takes precendence over the // explicit stop call @@ -188,8 +209,9 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { REQUIRE(scope.GetActiveView() == std::nullopt); // When we process StartView - const auto result = - scope.Process(RumCommand::StartView(GetBaseParams(), "view-a", "View A")); + const auto result = scope.Process( + RumCommand::StartView(GetBaseParams(), "view-a", "View A"), context, writer + ); // Then the session has an active view scope that reflects the parameters configured // in the command @@ -206,13 +228,16 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { SECTION("M start new view W StartView is processed while another view is active") { // Given a session with an active view A REQUIRE(scope.GetActiveView() == std::nullopt); - scope.Process(RumCommand::StartView(GetBaseParams(), "view-a", "View A")); + scope.Process( + RumCommand::StartView(GetBaseParams(), "view-a", "View A"), context, writer + ); REQUIRE(scope.GetActiveView()->get().GetKey() == "view-a"); const UUID view_a_id = scope.GetActiveView()->get().GetViewID(); // When we create another view B in response to a StartView command - const auto result = - scope.Process(RumCommand::StartView(GetBaseParams(), "view-b", "View B")); + const auto result = scope.Process( + RumCommand::StartView(GetBaseParams(), "view-b", "View B"), context, writer + ); // Then the session's active view is now B REQUIRE(result == RumScopeResult::RemainOpen); @@ -228,13 +253,16 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { SECTION("M start new view W StartView has same key as active view") { // Given a session with an active view with key 'view-a' REQUIRE(scope.GetActiveView() == std::nullopt); - scope.Process(RumCommand::StartView(GetBaseParams(), "view-a", "View A")); + scope.Process( + RumCommand::StartView(GetBaseParams(), "view-a", "View A"), context, writer + ); REQUIRE(scope.GetActiveView()->get().GetKey() == "view-a"); const UUID initial_view_id = scope.GetActiveView()->get().GetViewID(); // When we create another view, also 'view-a', in response to a StartView command - const auto result = - scope.Process(RumCommand::StartView(GetBaseParams(), "view-a", "View A")); + const auto result = scope.Process( + RumCommand::StartView(GetBaseParams(), "view-a", "View A"), context, writer + ); // Then the session's active view changes to the new scope REQUIRE(result == RumScopeResult::RemainOpen); @@ -249,11 +277,14 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { SECTION("M deactivate active view W StopView has matching key") { // Given a session with 'view-a' active - scope.Process(RumCommand::StartView(GetBaseParams(), "view-a", "View A")); + scope.Process( + RumCommand::StartView(GetBaseParams(), "view-a", "View A"), context, writer + ); REQUIRE(scope.GetActiveView()->get().GetKey() == "view-a"); // When we process a StopView command that targets 'view-a' - const auto result = scope.Process(RumCommand::StopView(GetBaseParams(), "view-a")); + const auto result = + scope.Process(RumCommand::StopView(GetBaseParams(), "view-a"), context, writer); // Then our session remains open, but it no longer has an active view REQUIRE(result == RumScopeResult::RemainOpen); @@ -262,12 +293,15 @@ TEST_CASE_METHOD(SessionFixture, "RumSessionScope::Process", "[unit][rum]") { SECTION("M do nothing W StopView does not target any existing view") { // Given a session with 'view-a' active - scope.Process(RumCommand::StartView(GetBaseParams(), "view-a", "View A")); + scope.Process( + RumCommand::StartView(GetBaseParams(), "view-a", "View A"), context, writer + ); REQUIRE(scope.GetActiveView()->get().GetKey() == "view-a"); const UUID initial_view_id = scope.GetActiveView()->get().GetViewID(); // When we process a StopView command that targets 'view-b' - const auto result = scope.Process(RumCommand::StopView(GetBaseParams(), "view-b")); + const auto result = + scope.Process(RumCommand::StopView(GetBaseParams(), "view-b"), context, writer); // Then our original view scope remains in place: nothing changes REQUIRE(result == RumScopeResult::RemainOpen); @@ -288,7 +322,9 @@ TEST_CASE_METHOD( REQUIRE(scope.GetActiveView() == std::nullopt); // When we handle a StartView call - auto result = scope.Process(RumCommand::StartView(GetBaseParams(), "foo", "")); + auto result = scope.Process( + RumCommand::StartView(GetBaseParams(), "foo", ""), context, writer + ); // The session scope does not create a new view or otherwise update its state, as it // doesn't need to send any events @@ -304,17 +340,23 @@ TEST_CASE_METHOD( // When we wait 10 minutes between StartView commands, the session remains open clock.Tick(std::chrono::minutes(10)); - auto result = scope.Process(RumCommand::StartView(GetBaseParams(), "foo", "")); + auto result = scope.Process( + RumCommand::StartView(GetBaseParams(), "foo", ""), context, writer + ); REQUIRE(result == RumScopeResult::RemainOpen); REQUIRE(scope.GetEndReason() == std::nullopt); clock.Tick(std::chrono::minutes(10)); - result = scope.Process(RumCommand::StartView(GetBaseParams(), "foo", "")); + result = scope.Process( + RumCommand::StartView(GetBaseParams(), "foo", ""), context, writer + ); REQUIRE(result == RumScopeResult::RemainOpen); REQUIRE(scope.GetEndReason() == std::nullopt); // Next: When we wait 16 minutes and process another StartView clock.Tick(std::chrono::minutes(16)); - result = scope.Process(RumCommand::StartView(GetBaseParams(), "foo", "")); + result = scope.Process( + RumCommand::StartView(GetBaseParams(), "foo", ""), context, writer + ); // Then the session is closed due to user inactivity REQUIRE(result == RumScopeResult::Close); @@ -334,14 +376,18 @@ TEST_CASE_METHOD( clock.Tick(std::chrono::minutes(10)); // Then every such interaction is accepted and keeps the session open - auto result = scope.Process(RumCommand::StartView(GetBaseParams(), "foo", "")); + auto result = scope.Process( + RumCommand::StartView(GetBaseParams(), "foo", ""), context, writer + ); REQUIRE(result == RumScopeResult::RemainOpen); REQUIRE(scope.GetEndReason() == std::nullopt); } // Next: When we advance time to T+4h01m and process another user interaction clock.Tick(std::chrono::minutes(11)); - auto result = scope.Process(RumCommand::StartView(GetBaseParams(), "foo", "")); + auto result = scope.Process( + RumCommand::StartView(GetBaseParams(), "foo", ""), context, writer + ); // Then the session is closed due to excessive duration REQUIRE(result == RumScopeResult::Close); @@ -355,7 +401,8 @@ TEST_CASE_METHOD( REQUIRE(scope.GetEndReason() == std::nullopt); // When we handle a StopSession call - auto result = scope.Process(RumCommand::StopSession(GetBaseParams())); + auto result = + scope.Process(RumCommand::StopSession(GetBaseParams()), context, writer); // Then the session is closed as usual: the command is heeded even though the // session isn't sampled @@ -384,6 +431,12 @@ class SessionEventFixture { RumEventCapture event_capture; public: + CoreContext GetTestContext() { return event_capture.GetFeatureScope().GetContext(); } + EventWriter GetTestWriter() { + return [this](Block event, Block metadata) { + return event_capture.GetFeatureScope().WriteEvent(event, metadata); + }; + } SessionEventFixture() : config(APPLICATION_ID), deps(config, clock), @@ -401,7 +454,6 @@ class SessionEventFixture { std::nullopt ), event_capture(APPLICATION_ID, SESSION_ID, nullptr) { - deps.scope = &event_capture.GetFeatureScope(); deps.diagnostic_logger = event_capture.GetFeatureScope().diagnostic_logger; clock.FreezeAtMilliseconds(1700000000000); } @@ -413,7 +465,11 @@ class SessionEventFixture { void StartView( std::string_view key = "my-view-key", std::string_view name = "My View" ) { - scope.Process(RumCommand::StartView(GetBaseParams(), key, name)); + scope.Process( + RumCommand::StartView(GetBaseParams(), key, name), + GetTestContext(), + GetTestWriter() + ); } }; @@ -424,7 +480,11 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // When we process a StartFeatureOperation command scope.Process( - RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt) + RumCommand::StartFeatureOperation( + GetBaseParams(), "checkout", std::nullopt, vital_id + ), + GetTestContext(), + GetTestWriter() ); // Then a vital event is emitted @@ -451,14 +511,20 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Given an active session with a view and an active operation StartView(); scope.Process( - RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt) + RumCommand::StartFeatureOperation( + GetBaseParams(), "checkout", std::nullopt, start_vital_id + ), + GetTestContext(), + GetTestWriter() ); // When we stop the operation successfully scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, std::nullopt - ) + GetBaseParams(), "checkout", std::nullopt, stop_vital_id, std::nullopt + ), + GetTestContext(), + GetTestWriter() ); // Then two vital events are emitted (start + end) @@ -486,14 +552,24 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Given an active session with a view and an active operation StartView(); scope.Process( - RumCommand::StartFeatureOperation(GetBaseParams(), "upload", std::nullopt) + RumCommand::StartFeatureOperation( + GetBaseParams(), "upload", std::nullopt, start_vital_id + ), + GetTestContext(), + GetTestWriter() ); // When we fail the operation with an error reason scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), "upload", std::nullopt, RumOperationFailureReason::Error - ) + GetBaseParams(), + "upload", + std::nullopt, + stop_vital_id, + RumOperationFailureReason::Error + ), + GetTestContext(), + GetTestWriter() ); // Then the end event includes failure_reason @@ -517,8 +593,10 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] StartView(); scope.Process( RumCommand::StartFeatureOperation( - GetBaseParams(), "checkout", std::string_view{"cart-42"} - ) + GetBaseParams(), "checkout", std::string_view{"cart-42"}, vital_id + ), + GetTestContext(), + GetTestWriter() ); auto vitals = event_capture.Vitals(); @@ -529,13 +607,23 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] SECTION("M emit vital with abandoned failure_reason W abandoned") { StartView(); scope.Process( - RumCommand::StartFeatureOperation(GetBaseParams(), "login", std::nullopt) + RumCommand::StartFeatureOperation( + GetBaseParams(), "login", std::nullopt, start_id + ), + GetTestContext(), + GetTestWriter() ); scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), "login", std::nullopt, RumOperationFailureReason::Abandoned - ) + GetBaseParams(), + "login", + std::nullopt, + stop_id, + RumOperationFailureReason::Abandoned + ), + GetTestContext(), + GetTestWriter() ); auto vitals = event_capture.Vitals(); @@ -556,12 +644,20 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] SECTION("M warn on duplicate start W same operation started twice") { StartView(); scope.Process( - RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt) + RumCommand::StartFeatureOperation( + GetBaseParams(), "checkout", std::nullopt, id1 + ), + GetTestContext(), + GetTestWriter() ); // Start the same operation again scope.Process( - RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt) + RumCommand::StartFeatureOperation( + GetBaseParams(), "checkout", std::nullopt, id2 + ), + GetTestContext(), + GetTestWriter() ); // Both events are emitted (warnings never suppress events) @@ -578,8 +674,10 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] StartView(); scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), "unknown-op", std::nullopt, std::nullopt - ) + GetBaseParams(), "unknown-op", std::nullopt, vital_id, std::nullopt + ), + GetTestContext(), + GetTestWriter() ); // Event is still emitted despite no matching start @@ -598,8 +696,10 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // When we process a StartFeatureOperation command scope.Process( RumCommand::StartFeatureOperation( - GetBaseParams(), "background-op", std::nullopt - ) + GetBaseParams(), "background-op", std::nullopt, vital_id + ), + GetTestContext(), + GetTestWriter() ); // Then a vital event is still emitted with zero-valued view @@ -615,13 +715,17 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Start two operations with same name but different operation_keys scope.Process( RumCommand::StartFeatureOperation( - GetBaseParams(), "upload", std::string_view{"file-1"} - ) + GetBaseParams(), "upload", std::string_view{"file-1"}, UUID::Random() + ), + GetTestContext(), + GetTestWriter() ); scope.Process( RumCommand::StartFeatureOperation( - GetBaseParams(), "upload", std::string_view{"file-2"} - ) + GetBaseParams(), "upload", std::string_view{"file-2"}, UUID::Random() + ), + GetTestContext(), + GetTestWriter() ); // No warnings - these are distinct operations @@ -633,8 +737,14 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Stop one scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), "upload", std::string_view{"file-1"}, std::nullopt - ) + GetBaseParams(), + "upload", + std::string_view{"file-1"}, + UUID::Random(), + std::nullopt + ), + GetTestContext(), + GetTestWriter() ); // No warning for stop-with-matching-start @@ -648,11 +758,17 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Start an operation scope.Process( - RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt) + RumCommand::StartFeatureOperation( + GetBaseParams(), "checkout", std::nullopt, UUID::Random() + ), + GetTestContext(), + GetTestWriter() ); // Stop the session (this clears active operations) - scope.Process(RumCommand::StopSession(GetBaseParams())); + scope.Process( + RumCommand::StopSession(GetBaseParams()), GetTestContext(), GetTestWriter() + ); // The session ended, so the test verifies no crash occurred and state was cleaned // up @@ -669,7 +785,11 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] auto params = RumCommandParams(clock.Now(), {}, cmd_attrs); scope.Process( - RumCommand::StartFeatureOperation(std::move(params), "checkout", std::nullopt) + RumCommand::StartFeatureOperation( + std::move(params), "checkout", std::nullopt, UUID::Random() + ), + GetTestContext(), + GetTestWriter() ); auto vitals = event_capture.Vitals(); @@ -687,7 +807,11 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Given no active view - start event has zero view ID scope.Process( - RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt) + RumCommand::StartFeatureOperation( + GetBaseParams(), "checkout", std::nullopt, UUID::Random() + ), + GetTestContext(), + GetTestWriter() ); auto vitals = event_capture.Vitals(); REQUIRE(vitals.size() == 1); @@ -699,8 +823,10 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // And the operation is stopped scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, std::nullopt - ) + GetBaseParams(), "checkout", std::nullopt, UUID::Random(), std::nullopt + ), + GetTestContext(), + GetTestWriter() ); // Then the stop event captures the current (non-zero) view context @@ -726,14 +852,22 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // When a non-UserInteraction command (operation) is processed scope.Process( - RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt) + RumCommand::StartFeatureOperation( + GetBaseParams(), "checkout", std::nullopt, UUID::Random() + ), + GetTestContext(), + GetTestWriter() ); // And then another minute passes clock.Tick(std::chrono::minutes(2)); // Then the session should expire because operations don't refresh inactivity - auto result = scope.Process(RumCommand::StartView(GetBaseParams(), "foo", "")); + auto result = scope.Process( + RumCommand::StartView(GetBaseParams(), "foo", ""), + GetTestContext(), + GetTestWriter() + ); REQUIRE(result == RumScopeResult::Close); REQUIRE( scope.GetEndReason().value() == @@ -829,9 +963,20 @@ TEST_CASE("RumSessionScope::PopulateContext", "[unit][rum]") { Timestamp{}, std::nullopt ); + MockSystemInfo local_system_info; + CoreContext local_context( + CoreConfig{"test-token", "test-service", "test-env"}, + local_system_info.os_info, + local_system_info.device_info + ); + EventWriter local_writer = [](Block, Block) { return true; }; // When the session ends for any reason - scope.Process(RumCommand::StopSession(RumCommandParams(Timestamp{}, {}, {}))); + scope.Process( + RumCommand::StopSession(RumCommandParams(Timestamp{}, {}, {})), + local_context, + local_writer + ); // And we then populate a RumContext from the session scope RumContext ctx; diff --git a/tests/impl/features/rum/scopes/view_test.cpp b/tests/impl/features/rum/scopes/view_test.cpp index 536e5a7f..3354e698 100644 --- a/tests/impl/features/rum/scopes/view_test.cpp +++ b/tests/impl/features/rum/scopes/view_test.cpp @@ -43,6 +43,12 @@ class ViewFixture { RumEventCapture event_capture; public: + CoreContext GetTestContext() { return event_capture.GetFeatureScope().GetContext(); } + EventWriter GetTestWriter() { + return [this](Block event, Block metadata) { + return event_capture.GetFeatureScope().WriteEvent(event, metadata); + }; + } ViewFixture() : config(APPLICATION_ID), deps(config, clock), @@ -71,7 +77,6 @@ class ViewFixture { )} ), event_capture(APPLICATION_ID, SESSION_ID, VIEW_ID) { - deps.scope = &event_capture.GetFeatureScope(); deps.diagnostic_logger = event_capture.GetFeatureScope().diagnostic_logger; clock.FreezeAtMilliseconds(1700000000000); } @@ -85,7 +90,9 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { REQUIRE(scope.IsActive() == true); // When we process StopSession - const auto result = scope.Process(RumCommand::StopSession(GetBaseParams())); + const auto result = scope.Process( + RumCommand::StopSession(GetBaseParams()), GetTestContext(), GetTestWriter() + ); // Then the view scope is rendered inactive REQUIRE(scope.IsActive() == false); @@ -106,7 +113,9 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { ) { // When we process an initial StartView command whose key matches ours const auto result = scope.Process( - RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name") + RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name"), + GetTestContext(), + GetTestWriter() ); // Then the view remains active and open @@ -126,7 +135,9 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { ) { // Given a view scope that's already seen an initial StartView command auto result = scope.Process( - RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name") + RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name"), + GetTestContext(), + GetTestWriter() ); REQUIRE(scope.IsActive() == true); REQUIRE(result == RumScopeResult::RemainOpen); @@ -134,7 +145,9 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { // When we process another StartView command whose key is the same result = scope.Process( - RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name") + RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name"), + GetTestContext(), + GetTestWriter() ); // Then the original view scope is deactivated (and in this case, with no pending @@ -158,7 +171,9 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { ) { // Given an active view scope with 'my-view-key' auto result = scope.Process( - RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name") + RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name"), + GetTestContext(), + GetTestWriter() ); REQUIRE(scope.IsActive() == true); REQUIRE(result == RumScopeResult::RemainOpen); @@ -166,7 +181,9 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { // When we process a StartView command with a different view key result = scope.Process( - RumCommand::StartView(GetBaseParams(), "a-different-view-key", "Different!") + RumCommand::StartView(GetBaseParams(), "a-different-view-key", "Different!"), + GetTestContext(), + GetTestWriter() ); // Then our original scope is deactivated (and closed), as the newly-created view @@ -189,7 +206,9 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { // When we process a StartView command with a different view key auto result = scope.Process( - RumCommand::StartView(GetBaseParams(), "a-different-view-key", "Different!") + RumCommand::StartView(GetBaseParams(), "a-different-view-key", "Different!"), + GetTestContext(), + GetTestWriter() ); // Then our original scope is deactivated and closed, irrespective of whether it @@ -207,14 +226,20 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { SECTION("M deactivate and send a final event W StopView has matching key") { // Given an active view scope with 'my-view-key' auto result = scope.Process( - RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name") + RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name"), + GetTestContext(), + GetTestWriter() ); REQUIRE(scope.IsActive() == true); REQUIRE(result == RumScopeResult::RemainOpen); REQUIRE(event_capture.Views().size() == 1); // When we process a StopView command that matches 'my-view-key' - result = scope.Process(RumCommand::StopView(GetBaseParams(), "my-view-key")); + result = scope.Process( + RumCommand::StopView(GetBaseParams(), "my-view-key"), + GetTestContext(), + GetTestWriter() + ); // Then our scope is deactivated (and closed), and it generates an event on close REQUIRE(scope.IsActive() == false); @@ -229,14 +254,20 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { SECTION("M do nothing W StopView has non-matching key") { // Given an active view scope with 'my-view-key' auto result = scope.Process( - RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name") + RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name"), + GetTestContext(), + GetTestWriter() ); REQUIRE(scope.IsActive() == true); REQUIRE(result == RumScopeResult::RemainOpen); REQUIRE(event_capture.Views().size() == 1); // When we process a StopView command that targets a different view key - result = scope.Process(RumCommand::StopView(GetBaseParams(), "different-key")); + result = scope.Process( + RumCommand::StopView(GetBaseParams(), "different-key"), + GetTestContext(), + GetTestWriter() + ); // Then our scope remains active and open, and it sends no additional events REQUIRE(scope.IsActive() == true); @@ -250,7 +281,9 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { SECTION("M remain open until pending resources finish W StopView called") { // Given an active view scope with 'my-view-key' auto result = scope.Process( - RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name") + RumCommand::StartView(GetBaseParams(), "my-view-key", "My View Name"), + GetTestContext(), + GetTestWriter() ); REQUIRE(scope.IsActive() == true); REQUIRE(result == RumScopeResult::RemainOpen); @@ -262,13 +295,19 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { GetBaseParams(), "my-resource-key", RumRequestDetails{RumResourceMethod::Get, "/foo"} - ) + ), + GetTestContext(), + GetTestWriter() ); REQUIRE(result == RumScopeResult::RemainOpen); REQUIRE(event_capture.Views().size() == 1); // When we process a StopView command that matches 'my-view-key' - result = scope.Process(RumCommand::StopView(GetBaseParams(), "my-view-key")); + result = scope.Process( + RumCommand::StopView(GetBaseParams(), "my-view-key"), + GetTestContext(), + GetTestWriter() + ); // Then our scope is deactivated, and it generates an event, but: // - the scope remains open due to the pending resource @@ -284,8 +323,11 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { // Next: when we provide the same (now-inactive but still-open) RumViewScope with a // command that stops our pending resource - result = - scope.Process(RumCommand::StopResource(GetBaseParams(), "my-resource-key")); + result = scope.Process( + RumCommand::StopResource(GetBaseParams(), "my-resource-key"), + GetTestContext(), + GetTestWriter() + ); // Then our scope is finally closed, and it sends a final event that records the // final state of the view with is_active == false and 1 completed resource @@ -325,8 +367,16 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { // Given a specific type of command that affects view lifecycle DYNAMIC_SECTION(" {" << tt.name << "}") { // And a scope that's been rendered inactive by a StopView call - scope.Process(RumCommand::StartView(GetBaseParams(), "my-view-key", "")); - scope.Process(RumCommand::StopView(GetBaseParams(), "my-view-key")); + scope.Process( + RumCommand::StartView(GetBaseParams(), "my-view-key", ""), + GetTestContext(), + GetTestWriter() + ); + scope.Process( + RumCommand::StopView(GetBaseParams(), "my-view-key"), + GetTestContext(), + GetTestWriter() + ); REQUIRE(scope.IsActive() == false); auto views = event_capture.Views(); REQUIRE(views.size() == 2); @@ -336,7 +386,7 @@ TEST_CASE_METHOD(ViewFixture, "RumSessionScope::Process", "[unit][rum]") { // When we process a command that would ordinarily affect this view's lifecycle // and cause it to generate an event RumCommand cmd = tt.cmd_thunk(); - auto result = scope.Process(cmd); + auto result = scope.Process(cmd, GetTestContext(), GetTestWriter()); // Then the command is ignored, and no new events are generated, because the // view is already inactive From 01af16126411417c55e4ee6fcd417b93da4c4685 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Fri, 6 Mar 2026 14:32:35 -0500 Subject: [PATCH 05/25] fix: Make FeatureScope creation explicit about test-only mode --- src/datadog/impl/core/core.cpp | 14 ++-- src/datadog/impl/core/feature_scope.cpp | 50 +++++++++++++-- src/datadog/impl/core/feature_scope.hpp | 80 ++++++++++++++++++----- tests/impl/core/feature_scope_test.cpp | 6 +- tests/support/feature.hpp | 12 ++-- tests/support/rum_event_capture.hpp | 85 +++++++++++++------------ 6 files changed, 169 insertions(+), 78 deletions(-) diff --git a/src/datadog/impl/core/core.cpp b/src/datadog/impl/core/core.cpp index 86876898..45e8084f 100644 --- a/src/datadog/impl/core/core.cpp +++ b/src/datadog/impl/core/core.cpp @@ -378,12 +378,14 @@ bool Core::Start() { [this, id](Block event, Block event_metadata) -> bool { return EnqueueStorageWrite(id, event, event_metadata); }; - feature.impl->OnCoreStarted(FeatureScope( - *_context_provider, - event_generated_func, - _diagnostic_logger, - _context_queue.get() - )); + feature.impl->OnCoreStarted( + FeatureScope::Create( + *_context_provider, + event_generated_func, + _diagnostic_logger, + *_context_queue + ) + ); } return true; } diff --git a/src/datadog/impl/core/feature_scope.cpp b/src/datadog/impl/core/feature_scope.cpp index ca64399e..0f5552a3 100644 --- a/src/datadog/impl/core/feature_scope.cpp +++ b/src/datadog/impl/core/feature_scope.cpp @@ -14,12 +14,49 @@ FeatureScope::FeatureScope( CoreContextProvider& context_provider, const EventGeneratedFunc& event_generated_func, const DiagnosticLogger& in_diagnostic_logger, + FeatureScope::ExecutionMode mode, Queue>* context_queue ) : _context_provider(&context_provider), _event_generated_func(event_generated_func), + _mode(mode), _context_queue(context_queue), - diagnostic_logger(in_diagnostic_logger) {} + 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" + ); +} + +FeatureScope FeatureScope::Create( + CoreContextProvider& context_provider, + const EventGeneratedFunc& event_generated_func, + const DiagnosticLogger& diagnostic_logger, + Queue>& context_queue +) { + return FeatureScope( + context_provider, + event_generated_func, + diagnostic_logger, + FeatureScope::ExecutionMode::OnContextThread, + &context_queue + ); +} + +FeatureScope FeatureScope::CreateForTesting( + CoreContextProvider& context_provider, + const EventGeneratedFunc& event_generated_func, + const DiagnosticLogger& diagnostic_logger +) { + return FeatureScope( + context_provider, + event_generated_func, + diagnostic_logger, + FeatureScope::ExecutionMode::Synchronous, + nullptr + ); +} CoreContext FeatureScope::GetContext() const { DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); @@ -39,20 +76,21 @@ bool FeatureScope::WriteEvent(Block event, Block event_metadata) const { } void FeatureScope::ExecuteOnContextThread(const ContextThreadFunc& func) { - // If no context queue is available (testing mode), execute synchronously - if (_context_queue == nullptr) { + 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; } - // Queue a thunk that will execute on the context thread + // 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"); - // Get a CoreContext snapshot at execution time const CoreContext context = _context_provider->Get(); - // Invoke the user's function with the context and event writer func(context, _event_generated_func); }); } diff --git a/src/datadog/impl/core/feature_scope.hpp b/src/datadog/impl/core/feature_scope.hpp index 9b1da3f1..dbbf5619 100644 --- a/src/datadog/impl/core/feature_scope.hpp +++ b/src/datadog/impl/core/feature_scope.hpp @@ -6,6 +6,7 @@ #pragma once +#include #include #include "datadog/impl/core/block.hpp" @@ -48,33 +49,74 @@ using ContextThreadFunc = */ class FeatureScope { private: + /** + * Determines how functions passed to OnContextThread will be evaluated. In + * production usage, mode is always OnContextThread and _context_queue is always + * non-null. + */ + enum class ExecutionMode : uint8_t { + OnContextThread, // Used in production: functions are queued for async execution + Synchronous, // Used for testing: functions are executed synchronously + }; + CoreContextProvider* _context_provider; EventGeneratedFunc _event_generated_func; + ExecutionMode _mode; Queue>* _context_queue; - public: - DiagnosticLogger diagnostic_logger; - + private: /** - * Initializes a FeatureScope for a single feature, given the state necessary to - * connect it to the Core. - * - * @param context_queue Optional pointer to the context queue. If null, operations - * will execute synchronously (used in tests). If non-null, operations will be - * queued for asynchronous execution on the context thread. + * Private constructor. Use Create() or CreateForTesting() factory methods. */ explicit FeatureScope( CoreContextProvider& context_provider, const EventGeneratedFunc& event_generated_func, const DiagnosticLogger& in_diagnostic_logger, - Queue>* context_queue = nullptr + ExecutionMode mode, + Queue>* context_queue ); + public: + DiagnosticLogger diagnostic_logger; + // Noncopyable, but movable (so ownership can be transferred to Feature from Core) FeatureScope(const FeatureScope&) = delete; FeatureScope& operator=(const FeatureScope&) = delete; FeatureScope(FeatureScope&&) noexcept = default; FeatureScope& operator=(FeatureScope&&) noexcept = default; + ~FeatureScope() = default; // Add this line + + /** + * Creates a FeatureScope for production use, where ExecuteOnContextThread operations + * are enqueued on the provided `context_queue` for async execution on the SDK's + * context thread. + * + * @param context_provider Provides thread-safe access to CoreContext + * @param event_generated_func Callback invoked when features generate events + * @param diagnostic_logger Logger for diagnostic messages + * @param context_queue Queue containing thunks to be executed serially by the context + * thread; must outlive the FeatureScope + */ + static FeatureScope Create( + CoreContextProvider& context_provider, + const EventGeneratedFunc& event_generated_func, + const DiagnosticLogger& diagnostic_logger, + Queue>& context_queue + ); + + /** + * Creates a FeatureScope for use in unit tests, where ExecuteOnContextThread + * operations are executed synchronously on the calling thread. + * + * @param context_provider Provides thread-safe access to CoreContext + * @param event_generated_func Callback invoked when features generate events + * @param diagnostic_logger Logger for diagnostic messages + */ + static FeatureScope CreateForTesting( + CoreContextProvider& context_provider, + const EventGeneratedFunc& event_generated_func, + const DiagnosticLogger& diagnostic_logger + ); /** * Creates an immutable, thread-safe copy of the CoreContext, which contains all the @@ -124,15 +166,19 @@ class FeatureScope { bool WriteEvent(Block event, Block event_metadata) const; /** - * Executes a function on the context thread, providing it with a CoreContext snapshot - * and an EventWriter for generating events. + * Executes a function on the context thread, providing it with a CoreContext + * snapshot and an EventWriter for generating events. + * + * Given a function that accepts an immutable snapshot of the CoreContext (taken at + * execution time) and a callback that can be used to produce events, ensures that the + * function is executed at the appropriate time. * - * If no context queue is configured (testing mode), executes the function - * synchronously on the calling thread. + * In production usage, where _mode == OnContextThread, the function will be pushed + * onto the context queue, which the context thread processes serially until SDK + * shutdown. * - * The function receives: - * - A const reference to a CoreContext snapshot (taken at execution time) - * - An EventWriter callback that can be used to generate events + * If initialized with FeatureScope::CreateForTesting(), where mode == Synchronous, + * the function will be immediately executed on the calling thread. */ void ExecuteOnContextThread(const ContextThreadFunc& func); }; diff --git a/tests/impl/core/feature_scope_test.cpp b/tests/impl/core/feature_scope_test.cpp index 3cfcf505..da6f1be2 100644 --- a/tests/impl/core/feature_scope_test.cpp +++ b/tests/impl/core/feature_scope_test.cpp @@ -102,21 +102,21 @@ TEST_CASE("FeatureScope thread safety", "[unit][core][thread-safety]") { FeatureState feature_c; // And a separate scope for each feature, initialized from the same context provider - FeatureScope scope_a( + FeatureScope scope_a = FeatureScope::CreateForTesting( context_provider, [&](Block event, Block event_metadata) { return feature_a.HandleEvent(event, event_metadata); }, DiagnosticLogger{} ); - FeatureScope scope_b( + FeatureScope scope_b = FeatureScope::CreateForTesting( context_provider, [&](Block event, Block event_metadata) { return feature_b.HandleEvent(event, event_metadata); }, DiagnosticLogger{} ); - FeatureScope scope_c( + FeatureScope scope_c = FeatureScope::CreateForTesting( context_provider, [&](Block event, Block event_metadata) { return feature_c.HandleEvent(event, event_metadata); diff --git a/tests/support/feature.hpp b/tests/support/feature.hpp index 58ba9de1..4c1f27dd 100644 --- a/tests/support/feature.hpp +++ b/tests/support/feature.hpp @@ -54,11 +54,13 @@ class FeatureTest { auto diagnostic_handler = [this](const DiagnosticMessage& message) { messages.emplace_back(message); }; - feature->OnCoreStarted(FeatureScope( - _context_provider, - event_generated_func, - DiagnosticLogger(diagnostic_handler, DiagnosticLevel::Debug) - )); + feature->OnCoreStarted( + FeatureScope::CreateForTesting( + _context_provider, + event_generated_func, + DiagnosticLogger(diagnostic_handler, DiagnosticLevel::Debug) + ) + ); } void Stop(const std::shared_ptr& feature) { feature->OnCoreStopping(); } diff --git a/tests/support/rum_event_capture.hpp b/tests/support/rum_event_capture.hpp index c97eafd9..040c2af6 100644 --- a/tests/support/rum_event_capture.hpp +++ b/tests/support/rum_event_capture.hpp @@ -77,49 +77,52 @@ class RumEventCapture { device_info )), feature_scope( - context_provider, - [this](Block event, Block event_metadata) { - // RUM implementation doesn't produce events with metadata - REQUIRE(event_metadata.empty()); - - // Require valid JSON object - auto obj = nlohmann::json::parse(event); - REQUIRE(obj.is_object()); - - // Validate IDs based on event type - if (obj.contains("application") && obj["application"].contains("id")) { - REQUIRE(obj["application"]["id"] == this->application_id); - } - if (obj.contains("session") && obj["session"].contains("id")) { - REQUIRE(obj["session"]["id"] == this->session_id); - } - if (this->view_id != nullptr && obj.contains("view") && - obj["view"].contains("id")) { - REQUIRE(obj["view"]["id"] == this->view_id); - } - - // Capture all events - all_events.emplace_back(std::move(obj)); - return true; - }, - DiagnosticLogger( - [&](const DiagnosticMessage& message) { - switch (message.level) { - case DiagnosticLevel::Debug: - diagnostics.debug.emplace_back(message.text); - break; - case DiagnosticLevel::Status: - diagnostics.status.emplace_back(message.text); - break; - case DiagnosticLevel::Warning: - diagnostics.warning.emplace_back(message.text); - break; - case DiagnosticLevel::Error: - diagnostics.error.emplace_back(message.text); - break; + FeatureScope::CreateForTesting( + context_provider, + [this](Block event, Block event_metadata) { + // RUM implementation doesn't produce events with metadata + REQUIRE(event_metadata.empty()); + + // Require valid JSON object + auto obj = nlohmann::json::parse(event); + REQUIRE(obj.is_object()); + + // Validate IDs based on event type + if (obj.contains("application") && + obj["application"].contains("id")) { + REQUIRE(obj["application"]["id"] == this->application_id); } + if (obj.contains("session") && obj["session"].contains("id")) { + REQUIRE(obj["session"]["id"] == this->session_id); + } + if (this->view_id != nullptr && obj.contains("view") && + obj["view"].contains("id")) { + REQUIRE(obj["view"]["id"] == this->view_id); + } + + // Capture all events + all_events.emplace_back(std::move(obj)); + return true; }, - DiagnosticLevel::Debug + DiagnosticLogger( + [&](const DiagnosticMessage& message) { + switch (message.level) { + case DiagnosticLevel::Debug: + diagnostics.debug.emplace_back(message.text); + break; + case DiagnosticLevel::Status: + diagnostics.status.emplace_back(message.text); + break; + case DiagnosticLevel::Warning: + diagnostics.warning.emplace_back(message.text); + break; + case DiagnosticLevel::Error: + diagnostics.error.emplace_back(message.text); + break; + } + }, + DiagnosticLevel::Debug + ) ) ) {} From 3ad3c533a70901b9ca9eba8948dfc418e631aab8 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Fri, 6 Mar 2026 15:13:36 -0500 Subject: [PATCH 06/25] fix: Make UpdateContext queue context mutations for async execution --- src/datadog/impl/core/feature_scope.cpp | 16 +++++++++++++++- src/datadog/impl/core/feature_scope.hpp | 6 +++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/datadog/impl/core/feature_scope.cpp b/src/datadog/impl/core/feature_scope.cpp index 0f5552a3..2b9d01e1 100644 --- a/src/datadog/impl/core/feature_scope.cpp +++ b/src/datadog/impl/core/feature_scope.cpp @@ -65,7 +65,21 @@ CoreContext FeatureScope::GetContext() const { void FeatureScope::UpdateContext(const std::function& 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); + }); } bool FeatureScope::WriteEvent(Block event, Block event_metadata) const { diff --git a/src/datadog/impl/core/feature_scope.hpp b/src/datadog/impl/core/feature_scope.hpp index dbbf5619..6382dff0 100644 --- a/src/datadog/impl/core/feature_scope.hpp +++ b/src/datadog/impl/core/feature_scope.hpp @@ -153,7 +153,11 @@ class FeatureScope { * By convention, a feature should only modify the member(s) of CoreContext that it * exclusively owns: e.g. the RUM feature modifies the `RumFeatureContext` value, etc. * - * NOTE: CoreContext is modified synchronously. This may change in the future. + * In production usage, where _mode == OnContextThread, the update will be pushed + * onto the context queue for async execution on the context thread. + * + * If initialized with FeatureScope::CreateForTesting(), where mode == Synchronous, + * the update will be immediately executed on the calling thread. */ void UpdateContext(const std::function& callback); From dc4db37930194e790f8957b4149c176825ca15fd Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Fri, 6 Mar 2026 16:12:01 -0500 Subject: [PATCH 07/25] chore: Fix comment to avoid "feature operation" confusion --- src/datadog/impl/core/core.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/datadog/impl/core/core.cpp b/src/datadog/impl/core/core.cpp index 45e8084f..946a550f 100644 --- a/src/datadog/impl/core/core.cpp +++ b/src/datadog/impl/core/core.cpp @@ -420,8 +420,8 @@ void Core::Stop() { ); // Stop the context queue, then block until the context thread drains the queue and - // exits. This ensures all pending feature operations complete before we stop the - // storage thread. + // exits. This ensures all pending feature work completes before we stop the storage + // thread. _context_queue->Stop(); if (_context_thread) { _diagnostic_logger.Debug("Joining on context thread"); From 77d66412a7c60d0c2647254113da9c5f6630c158 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Fri, 6 Mar 2026 16:13:04 -0500 Subject: [PATCH 08/25] chore: Remove `Core::FlushContextQueue`; it's not actually used --- src/datadog/impl/core/core.cpp | 25 ------------------------- src/datadog/impl/core/core.hpp | 8 -------- 2 files changed, 33 deletions(-) diff --git a/src/datadog/impl/core/core.cpp b/src/datadog/impl/core/core.cpp index 946a550f..957dc818 100644 --- a/src/datadog/impl/core/core.cpp +++ b/src/datadog/impl/core/core.cpp @@ -7,10 +7,8 @@ #include "datadog/impl/core/core.hpp" #include -#include #include #include -#include #include #include "datadog/impl/assert.hpp" @@ -532,27 +530,4 @@ std::string_view Core::GetApplicationVersion() const { return _context_provider->GetHttpContext().application_version; } -void Core::FlushContextQueue() { - DATADOG_ASSERT( - _state == CoreState::Started, "FlushContextQueue called while Core not running" - ); - DATADOG_ASSERT(_context_queue, "_context_queue is null on FlushContextQueue"); - - // Use a condition variable to wait until the sentinel function executes - std::mutex mutex; - std::condition_variable cv; - bool sentinel_executed = false; - - // Queue a sentinel function that signals completion - _context_queue->Push([&]() { - std::lock_guard lock(mutex); - sentinel_executed = true; - cv.notify_one(); - }); - - // Wait until the sentinel has been executed by the context thread - std::unique_lock lock(mutex); - cv.wait(lock, [&] { return sentinel_executed; }); -} - } // namespace datadog::impl diff --git a/src/datadog/impl/core/core.hpp b/src/datadog/impl/core/core.hpp index 3535275d..94e33ca0 100644 --- a/src/datadog/impl/core/core.hpp +++ b/src/datadog/impl/core/core.hpp @@ -365,14 +365,6 @@ class Core { */ void Stop(); - /** - * Blocks until all queued functions on the context thread have been processed. - * - * This is intended for testing, to ensure deterministic execution of asynchronous - * operations. Must only be called when the core is running. - */ - void FlushContextQueue(); - private: bool EnqueueStorageWrite(FeatureId feature_id, Block event, Block event_metadata); From 4fcd1929f9acdc0c3547b47b48d3fc53dcc6387d Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Fri, 6 Mar 2026 16:18:20 -0500 Subject: [PATCH 09/25] chore: Update comments --- src/datadog/impl/core/feature_scope.hpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/datadog/impl/core/feature_scope.hpp b/src/datadog/impl/core/feature_scope.hpp index 6382dff0..4a122ba6 100644 --- a/src/datadog/impl/core/feature_scope.hpp +++ b/src/datadog/impl/core/feature_scope.hpp @@ -50,9 +50,8 @@ using ContextThreadFunc = class FeatureScope { private: /** - * Determines how functions passed to OnContextThread will be evaluated. In - * production usage, mode is always OnContextThread and _context_queue is always - * non-null. + * Determines how functions submitted by Features will be executed. In production + * usage, mode is always OnContextThread. */ enum class ExecutionMode : uint8_t { OnContextThread, // Used in production: functions are queued for async execution @@ -87,9 +86,9 @@ class FeatureScope { ~FeatureScope() = default; // Add this line /** - * Creates a FeatureScope for production use, where ExecuteOnContextThread operations - * are enqueued on the provided `context_queue` for async execution on the SDK's - * context thread. + * Creates a FeatureScope for production use, where functions passed to UpdateContext + * and ExecuteOnContextThread are enqueued on the provided `context_queue` for async + * execution on the SDK's context thread. * * @param context_provider Provides thread-safe access to CoreContext * @param event_generated_func Callback invoked when features generate events @@ -105,8 +104,9 @@ class FeatureScope { ); /** - * Creates a FeatureScope for use in unit tests, where ExecuteOnContextThread - * operations are executed synchronously on the calling thread. + * Creates a FeatureScope for use in unit tests, where functions passed to + * UpdateContext and ExecuteOnContextThread are executed synchronously on the calling + * thread. * * @param context_provider Provides thread-safe access to CoreContext * @param event_generated_func Callback invoked when features generate events From f840aef2000860c6ee7d9ab42dc14797961179ec Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Mon, 9 Mar 2026 16:34:12 -0400 Subject: [PATCH 10/25] chore: Remove vital_id references to reconcile --- .../impl/features/rum/scopes/session_test.cpp | 72 ++++++------------- 1 file changed, 20 insertions(+), 52 deletions(-) diff --git a/tests/impl/features/rum/scopes/session_test.cpp b/tests/impl/features/rum/scopes/session_test.cpp index 8cbaebc2..d8ef652f 100644 --- a/tests/impl/features/rum/scopes/session_test.cpp +++ b/tests/impl/features/rum/scopes/session_test.cpp @@ -480,9 +480,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // When we process a StartFeatureOperation command scope.Process( - RumCommand::StartFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, vital_id - ), + RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt), GetTestContext(), GetTestWriter() ); @@ -511,9 +509,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Given an active session with a view and an active operation StartView(); scope.Process( - RumCommand::StartFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, start_vital_id - ), + RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt), GetTestContext(), GetTestWriter() ); @@ -521,7 +517,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // When we stop the operation successfully scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, stop_vital_id, std::nullopt + GetBaseParams(), "checkout", std::nullopt, std::nullopt ), GetTestContext(), GetTestWriter() @@ -552,9 +548,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Given an active session with a view and an active operation StartView(); scope.Process( - RumCommand::StartFeatureOperation( - GetBaseParams(), "upload", std::nullopt, start_vital_id - ), + RumCommand::StartFeatureOperation(GetBaseParams(), "upload", std::nullopt), GetTestContext(), GetTestWriter() ); @@ -562,11 +556,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // When we fail the operation with an error reason scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), - "upload", - std::nullopt, - stop_vital_id, - RumOperationFailureReason::Error + GetBaseParams(), "upload", std::nullopt, RumOperationFailureReason::Error ), GetTestContext(), GetTestWriter() @@ -593,7 +583,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] StartView(); scope.Process( RumCommand::StartFeatureOperation( - GetBaseParams(), "checkout", std::string_view{"cart-42"}, vital_id + GetBaseParams(), "checkout", std::string_view{"cart-42"} ), GetTestContext(), GetTestWriter() @@ -607,20 +597,14 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] SECTION("M emit vital with abandoned failure_reason W abandoned") { StartView(); scope.Process( - RumCommand::StartFeatureOperation( - GetBaseParams(), "login", std::nullopt, start_id - ), + RumCommand::StartFeatureOperation(GetBaseParams(), "login", std::nullopt), GetTestContext(), GetTestWriter() ); scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), - "login", - std::nullopt, - stop_id, - RumOperationFailureReason::Abandoned + GetBaseParams(), "login", std::nullopt, RumOperationFailureReason::Abandoned ), GetTestContext(), GetTestWriter() @@ -644,18 +628,14 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] SECTION("M warn on duplicate start W same operation started twice") { StartView(); scope.Process( - RumCommand::StartFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, id1 - ), + RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt), GetTestContext(), GetTestWriter() ); // Start the same operation again scope.Process( - RumCommand::StartFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, id2 - ), + RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt), GetTestContext(), GetTestWriter() ); @@ -674,7 +654,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] StartView(); scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), "unknown-op", std::nullopt, vital_id, std::nullopt + GetBaseParams(), "unknown-op", std::nullopt, std::nullopt ), GetTestContext(), GetTestWriter() @@ -696,7 +676,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // When we process a StartFeatureOperation command scope.Process( RumCommand::StartFeatureOperation( - GetBaseParams(), "background-op", std::nullopt, vital_id + GetBaseParams(), "background-op", std::nullopt ), GetTestContext(), GetTestWriter() @@ -715,14 +695,14 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Start two operations with same name but different operation_keys scope.Process( RumCommand::StartFeatureOperation( - GetBaseParams(), "upload", std::string_view{"file-1"}, UUID::Random() + GetBaseParams(), "upload", std::string_view{"file-1"} ), GetTestContext(), GetTestWriter() ); scope.Process( RumCommand::StartFeatureOperation( - GetBaseParams(), "upload", std::string_view{"file-2"}, UUID::Random() + GetBaseParams(), "upload", std::string_view{"file-2"} ), GetTestContext(), GetTestWriter() @@ -737,11 +717,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Stop one scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), - "upload", - std::string_view{"file-1"}, - UUID::Random(), - std::nullopt + GetBaseParams(), "upload", std::string_view{"file-1"}, std::nullopt ), GetTestContext(), GetTestWriter() @@ -758,9 +734,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Start an operation scope.Process( - RumCommand::StartFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, UUID::Random() - ), + RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt), GetTestContext(), GetTestWriter() ); @@ -785,9 +759,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] auto params = RumCommandParams(clock.Now(), {}, cmd_attrs); scope.Process( - RumCommand::StartFeatureOperation( - std::move(params), "checkout", std::nullopt, UUID::Random() - ), + RumCommand::StartFeatureOperation(std::move(params), "checkout", std::nullopt), GetTestContext(), GetTestWriter() ); @@ -807,9 +779,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // Given no active view - start event has zero view ID scope.Process( - RumCommand::StartFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, UUID::Random() - ), + RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt), GetTestContext(), GetTestWriter() ); @@ -823,7 +793,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // And the operation is stopped scope.Process( RumCommand::StopFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, UUID::Random(), std::nullopt + GetBaseParams(), "checkout", std::nullopt, std::nullopt ), GetTestContext(), GetTestWriter() @@ -852,9 +822,7 @@ TEST_CASE_METHOD(SessionEventFixture, "RumSessionScope operations", "[unit][rum] // When a non-UserInteraction command (operation) is processed scope.Process( - RumCommand::StartFeatureOperation( - GetBaseParams(), "checkout", std::nullopt, UUID::Random() - ), + RumCommand::StartFeatureOperation(GetBaseParams(), "checkout", std::nullopt), GetTestContext(), GetTestWriter() ); From d5ae4aeb354508f3cac4b5cfb72743a14ab58924 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Mon, 9 Mar 2026 17:45:14 -0400 Subject: [PATCH 11/25] fix: Update Logging implementation to use context thread --- src/datadog/impl/features/logging/logger.hpp | 4 - src/datadog/impl/features/logging/logging.cpp | 231 +++++++++++------- src/datadog/impl/features/logging/logging.hpp | 23 ++ 3 files changed, 169 insertions(+), 89 deletions(-) diff --git a/src/datadog/impl/features/logging/logger.hpp b/src/datadog/impl/features/logging/logger.hpp index 67f0bc47..6f661ddd 100644 --- a/src/datadog/impl/features/logging/logger.hpp +++ b/src/datadog/impl/features/logging/logger.hpp @@ -38,10 +38,6 @@ struct LoggerState { // message is logged; reused between log calls LogEvent event; - // Reusable buffer where the final JSON payload will be encoded prior to being - // copied onto the storage thread's queue - std::vector event_buffer; - explicit LoggerState( const std::optional& in_service_name, const std::optional& in_logger_name, diff --git a/src/datadog/impl/features/logging/logging.cpp b/src/datadog/impl/features/logging/logging.cpp index 74b85033..8e6e8cda 100644 --- a/src/datadog/impl/features/logging/logging.cpp +++ b/src/datadog/impl/features/logging/logging.cpp @@ -24,6 +24,22 @@ namespace datadog::impl { +/** + * Parameters captured synchronously on the caller thread and passed to the context + * thread for async log event processing. + */ +struct LogCommandParams { + Timestamp timestamp; + LogLevel level; + std::string message; + LoggerEnrichmentConfig enrichment; + Attribute global_attributes; + Attribute logger_attributes; + Attribute message_attributes; + std::string service; + std::string logger_name; +}; + Logging::Logging( const platform::IClock& clock, std::string_view service_name, @@ -91,103 +107,148 @@ void Logging::OnLoggerEmit( LogLevel level, std::string_view message, const Attribute& message_attributes -) const { - // Grab a mutable reference to the LogEvent that represents our payload: each Logger - // holds its own mutable LogEvent struct so that multiple OnLoggerEmit calls can build - // and generate log events concurrently, so long as each comes from a different Logger - LogEvent& ev = mut_state.event; - const LoggerState& state = mut_state; - - // Clear the subset of LogEvent members that may change between events but which are - // not unconditionally set - ev.Reset(); - - // Update basic log event properties - ev.status = level; - if (ev.service.empty()) { - ev.service = _default_service_name; - } - ev.date = _clock.Now(); - ev.message = message; - - // Read CoreContext if available, so we can enrich events with additional SDK state - if (_scope) { - // Obtain a read-only copy of the context - const CoreContext ctx = _scope->GetContext(); - - // If we're configured to enrich log events with RUM context, write all RUM IDs into - // the event (UUID::Zero values will be omitted from the JSON payload) - if (enrichment.enable_rum && ctx.rum) { - ev.rum_application_id = ctx.rum->application_id; - ev.rum_session_id = ctx.rum->session_id; - ev.rum_view_id = ctx.rum->view_id; - ev.rum_action_id = ctx.rum->action_id; - } +) { + // Phase A: Capture time-sensitive values synchronously on caller thread - // If we have OS properties, add them to the event payload - if (ctx.os) { - ev.os.value.emplace(ctx.os->name, ctx.os->version, ctx.os->version_major); - if (!ctx.os->build.empty()) { - ev.os.value->build = ctx.os->build; - } - } + // Capture timestamp immediately + Timestamp timestamp = _clock.Now(); - // If we have device properties, add them to the event payload - if (ctx.device) { - RumDeviceProperties& device = ev.device.value.emplace(); - if (!ctx.device->type.empty()) { - DATADOG_ASSERT( - ctx.device->type == "desktop", - "DeviceInfo specifies non-desktop platform; log event serialization code " - "must be updated" - ); - device.type = RumDeviceType::Desktop; - } - device.name = ctx.device->name; - device.model = ctx.device->model; - device.brand = ctx.device->brand; - device.architecture = ctx.device->architecture; - device.locale = ctx.device->locale; - device.time_zone = ctx.device->time_zone; - } - } + // Snapshot logger attributes + Attribute logger_attributes = mut_state.user_attributes.attribute; - // Create a shallow copy of the Logging feature's global attributes, so we don't need - // to hold a lock during event serialization + // Snapshot global attributes with read lock Attribute global_attributes; { std::shared_lock read_lock(_global_attributes_mutex); global_attributes = _global_attributes.attribute; } - // Merge the full set of custom, user-specified attribute values into the LogEvent - // payload's 'user_attributes' member. User attributes are merged into the event at - // top-level, with the following order: - // - // 1. The global attributes set via Logging::AddAttribute - // 2. The logger-level attributes set via Logger::AddAttribute - // 3. The message-level attributes supplied in the log call - // - // If multiple user attributes share the same property name, the value that appears - // later in the list will take precedence. + // Resolve service name + std::string service = + mut_state.event.service.empty() ? _default_service_name : mut_state.event.service; + + // Copy logger name (OmitIfEmpty needs .value access) + std::string logger_name = mut_state.event.logger_name.value.empty() + ? "" + : mut_state.event.logger_name.value; + + // Package into LogCommandParams + LogCommandParams params{ + timestamp, + level, + std::string(message), + enrichment, + global_attributes, + logger_attributes, + message_attributes, + service, + logger_name + }; + + // Dispatch async processing + DispatchAsync(params); +} + +void Logging::DispatchAsync(const LogCommandParams& params) { + // Phase B: Dispatch to context thread with weak_ptr for shutdown safety + + if (!_scope) { + return; + } + + // Store reference to scope to satisfy linter's optional access check + FeatureScope& scope = *_scope; + + // Capture weak_ptr to self for fail-safe shutdown detection + auto weak_logging = + std::weak_ptr(std::static_pointer_cast(shared_from_this())); + + scope.ExecuteOnContextThread( + // NOLINTNEXTLINE(bugprone-exception-escape) + [weak_logging, params](const CoreContext& context, const EventWriter& writer) { + // Check if Logging object still alive + auto logging = weak_logging.lock(); + if (!logging) { + // Logging destroyed during shutdown, exit gracefully + return; + } + + // Safe to proceed - process the log event + logging->ProcessLogEvent(params, context, writer); + } + ); +} + +void Logging::ProcessLogEvent( + const LogCommandParams& params, + const CoreContext& context, + const EventWriter& writer +) const { + // Phase C: Build event, enrich, encode, and write on context thread + + // Create a new LogEvent (no reuse from LoggerState) + LogEvent ev(params.service, params.logger_name, _sdk_version, 8); + + // Populate basic fields from params + ev.status = params.level; + ev.date = params.timestamp; + ev.message = params.message; + + // Enrich with RUM context if enabled + if (params.enrichment.enable_rum && context.rum) { + ev.rum_application_id = context.rum->application_id; + ev.rum_session_id = context.rum->session_id; + ev.rum_view_id = context.rum->view_id; + ev.rum_action_id = context.rum->action_id; + } + + // Enrich with OS properties + if (context.os) { + ev.os.value.emplace( + context.os->name, context.os->version, context.os->version_major + ); + if (!context.os->build.empty()) { + ev.os.value->build = context.os->build; + } + } + + // Enrich with device properties + if (context.device) { + RumDeviceProperties& device = ev.device.value.emplace(); + if (!context.device->type.empty()) { + DATADOG_ASSERT( + context.device->type == "desktop", + "DeviceInfo specifies non-desktop platform; log event serialization code " + "must be updated" + ); + device.type = RumDeviceType::Desktop; + } + device.name = context.device->name; + device.model = context.device->model; + device.brand = context.device->brand; + device.architecture = context.device->architecture; + device.locale = context.device->locale; + device.time_zone = context.device->time_zone; + } + + // Merge attributes with correct precedence: global < logger < message AttributeMerge::AssembleObject( - mut_state.event.user_attributes, - {global_attributes, state.user_attributes.attribute, message_attributes} + ev.user_attributes, + {params.global_attributes, params.logger_attributes, params.message_attributes} ); - // Now that we've fully populated our LogEvent value, serialize it as a JSON object, - // with all custom attributes from 'user_attributes' merged in at top level. If any - // properties in user_attributes use names that are already used by LogEvent struct - // fields, those custom attribute values will be ignored. - EncodeJson(mut_state.event_buffer, state.event); - - // Create a view of the JSON payload now held in our buffer, and call WriteEvent, - // which will copy our event data onto the storage queue - WriteEvent(Block( - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - reinterpret_cast(mut_state.event_buffer.data()), - mut_state.event_buffer.size() - )); + // Encode to the shared buffer (accessed only on context thread) + EncodeJson(_encode_buffer, ev); + + // Write the event + writer( + Block( + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + reinterpret_cast(_encode_buffer.data()), + _encode_buffer.size() + ), + Block{} + ); } } // namespace datadog::impl diff --git a/src/datadog/impl/features/logging/logging.hpp b/src/datadog/impl/features/logging/logging.hpp index a816d52e..93995c95 100644 --- a/src/datadog/impl/features/logging/logging.hpp +++ b/src/datadog/impl/features/logging/logging.hpp @@ -18,6 +18,10 @@ namespace datadog::impl { +struct LoggerEnrichmentConfig; +struct LoggerState; +struct LogCommandParams; + /** * Logging feature implementation. Keeps track of global state, creates loggers, and * handles generation of log events in response to logger calls. @@ -64,6 +68,22 @@ class Logging final : public Feature { LogLevel level, std::string_view message, const Attribute& message_attributes + ); + + /** + * Dispatches async log event processing to the context thread via + * ExecuteOnContextThread. Creates a weak_ptr for shutdown safety. + */ + void DispatchAsync(const LogCommandParams& params); + + /** + * Processes a log event on the context thread. Builds the event, enriches it with + * context data, encodes it to JSON, and writes it via the EventWriter. + */ + void ProcessLogEvent( + const LogCommandParams& params, + const CoreContext& context, + const EventWriter& writer ) const; private: @@ -79,6 +99,9 @@ class Logging final : public Feature { ObjectAttribute _global_attributes; mutable std::shared_mutex _global_attributes_mutex; + // Reusable buffer for encoding events; accessed only on the context thread + mutable std::vector _encode_buffer; + // HTTP request details used on upload; owned by the upload thread std::string _request_url; std::string _request_headers; From e1d8b3a520344cba33b1747c73d0dec2c66f8cb7 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Mon, 9 Mar 2026 18:46:18 -0400 Subject: [PATCH 12/25] chore: Remove mutex on RUM event encode buffer It's only used on the context thread, which processes functions serially, so it does not require synchronization. --- src/datadog/impl/features/rum/scope.cpp | 2 -- src/datadog/impl/features/rum/scope.hpp | 9 ++------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/datadog/impl/features/rum/scope.cpp b/src/datadog/impl/features/rum/scope.cpp index 4604ced5..207cbf70 100644 --- a/src/datadog/impl/features/rum/scope.cpp +++ b/src/datadog/impl/features/rum/scope.cpp @@ -26,8 +26,6 @@ RumScopeDependencies::RumScopeDependencies( } bool RumScopeDependencies::ShouldSampleSession() const { - std::unique_lock write_lock(mutex); - // If sampling rate is 100%, all sessions are sampled if (_sampling_rate_unit >= 1.0f) { return true; diff --git a/src/datadog/impl/features/rum/scope.hpp b/src/datadog/impl/features/rum/scope.hpp index 9c348a7b..bf01f3c4 100644 --- a/src/datadog/impl/features/rum/scope.hpp +++ b/src/datadog/impl/features/rum/scope.hpp @@ -9,10 +9,8 @@ #include #include #include -#include #include #include -#include #include #include @@ -38,15 +36,13 @@ struct RumScopeDependencies { const platform::IClock& clock; private: - // Synchronization for internal state - mutable std::shared_mutex mutex; - // Internal state used in sampling decisions float _sampling_rate_unit; + // Sampling state accessed only on the context thread mutable std::mt19937 _sampling_rng; mutable std::uniform_real_distribution _sampling_distribution; - // Internal state for encoding RUM event payloads + // Reusable buffer for encoding events; accessed only on the context thread mutable std::vector _encode_buffer; public: @@ -73,7 +69,6 @@ struct RumScopeDependencies { */ template std::string_view EncodeEvent(const T& event) const { - std::unique_lock write_lock(mutex); EncodeJson(_encode_buffer, event); return std::string_view( reinterpret_cast(_encode_buffer.data()), _encode_buffer.size() From 3f579fbcdb6354de128ae9c0c78b7860d8c87dfd Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Tue, 10 Mar 2026 12:40:13 -0400 Subject: [PATCH 13/25] chore: Update FeatureScope tests to use a real context thread --- tests/impl/core/feature_scope_test.cpp | 150 ++++++++++++++++--------- tests/support/diagnostics.hpp | 38 +++++-- 2 files changed, 123 insertions(+), 65 deletions(-) diff --git a/tests/impl/core/feature_scope_test.cpp b/tests/impl/core/feature_scope_test.cpp index da6f1be2..7aac3001 100644 --- a/tests/impl/core/feature_scope_test.cpp +++ b/tests/impl/core/feature_scope_test.cpp @@ -16,16 +16,88 @@ #include "datadog/core.hpp" #include "datadog/impl/core/context.hpp" +#include "datadog/impl/core/context_thread.hpp" #include "datadog/impl/core/feature_types/rum.hpp" #include "datadog/impl/platform/system_info.hpp" +#include "support/diagnostics.hpp" #include "support/threading.hpp" using namespace datadog; using namespace datadog::impl; +// Initial CoreContext values used in FeatureScope tests +static const platform::OsInfo MOCK_OS_INFO{ + "mock-os", "2.3.4", "mock-build-number", "2" +}; +static const platform::DeviceInfo MOCK_DEVICE_INFO{ + "desktop", + "mock-device", + "mock-model", + "mock-brand", + "x86_64", + "en-US", + "America/New_York" +}; +static const CoreContext MOCK_CONTEXT( + CoreConfig("client-token", "service", "env"), MOCK_OS_INFO, MOCK_DEVICE_INFO +); + +// FeatureScope tests rely on dummy features that produce events in the form of uint64 +// values; we will buffer up to this number of events in a thread-safe array static const size_t MAX_EVENTS = 512; +/** + * Stand-in for a Feature that interacts with the FeatureScope in order to read + * CoreContext, mutate CoreContext, and generate events. + * + * Provides an EventGeneratedFunc that buffers the event into a thread-safe array with a + * fixed capacity. + */ +struct FeatureState { + std::array events; + std::atomic num_events{0}; + + /** + * Parses `event` as a string-formatted uint64_t, then atomically stores it in the + * next available slot within the events array. If `events` is at capacity, a test + * assert will be triggered. + */ + bool HandleEvent(Block event, Block event_metadata) { + // Metadata is unused in these tests + REQUIRE(event_metadata.empty()); + + // Parse the event payload as a non-null-terminated ASCII-encoded int + uint64_t value; + auto result = std::from_chars(event.data(), event.data() + event.size(), value); + REQUIRE(result.ec == std::errc{}); + + // Atomically reserve an array slot and write the value to it + const size_t i = num_events.fetch_add(1, std::memory_order_relaxed); + REQUIRE(i >= 0); + REQUIRE(i < MAX_EVENTS); + events[i] = value; + return true; + } + + /** + * Creates a FeatureScope that can be used to simulate the interactions of a `Feature` + * implementation with the Core. + */ + FeatureScope CreateScope( + CoreContextProvider& context_provider, Queue>& context_queue + ) { + return FeatureScope::Create( + context_provider, + [this](Block event, Block event_metadata) { + return this->HandleEvent(event, event_metadata); + }, + DiagnosticLogger{}, + context_queue + ); + } +}; + /** * Reads an arbitrary value from context, via FeatureScope, for use in events. */ @@ -57,44 +129,27 @@ static void WriteContextValue(FeatureScope& scope, uint64_t value) { }); } -struct FeatureState { - std::array events; - std::atomic num_events{0}; - - bool HandleEvent(Block event, Block event_metadata) { - // Metadata is unused in these tests - REQUIRE(event_metadata.empty()); - - // Parse the event payload as a non-null-terminated ASCII-encoded int - uint64_t value; - auto result = std::from_chars(event.data(), event.data() + event.size(), value); - REQUIRE(result.ec == std::errc{}); - - // Atomically reserve an array slot and write the value to it - const size_t i = num_events.fetch_add(1, std::memory_order_relaxed); - REQUIRE(i >= 0); - REQUIRE(i < MAX_EVENTS); - events[i] = value; - return true; - } -}; - TEST_CASE("FeatureScope thread safety", "[unit][core][thread-safety]") { // Given a CoreContextProvider - platform::OsInfo os_info{"mock-os", "2.3.4", "mock-build-number", "2"}; - platform::DeviceInfo device_info{ - "desktop", - "mock-device", - "mock-model", - "mock-brand", - "x86_64", - "en-US", - "America/New_York" + CoreContextProvider context_provider(MOCK_CONTEXT); + + // And a DiagnosticLogger that will capture all emitted messages in a buffer + DiagnosticMessageBuffer diagnostics; + DiagnosticLogger diagnostic_logger = diagnostics.CreateTestLogger(); + + // And a fully functional context thread that reads functions from a queue and + // executes them serially in the background + Queue> context_queue; + std::thread context_thread{ + ContextThreadMain, + std::ref(diagnostic_logger), + std::ref(context_queue), + std::ref(context_provider) + }; + auto stop_context_thread = [&]() { + context_queue.Stop(); + context_thread.join(); }; - CoreContext initial_context( - CoreConfig("client-token", "service", "env"), os_info, device_info - ); - CoreContextProvider context_provider(initial_context); // And three independent features that will handle API calls from different threads FeatureState feature_a; @@ -102,27 +157,9 @@ TEST_CASE("FeatureScope thread safety", "[unit][core][thread-safety]") { FeatureState feature_c; // And a separate scope for each feature, initialized from the same context provider - FeatureScope scope_a = FeatureScope::CreateForTesting( - context_provider, - [&](Block event, Block event_metadata) { - return feature_a.HandleEvent(event, event_metadata); - }, - DiagnosticLogger{} - ); - FeatureScope scope_b = FeatureScope::CreateForTesting( - context_provider, - [&](Block event, Block event_metadata) { - return feature_b.HandleEvent(event, event_metadata); - }, - DiagnosticLogger{} - ); - FeatureScope scope_c = FeatureScope::CreateForTesting( - context_provider, - [&](Block event, Block event_metadata) { - return feature_c.HandleEvent(event, event_metadata); - }, - DiagnosticLogger{} - ); + FeatureScope scope_a = feature_a.CreateScope(context_provider, context_queue); + FeatureScope scope_b = feature_b.CreateScope(context_provider, context_queue); + FeatureScope scope_c = feature_c.CreateScope(context_provider, context_queue); // When we run three threads, each of which sporadically reads from or writes to a // shared context value, then produces an event based on that value, 512 times each @@ -152,6 +189,7 @@ TEST_CASE("FeatureScope thread safety", "[unit][core][thread-safety]") { for (auto& thread : threads) { thread.join(); } + stop_context_thread(); // Then we should have populated 512 events for each feature REQUIRE(feature_a.num_events.load(std::memory_order_relaxed) == MAX_EVENTS); diff --git a/tests/support/diagnostics.hpp b/tests/support/diagnostics.hpp index 3fb674d2..93ddbbcf 100644 --- a/tests/support/diagnostics.hpp +++ b/tests/support/diagnostics.hpp @@ -13,6 +13,8 @@ #include "datadog/core.h" #include "datadog/core.hpp" +#include "datadog/impl/diagnostics.hpp" + #include "support/catch.hpp" using namespace datadog; @@ -125,25 +127,43 @@ struct DiagnosticMessageBuffer { } /** - * Configures an SDK instance initialized via the C++ API to route all diagnostic - * messages to this struct. + * Creates a DiagnosticHandler function that will copy any messages received into the + * vectors held by this DiagnosticMessageBuffer. Uses the types defined in the C++ + * API (i.e. datadog::DiagnosticHandler), which are also used internally within the + * implementation layer. */ - void ConfigureCpp(datadog::CoreConfig& config) { - config.SetDiagnosticHandler([&](const datadog::DiagnosticMessage& message) { + DiagnosticHandler CreateHandler() { + return [this](const DiagnosticMessage& message) { switch (message.level) { - case datadog::DiagnosticLevel::Debug: + case DiagnosticLevel::Debug: debug.emplace_back(message.text); break; - case datadog::DiagnosticLevel::Status: + case DiagnosticLevel::Status: status.emplace_back(message.text); break; - case datadog::DiagnosticLevel::Warning: + case DiagnosticLevel::Warning: warning.emplace_back(message.text); break; - case datadog::DiagnosticLevel::Error: + case DiagnosticLevel::Error: error.emplace_back(message.text); break; } - }); + }; + } + + /** + * Configures an SDK instance initialized via the C++ API to route all diagnostic + * messages to this struct. + */ + void ConfigureCpp(CoreConfig& config) { + config.SetDiagnosticHandler(CreateHandler()); + } + + /** + * Creates a DiagnosticLogger for use in unit tests: any and all messages emitted via + * that DiagnosticLogger will be copied into this DiagnosticMessageBuffer. + */ + impl::DiagnosticLogger CreateTestLogger() { + return impl::DiagnosticLogger(CreateHandler(), DiagnosticLevel::Debug); } }; From e743d19c60ed9c4fce3be87721287ba5c2c3c3b0 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Tue, 10 Mar 2026 13:12:30 -0400 Subject: [PATCH 14/25] chore: Refactor repeated mock os/device info into a common header --- tests/impl/core/feature_scope_test.cpp | 20 ++----------- tests/impl/features/logging/logger_test.cpp | 21 ++++---------- tests/impl/features/rum/rum_test.cpp | 31 ++++++++------------- tests/support/context.hpp | 28 +++++++++++++++++++ tests/support/rum_event_capture.hpp | 16 ++--------- 5 files changed, 50 insertions(+), 66 deletions(-) create mode 100644 tests/support/context.hpp diff --git a/tests/impl/core/feature_scope_test.cpp b/tests/impl/core/feature_scope_test.cpp index 7aac3001..02e8fa2e 100644 --- a/tests/impl/core/feature_scope_test.cpp +++ b/tests/impl/core/feature_scope_test.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include #include @@ -20,29 +19,14 @@ #include "datadog/impl/core/feature_types/rum.hpp" #include "datadog/impl/platform/system_info.hpp" +#include "support/catch.hpp" +#include "support/context.hpp" #include "support/diagnostics.hpp" #include "support/threading.hpp" using namespace datadog; using namespace datadog::impl; -// Initial CoreContext values used in FeatureScope tests -static const platform::OsInfo MOCK_OS_INFO{ - "mock-os", "2.3.4", "mock-build-number", "2" -}; -static const platform::DeviceInfo MOCK_DEVICE_INFO{ - "desktop", - "mock-device", - "mock-model", - "mock-brand", - "x86_64", - "en-US", - "America/New_York" -}; -static const CoreContext MOCK_CONTEXT( - CoreConfig("client-token", "service", "env"), MOCK_OS_INFO, MOCK_DEVICE_INFO -); - // FeatureScope tests rely on dummy features that produce events in the form of uint64 // values; we will buffer up to this number of events in a thread-safe array static const size_t MAX_EVENTS = 512; diff --git a/tests/impl/features/logging/logger_test.cpp b/tests/impl/features/logging/logger_test.cpp index 494a9419..1ef31893 100644 --- a/tests/impl/features/logging/logger_test.cpp +++ b/tests/impl/features/logging/logger_test.cpp @@ -19,22 +19,13 @@ #include "datadog/impl/platform/system_info.hpp" #include "mock/clock.hpp" +#include "support/context.hpp" #include "support/feature.hpp" using namespace datadog; using namespace datadog::impl; static const CoreConfig CORE_CONFIG("mock-client-token", "mock-service", "mock-env"); -static const platform::OsInfo OS_INFO{"mock-os", "2.3.4", "mock-build-number", "2"}; -static const platform::DeviceInfo DEVICE_INFO{ - "desktop", - "mock-device", - "mock-model", - "mock-brand", - "x86_64", - "en-US", - "America/New_York" -}; TEST_CASE("Logger RUM enrichment", "[unit][logging]") { static const UUID uuid_9916 = *UUID::Parse("99163baf-48fe-458f-b777-eab1e4038342"); @@ -68,7 +59,7 @@ TEST_CASE("Logger RUM enrichment", "[unit][logging]") { auto logger = logging->CreateLogger(LoggerConfig()); // And a context that includes RUM session details - CoreContext context(CORE_CONFIG, OS_INFO, DEVICE_INFO); + CoreContext context(CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO); context.rum = RumFeatureContext(); context.rum->application_id = uuid_9916; context.rum->session_id = uuid_d927; @@ -100,7 +91,7 @@ TEST_CASE("Logger RUM enrichment", "[unit][logging]") { // And a context that includes a RUM application ID and session ID, but no view or // action ID - CoreContext context(CORE_CONFIG, OS_INFO, DEVICE_INFO); + CoreContext context(CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO); context.rum = RumFeatureContext(); context.rum->application_id = uuid_9916; context.rum->session_id = uuid_d927; @@ -129,7 +120,7 @@ TEST_CASE("Logger RUM enrichment", "[unit][logging]") { auto logger = logging->CreateLogger(LoggerConfig()); // And a context that includes a RUM application ID, but no session - CoreContext context(CORE_CONFIG, OS_INFO, DEVICE_INFO); + CoreContext context(CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO); context.rum = RumFeatureContext(); context.rum->application_id = uuid_9916; @@ -165,7 +156,7 @@ TEST_CASE("Logger RUM enrichment", "[unit][logging]") { auto logger = logging->CreateLogger(LoggerConfig().SetEnrichWithRumContext(false)); // And a context that includes RUM session details - CoreContext context(CORE_CONFIG, OS_INFO, DEVICE_INFO); + CoreContext context(CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO); context.rum = RumFeatureContext(); context.rum->application_id = uuid_9916; context.rum->session_id = uuid_d927; @@ -195,7 +186,7 @@ TEST_CASE("Logger RUM enrichment", "[unit][logging]") { auto logger = logging->CreateLogger(LoggerConfig()); // And a context that has no RumFeatureContext - CoreContext context(CORE_CONFIG, OS_INFO, DEVICE_INFO); + CoreContext context(CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO); // When we emit a log event while the SDK is running FeatureTest test(context); diff --git a/tests/impl/features/rum/rum_test.cpp b/tests/impl/features/rum/rum_test.cpp index 7d417864..86e9c972 100644 --- a/tests/impl/features/rum/rum_test.cpp +++ b/tests/impl/features/rum/rum_test.cpp @@ -15,6 +15,7 @@ #include "datadog/impl/platform/system_info.hpp" #include "mock/clock.hpp" +#include "support/context.hpp" #include "support/feature.hpp" using namespace datadog; @@ -23,16 +24,6 @@ using namespace datadog::impl; static const UUID APPLICATION_ID = *UUID::Parse("a991ca10-4004-4004-4004-beefbeefbeef"); static const CoreConfig CORE_CONFIG("mock-client-token", "mock-service", "mock-env"); static const RumConfig RUM_CONFIG(APPLICATION_ID); -static const platform::OsInfo OS_INFO{"mock-os", "2.3.4", "mock-build-number", "2"}; -static const platform::DeviceInfo DEVICE_INFO{ - "desktop", - "mock-device", - "mock-model", - "mock-brand", - "x86_64", - "en-US", - "America/New_York" -}; TEST_CASE("Rum context population", "[unit][rum]") { SECTION( @@ -44,7 +35,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { auto rum = std::make_shared(RUM_CONFIG, clock); // And a CoreContext that initially has no RumFeatureContext - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); CoreContext context = test.GetContextSync(); REQUIRE(!context.rum); @@ -66,7 +57,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { SECTION("M clear RumFeatureContext W SDK stops") { // Given a running RUM feature which has populated a RumFeatureContext MockClock clock; - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); @@ -83,7 +74,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { SECTION("M change session_id W session expires and is replaced") { // Given a running RUM feature MockClock clock; - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); @@ -114,7 +105,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { SECTION("M reset session_id W session is explicitly stopped") { // Given a running RUM feature with a valid session MockClock clock; - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); @@ -141,7 +132,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { SECTION("M populate view_id W a view is active") { // Given a running RUM feature with a valid session MockClock clock; - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); @@ -175,7 +166,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { SECTION("M not populate view_id W a view is no longer active") { // Given a running RUM feature with a valid session and view MockClock clock; - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); @@ -197,7 +188,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { SECTION("M populate action_id W an action is active") { // Given a running RUM feature with a valid session MockClock clock; - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); @@ -218,7 +209,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { SECTION("M not populate action_id W action is no longer active") { // Given a running RUM feature with a valid session and view MockClock clock; - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); @@ -240,7 +231,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { SECTION("M not populate action_id W view is no longer active") { // Given a running RUM feature with a valid session and view MockClock clock; - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); @@ -262,7 +253,7 @@ TEST_CASE("Rum context population", "[unit][rum]") { SECTION("M not populate action_id W session is no longer active") { // Given a running RUM feature with a valid session and view MockClock clock; - FeatureTest test(CoreContext{CORE_CONFIG, OS_INFO, DEVICE_INFO}); + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); diff --git a/tests/support/context.hpp b/tests/support/context.hpp new file mode 100644 index 00000000..c064a6f9 --- /dev/null +++ b/tests/support/context.hpp @@ -0,0 +1,28 @@ +// 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 "datadog/impl/core/context.hpp" +#include "datadog/impl/platform/system_info.hpp" + +static const datadog::platform::OsInfo MOCK_OS_INFO{ + "mock-os", "2.3.4", "mock-build-number", "2" +}; +static const datadog::platform::DeviceInfo MOCK_DEVICE_INFO{ + "desktop", + "mock-device", + "mock-model", + "mock-brand", + "x86_64", + "en-US", + "America/New_York" +}; +static const datadog::impl::CoreContext MOCK_CONTEXT( + datadog::CoreConfig("client-token", "service", "env"), + MOCK_OS_INFO, + MOCK_DEVICE_INFO +); diff --git a/tests/support/rum_event_capture.hpp b/tests/support/rum_event_capture.hpp index 040c2af6..7fa116b2 100644 --- a/tests/support/rum_event_capture.hpp +++ b/tests/support/rum_event_capture.hpp @@ -19,6 +19,7 @@ #include "datadog/impl/diagnostics.hpp" #include "datadog/impl/platform/system_info.hpp" +#include "support/context.hpp" #include "support/diagnostics.hpp" using namespace datadog; @@ -40,17 +41,6 @@ class RumEventCapture { const char* session_id; const char* view_id; - platform::OsInfo os_info{"mock-os", "2.3.4", "mock-build-number", "2"}; - platform::DeviceInfo device_info{ - "desktop", - "mock-device", - "mock-model", - "mock-brand", - "x86_64", - "en-US", - "America/New_York" - }; - CoreContextProvider context_provider; FeatureScope feature_scope; @@ -73,8 +63,8 @@ class RumEventCapture { view_id(view_id), context_provider(CoreContext( CoreConfig{"fake-client-token", "fake-service", "fake-env"}, - os_info, - device_info + MOCK_OS_INFO, + MOCK_DEVICE_INFO )), feature_scope( FeatureScope::CreateForTesting( From 0005f061ae8c53059e0404bff217965d7f631a03 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Tue, 10 Mar 2026 13:12:55 -0400 Subject: [PATCH 15/25] chore: Add context_thread_test.cpp to complement feature_scope_test.cpp --- tests/CMakeLists.txt | 1 + tests/impl/core/context_thread_test.cpp | 60 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 tests/impl/core/context_thread_test.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e9818fa4..dedc0a98 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -27,6 +27,7 @@ add_executable(dd_native_tests impl/core/feature_types/rum_test.cpp impl/core/block_test.cpp impl/core/context_test.cpp + impl/core/context_thread_test.cpp impl/core/core_test.cpp impl/core/feature_id_test.cpp impl/core/feature_read_test.cpp diff --git a/tests/impl/core/context_thread_test.cpp b/tests/impl/core/context_thread_test.cpp new file mode 100644 index 00000000..00ef3b36 --- /dev/null +++ b/tests/impl/core/context_thread_test.cpp @@ -0,0 +1,60 @@ +// 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" + +#include "support/catch.hpp" +#include "support/context.hpp" +#include "support/diagnostics.hpp" + +using namespace datadog; +using namespace datadog::impl; + +// This file tests the basic operation of the context thread. Feature implementations +// use FeatureScope to offload work to the context thread: for more extensive test +// coverage of that behavior, including queue processing that actually happens on a +// background thread, with related thread-safety tests, see feature_scope_test.cpp. + +TEST_CASE("ContextThreadMain", "[unit]") { + // Given a CoreContextProvider and a DiagnosticLogger + CoreContextProvider context_provider(MOCK_CONTEXT); + DiagnosticMessageBuffer diagnostics; + DiagnosticLogger diagnostic_logger = diagnostics.CreateTestLogger(); + + // And a queue containing functions to be executed by the context thread + Queue> context_queue; + + SECTION("M log debug messages W started and stopped cleanly") { + // When we run ContextThreadMain synchronously on an empty, stopped queue + context_queue.Stop(); + ContextThreadMain(diagnostic_logger, context_queue, context_provider); + + // Then we should have exactly two debug messages that signal thread start and stop + REQUIRE(diagnostics.debug.size() == 2); + REQUIRE(diagnostics.debug[0] == "Context thread starting"); + REQUIRE(diagnostics.debug[1] == "Context thread finished"); + + // And there should be no other diagnostic messages besides those two + REQUIRE(diagnostics.TotalSize() == 2); + } + + SECTION("M call enqueued functions") { + // When we enqueue a couple of functions that increment x, then run + // ContextThreadMain synchronously + int x = 0; + context_queue.Push([&x]() { x++; }); + context_queue.Push([&x]() { x++; }); + context_queue.Stop(); + ContextThreadMain(diagnostic_logger, context_queue, context_provider); + + // Then we should have no unexpected diagnostic output + REQUIRE(diagnostics.debug.size() == 2); + REQUIRE(diagnostics.debug.size() == diagnostics.TotalSize()); + + // And our increment function should have run twice + REQUIRE(x == 2); + } +} From dac8a2796570a469dcc4c75bde6c19c576e6e8fb Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Tue, 10 Mar 2026 15:06:39 -0400 Subject: [PATCH 16/25] fix: Update RUM context immediately after command processing --- src/datadog/impl/features/rum/rum.cpp | 38 +++++++++++++-------------- src/datadog/impl/features/rum/rum.hpp | 1 - 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/datadog/impl/features/rum/rum.cpp b/src/datadog/impl/features/rum/rum.cpp index 39e8305f..50bc7590 100644 --- a/src/datadog/impl/features/rum/rum.cpp +++ b/src/datadog/impl/features/rum/rum.cpp @@ -186,18 +186,18 @@ RumCommandParams Rum::GetBaseCommandParams(const Attribute& attributes) const { } void Rum::DispatchAsync(const RumCommand& command) { + // If our _scope is no longer valid, the SDK has been stopped prior to this call if (!_scope) { return; } - - // Store reference to scope to satisfy linter's optional access check FeatureScope& scope = *_scope; - // Capture weak_ptr to self for fail-safe shutdown detection. Matches iOS SDK - // pattern where closures capture weak references and fail gracefully when - // features are deallocated during shutdown. + // Capture weak_ptr to self for fail-safe shutdown detection auto weak_rum = std::weak_ptr(std::static_pointer_cast(shared_from_this())); + // Enqueue a function to run on the context thread, capturing the command being + // dispatched: when this function is executed, the context thread will process the + // command, updating internal RUM state and potentially producing events scope.ExecuteOnContextThread( [weak_rum, cmd = command](const CoreContext& context, const EventWriter& writer) { // Single-level check: Is Rum object still alive? @@ -211,24 +211,22 @@ void Rum::DispatchAsync(const RumCommand& command) { // _application (all valid as long as Rum is alive) rum->_application.Process(cmd, context, writer); - // After every command, update our RumFeatureContext, which makes current RUM - // state available to other features within the SDK - rum->UpdateFeatureContext(); + // After every command, build a RumContext value (in _application_snapshot) that + // describes the state of our internal scope tree + rum->UpdateApplicationSnapshot(); } ); -} - -void Rum::UpdateFeatureContext() { - // Build a RumContext value (_application_snapshot) that summarizes the current state - // of our internal scope tree - UpdateApplicationSnapshot(); - // Write the relevant values to the global RumFeatureContext, so that other features - // can enrich their events with the latest RUM context - if (_scope) { - const RumFeatureContext rum_ctx = _application_snapshot.ToFeatureContext(); - _scope->UpdateContext([rum_ctx](CoreContext& ctx) { ctx.rum = rum_ctx; }); - } + // Enqueue a context-mutation function that will run immediately following the + // processing of the command + scope.UpdateContext([weak_rum](CoreContext& ctx) { + if (auto rum = weak_rum.lock()) { + // _application_snapshot has been updated with the result of processing our + // command; write the relevant UUIDs to the global RumFeatureContext, so that + // other features can enrich their events with RUM data + ctx.rum = rum->_application_snapshot.ToFeatureContext(); + } + }); } void Rum::UpdateApplicationSnapshot() { diff --git a/src/datadog/impl/features/rum/rum.hpp b/src/datadog/impl/features/rum/rum.hpp index aa636eba..872f12f6 100644 --- a/src/datadog/impl/features/rum/rum.hpp +++ b/src/datadog/impl/features/rum/rum.hpp @@ -161,7 +161,6 @@ class Rum final : public Feature { void DispatchAsync(const RumCommand& command); - void UpdateFeatureContext(); void UpdateApplicationSnapshot(); private: From b00dd9a4430029dd75043a1d685a2cbbbff1dba8 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Tue, 10 Mar 2026 15:46:17 -0400 Subject: [PATCH 17/25] chore: Supplement FeatureScope tests for fixed context-mutation ordering --- tests/impl/core/feature_scope_test.cpp | 246 +++++++++++++++++-------- 1 file changed, 174 insertions(+), 72 deletions(-) diff --git a/tests/impl/core/feature_scope_test.cpp b/tests/impl/core/feature_scope_test.cpp index 02e8fa2e..c2c50f45 100644 --- a/tests/impl/core/feature_scope_test.cpp +++ b/tests/impl/core/feature_scope_test.cpp @@ -83,37 +83,35 @@ struct FeatureState { }; /** - * Reads an arbitrary value from context, via FeatureScope, for use in events. + * Writes an arbitrary 64-bit unsigned int into the CoreContext, such that it can be + * retrieved by ReadContextValue. */ -static uint64_t ReadContextValue(FeatureScope& scope) { +static void WriteContextValue(CoreContext& ctx, uint64_t value) { + // Pack an 8-byte uint into the first half of the RUM session_id + UUID new_session_id = UUID::Zero; + std::memcpy(new_session_id.bytes.data(), &value, sizeof(value)); + if (!ctx.rum) { + ctx.rum.emplace(); + } + ctx.rum->session_id = new_session_id; +} + +/** + * Returns the arbitrary value that was stored in the provided CoreContext value, or 0 + * if no such value was stored. + */ +static uint64_t ReadContextValue(const CoreContext& ctx) { // Unpack an 8-byte uint from the first half of the RUM session_id - CoreContext context = scope.GetContext(); - if (!context.rum) { + if (!ctx.rum) { return 0; } - const RumFeatureContext& rum_ctx = *context.rum; + const RumFeatureContext& rum_ctx = *ctx.rum; const uint8_t* uuid_bytes = rum_ctx.session_id.bytes.data(); const uint64_t* ptr = reinterpret_cast(uuid_bytes); return *ptr; } -/** - * Writes an arbitrary value to context, via FeatureScope, to influence events - * generated on any thread. - */ -static void WriteContextValue(FeatureScope& scope, uint64_t value) { - // Pack an 8-byte uint into the first half of the RUM session_id - UUID new_session_id = UUID::Zero; - std::memcpy(new_session_id.bytes.data(), &value, sizeof(value)); - scope.UpdateContext([new_session_id](CoreContext& ctx) { - if (!ctx.rum) { - ctx.rum.emplace(); - } - ctx.rum->session_id = new_session_id; - }); -} - -TEST_CASE("FeatureScope thread safety", "[unit][core][thread-safety]") { +TEST_CASE("FeatureScope", "[unit][core]") { // Given a CoreContextProvider CoreContextProvider context_provider(MOCK_CONTEXT); @@ -135,61 +133,165 @@ TEST_CASE("FeatureScope thread safety", "[unit][core][thread-safety]") { context_thread.join(); }; - // And three independent features that will handle API calls from different threads - FeatureState feature_a; - FeatureState feature_b; - FeatureState feature_c; - - // And a separate scope for each feature, initialized from the same context provider - FeatureScope scope_a = feature_a.CreateScope(context_provider, context_queue); - FeatureScope scope_b = feature_b.CreateScope(context_provider, context_queue); - FeatureScope scope_c = feature_c.CreateScope(context_provider, context_queue); - - // When we run three threads, each of which sporadically reads from or writes to a - // shared context value, then produces an event based on that value, 512 times each - auto threads = RunParallel(3, [&](size_t thread_id) { - FeatureScope& scope = - thread_id == 0 ? scope_a : (thread_id == 1 ? scope_b : scope_c); - - for (uint64_t i = 0; i < MAX_EVENTS; i++) { - // Either write `i` to context or read the most recent value from context - uint64_t value = i; - const uint64_t x = (thread_id * thread_id) + i; - if (x % 3 == 0) { - WriteContextValue(scope, i); - } else { - value = ReadContextValue(scope); - } + SECTION("M run function on context thread W enqueued via ExecuteOnContextThread") { + // Given a single feature with a FeatureScope + FeatureState feature; + FeatureScope scope = feature.CreateScope(context_provider, context_queue); + + // When we offload a function to be run on the context thread, and then join on the + // context thread + int num_executions = 0; + auto main_thread_id = std::this_thread::get_id(); + scope.ExecuteOnContextThread([&](const CoreContext&, EventGeneratedFunc) { + num_executions++; + + // Then our function is executed on a background thread + auto context_thread_id = std::this_thread::get_id(); + REQUIRE(context_thread_id != main_thread_id); + }); + stop_context_thread(); + + // And our function did indeed execute + REQUIRE(num_executions == 1); + } + + SECTION( + "M run CoreContext-mutation function on context thread W enqueud via " + "UpdateContext" + ) { + // Given a single feature with a FeatureScope + FeatureState feature; + FeatureScope scope = feature.CreateScope(context_provider, context_queue); - // Produce an event whose payload is just a string-encoded version of our value + // When we enqueue a function that writes an arbitrary value to CoreContext + int num_updates = 0; + scope.UpdateContext([&](CoreContext& ctx) { + WriteContextValue(ctx, 0xbeef); + num_updates++; + }); + + // And then we enqueue a read-only function that will run thereafter + int num_executions = 0; + scope.ExecuteOnContextThread([&](const CoreContext& ctx, EventGeneratedFunc) { + // Then the CoreContext snapshot passed to our second function reflects the + // modifications made by the first + REQUIRE(ReadContextValue(ctx) == 0xbeef); + num_executions++; + }); + + // And both functions actually ran + stop_context_thread(); + REQUIRE(num_updates == 1); + REQUIRE(num_executions == 1); + } + + SECTION( + "M provide functions with a consistent view of CoreContext W mutations and reads " + "are enqueued in a consistent order" + ) { + // Given two mock features, one that writes a value to CoreContext and another that + // reads from CoreContext and produces events that include that value + FeatureState context_mutator; + FeatureState context_consumer; + + // And the FeatureScope interfaces that would be used by each feature + FeatureScope mutate_scope = + context_mutator.CreateScope(context_provider, context_queue); + FeatureScope consume_scope = + context_consumer.CreateScope(context_provider, context_queue); + + uint64_t generation = 0; + auto mutate = [&](CoreContext& ctx) { + generation++; + WriteContextValue(ctx, generation); + }; + auto consume = [](const CoreContext& ctx, EventGeneratedFunc event_generated_func) { + const uint64_t value = ReadContextValue(ctx); char buf[20]; auto res = std::to_chars(buf, buf + sizeof(buf), value); REQUIRE(res.ec == std::errc{}); - scope.WriteEvent(Block(buf, res.ptr - buf), {}); - } - }); + event_generated_func(Block(buf, res.ptr - buf), {}); + }; - // And we join on all threads - for (auto& thread : threads) { - thread.join(); - } - stop_context_thread(); - - // Then we should have populated 512 events for each feature - REQUIRE(feature_a.num_events.load(std::memory_order_relaxed) == MAX_EVENTS); - REQUIRE(feature_b.num_events.load(std::memory_order_relaxed) == MAX_EVENTS); - REQUIRE(feature_c.num_events.load(std::memory_order_relaxed) == MAX_EVENTS); - - // And the range of values should be [0..512] - uint64_t sum = 0; - for (uint64_t value : feature_a.events) { - REQUIRE(value >= 0); - REQUIRE(value <= MAX_EVENTS); - sum += value; + mutate_scope.UpdateContext(mutate); // Stores generation 1 in context + consume_scope.ExecuteOnContextThread(consume); // Produces '1' + consume_scope.ExecuteOnContextThread(consume); // Produces '1' + mutate_scope.UpdateContext(mutate); // Stores generation 2 in context + mutate_scope.UpdateContext(mutate); // Stores generation 3 in context + consume_scope.ExecuteOnContextThread(consume); // Produces '3' + stop_context_thread(); + + REQUIRE(context_mutator.num_events == 0); + REQUIRE(context_consumer.num_events == 3); + REQUIRE(context_consumer.events[0] == 1); + REQUIRE(context_consumer.events[1] == 1); + REQUIRE(context_consumer.events[2] == 3); } - // And the sum of all values in a single array should be roughly `sum(range(512))`, - // i.e. 130816, give or take a sizable margin of error - REQUIRE(sum > 90000); - REQUIRE(sum < 170000); + SECTION("M handle concurrently-enqueued operations") { + // Given three independent features that will handle API calls from different + // threads + FeatureState feature_a; + FeatureState feature_b; + FeatureState feature_c; + + // And a separate scope for each feature, initialized from the same context provider + FeatureScope scope_a = feature_a.CreateScope(context_provider, context_queue); + FeatureScope scope_b = feature_b.CreateScope(context_provider, context_queue); + FeatureScope scope_c = feature_c.CreateScope(context_provider, context_queue); + + // When we run three threads, each of which enqueues a different set of + // context-thread functions on behalf of a different feature + auto threads = RunParallel(3, [&](size_t thread_id) { + // Select one of our three scopes based on our thread ID + FeatureScope& scope = + thread_id == 0 ? scope_a : (thread_id == 1 ? scope_b : scope_c); + + // Enqueue exactly MAX_EVENTS functions to run on the context thread, while also + // sometimes enqueuing a mutation of the CoreContext value that all threads are + // reading from + for (uint64_t i = 0; i < MAX_EVENTS; i++) { + // Every so often, write our current loop counter `i` into the CoreContext + const bool should_mutate_context = ((thread_id * thread_id) + i) % 3 == 0; + if (should_mutate_context) { + scope.UpdateContext([i](CoreContext& ctx) { WriteContextValue(ctx, i); }); + } + + // On each iteration, enqueue a function that will run on the context thread and + // produce an event with the latest value read from CoreContext + scope.ExecuteOnContextThread([](const CoreContext& ctx, + EventGeneratedFunc event_generated_func) { + // Produce an event whose payload is just a string version of our value + char buf[20]; + auto res = std::to_chars(buf, buf + sizeof(buf), ReadContextValue(ctx)); + REQUIRE(res.ec == std::errc{}); + event_generated_func(Block(buf, res.ptr - buf), {}); + }); + } + }); + + // And we join on all threads + for (auto& thread : threads) { + thread.join(); + } + stop_context_thread(); + + // Then we should have populated 512 events for each feature + REQUIRE(feature_a.num_events.load(std::memory_order_relaxed) == MAX_EVENTS); + REQUIRE(feature_b.num_events.load(std::memory_order_relaxed) == MAX_EVENTS); + REQUIRE(feature_c.num_events.load(std::memory_order_relaxed) == MAX_EVENTS); + + // And the range of values should be [0..512] + uint64_t sum = 0; + for (uint64_t value : feature_a.events) { + REQUIRE(value >= 0); + REQUIRE(value <= MAX_EVENTS); + sum += value; + } + + // And the sum of all values in a single array should be roughly `sum(range(512))`, + // i.e. 130816, give or take a sizable margin of error + REQUIRE(sum > 90000); + REQUIRE(sum < 170000); + } } From 6c12f14fcf46da01f1e1597374a2ade7286512eb Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Tue, 10 Mar 2026 17:38:37 -0400 Subject: [PATCH 18/25] chore: Add tentative context-queue-drain test --- tests/impl/core/feature_scope_test.cpp | 90 ++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 4 deletions(-) diff --git a/tests/impl/core/feature_scope_test.cpp b/tests/impl/core/feature_scope_test.cpp index c2c50f45..62dbedb8 100644 --- a/tests/impl/core/feature_scope_test.cpp +++ b/tests/impl/core/feature_scope_test.cpp @@ -96,6 +96,13 @@ static void WriteContextValue(CoreContext& ctx, uint64_t value) { ctx.rum->session_id = new_session_id; } +static bool GenerateUInt64Event(const EventGeneratedFunc& func, uint64_t value) { + char buf[20]; + auto res = std::to_chars(buf, buf + sizeof(buf), value); + REQUIRE(res.ec == std::errc{}); + return func(Block(buf, res.ptr - buf), {}); +} + /** * Returns the arbitrary value that was stored in the provided CoreContext value, or 0 * if no such value was stored. @@ -262,10 +269,7 @@ TEST_CASE("FeatureScope", "[unit][core]") { scope.ExecuteOnContextThread([](const CoreContext& ctx, EventGeneratedFunc event_generated_func) { // Produce an event whose payload is just a string version of our value - char buf[20]; - auto res = std::to_chars(buf, buf + sizeof(buf), ReadContextValue(ctx)); - REQUIRE(res.ec == std::errc{}); - event_generated_func(Block(buf, res.ptr - buf), {}); + GenerateUInt64Event(event_generated_func, ReadContextValue(ctx)); }); } }); @@ -294,4 +298,82 @@ TEST_CASE("FeatureScope", "[unit][core]") { REQUIRE(sum > 90000); REQUIRE(sum < 170000); } + + SECTION("M drain queue and execute all enqueued functions W stopped") { + // Given a single feature with a FeatureScope + FeatureState feature; + FeatureScope scope = feature.CreateScope(context_provider, context_queue); + + // And a promise that will block the context thread until we explicitly resolve it + std::promise gate; + std::future gate_signal = gate.get_future(); + scope.ExecuteOnContextThread([&](const CoreContext&, EventGeneratedFunc) { + gate_signal.wait(); + }); + + // When we enqueue a bunch of functions to run on the context thread + for (size_t i = 0; i < MAX_EVENTS; i++) { + scope.ExecuteOnContextThread([i](const CoreContext&, EventGeneratedFunc func) { + GenerateUInt64Event(func, i); + }); + } + + // And then we signal that the context queue should stop accepting new events due to + // shutdown + context_queue.Stop(); + + // And then we unblock queue processing on the context thread + gate.set_value(); + + // And then we join on the context thread + context_thread.join(); + + // Then we end up with events produced by all functions + REQUIRE(feature.num_events == MAX_EVENTS); + for (size_t i = 0; i < MAX_EVENTS; i++) { + REQUIRE(feature.events[i] == i); + } + } +} + +/* +TEST_CASE("FeatureScope shutdown safety", "[unit][core][shutdown]") { + SECTION("M not use FeatureScope after destruction W lambdas queued on shutdown") { + // Given a CoreContextProvider + CoreContextProvider context_provider(MOCK_CONTEXT); + + // And a context queue with a one-shot gate to hold the context thread + Queue> queue; + std::promise gate; + std::future gate_signal = gate.get_future(); + + // Start a context thread that processes the queue + std::thread context_thread([&]() { + while (auto thunk = queue.Pop()) { + (*thunk)(); + } + }); + + // Block the context thread by pushing a gating lambda first + queue.Push([&]() { gate_signal.wait(); }); + + // Now push real work via FeatureScope - these sit behind the gate + auto scope = std::make_unique(FeatureScope::Create( + context_provider, [](Block, Block) { return true; }, DiagnosticLogger{}, queue + )); + scope->ExecuteOnContextThread([](auto&, auto&) {}); + scope->ExecuteOnContextThread([](auto&, auto&) {}); + scope->UpdateContext([](CoreContext&) {}); + + // Destroy FeatureScope while those lambdas are still queued + scope.reset(); + + // Release the gate - context thread will now execute the lambdas + // with dangling `this` (ASAN will catch the use-after-free here) + gate.set_value(); + + queue.Stop(); + context_thread.join(); + } } +*/ \ No newline at end of file From 101afbf5f57e325a152096852bea1fe01b1df669 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Tue, 10 Mar 2026 18:12:50 -0400 Subject: [PATCH 19/25] chore: Remove old GetContext() and WriteEvent() functions --- src/datadog/impl/core/feature.cpp | 7 --- src/datadog/impl/core/feature.hpp | 15 +---- src/datadog/impl/core/feature_scope.cpp | 12 ---- src/datadog/impl/core/feature_scope.hpp | 36 ----------- tests/impl/core/feature_test.cpp | 15 +++-- .../impl/features/rum/scopes/action_test.cpp | 8 +-- .../features/rum/scopes/resource_test.cpp | 8 +-- .../impl/features/rum/scopes/session_test.cpp | 8 +-- tests/impl/features/rum/scopes/view_test.cpp | 8 +-- tests/mock/feature.hpp | 9 ++- tests/support/rum_event_capture.hpp | 61 +++++++++++-------- 11 files changed, 64 insertions(+), 123 deletions(-) diff --git a/src/datadog/impl/core/feature.cpp b/src/datadog/impl/core/feature.cpp index 90543fa8..3baeae2f 100644 --- a/src/datadog/impl/core/feature.cpp +++ b/src/datadog/impl/core/feature.cpp @@ -29,13 +29,6 @@ void Feature::OnCoreStopping() { _scope.reset(); } -bool Feature::WriteEvent(Block event, Block event_metadata) const { - if (_scope) { - return _scope->WriteEvent(event, event_metadata); - } - return false; -} - bool Feature::IsRunning() const { return _scope.has_value(); } } // namespace datadog::impl diff --git a/src/datadog/impl/core/feature.hpp b/src/datadog/impl/core/feature.hpp index d0380c51..e47ce170 100644 --- a/src/datadog/impl/core/feature.hpp +++ b/src/datadog/impl/core/feature.hpp @@ -74,8 +74,8 @@ struct FeatureStorageConfig { * - If the feature has unique storage requirements, implement `GetStorageConfig()` * - If the feature needs initialization/shutdown logic, implement `Start()`/`Stop()` * - Define main-thread-callable functions for the required feature-specific operations - * - In those functions, call `WriteEvent()` for each event the feature generate, using - * whatever binary format is appropriate for the feature + * - In those functions, call `_scope->ExecuteOnContextThread()` to generate events + * asynchronously on the context thread, using the provided `EventWriter` * - Implement `UploadThread_PrepareReport()` to read batches of events (in the same * format) and return a `Report` object describing the resulting HTTP request that * should be made to upload that batch to the appropriate intake endpoint @@ -120,17 +120,6 @@ class Feature : public std::enable_shared_from_this { */ virtual void Stop() {} - /** - * Callable from the main thread in order to produce a new event. Copies the given - * event payload(s) into the storage queue so that the storage thread can write them - * to persistent storage. Once the batch containing this event is ready for upload, - * the upload thread will pass it to UploadThread_PrepareReport(). - * - * @returns whether the event was successfully enqueued for storage. If called before - * Start() or after Stop(), always returns false. - */ - bool WriteEvent(Block event, Block event_metadata = {}) const; - /** * @returns whether the feature has received a call to Start() and has not yet * received a call to Stop(). diff --git a/src/datadog/impl/core/feature_scope.cpp b/src/datadog/impl/core/feature_scope.cpp index 2b9d01e1..d8b67f50 100644 --- a/src/datadog/impl/core/feature_scope.cpp +++ b/src/datadog/impl/core/feature_scope.cpp @@ -58,11 +58,6 @@ FeatureScope FeatureScope::CreateForTesting( ); } -CoreContext FeatureScope::GetContext() const { - DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); - return _context_provider->Get(); -} - void FeatureScope::UpdateContext(const std::function& callback) { DATADOG_ASSERT(_context_provider, "FeatureScope has no _context_provider"); @@ -82,13 +77,6 @@ void FeatureScope::UpdateContext(const std::function& callba }); } -bool FeatureScope::WriteEvent(Block event, Block event_metadata) const { - if (_event_generated_func) { - return _event_generated_func(event, event_metadata); - } - return false; -} - void FeatureScope::ExecuteOnContextThread(const ContextThreadFunc& func) { if (_mode == FeatureScope::ExecutionMode::Synchronous) { // Testing mode: execute synchronously on calling thread diff --git a/src/datadog/impl/core/feature_scope.hpp b/src/datadog/impl/core/feature_scope.hpp index 4a122ba6..6a3586f9 100644 --- a/src/datadog/impl/core/feature_scope.hpp +++ b/src/datadog/impl/core/feature_scope.hpp @@ -118,34 +118,6 @@ class FeatureScope { const DiagnosticLogger& diagnostic_logger ); - /** - * Creates an immutable, thread-safe copy of the CoreContext, which contains all the - * external information that a feature might need in order to generate fully-enriched - * events. - * - * NOTE: Our current logging implementation is naively optimized to ensure - * as-fast-as-possible best-case performance, and to minimize copying and allocations, - * but it may exhibit variable and undesirable worst-case performance when there's - * contention on the CoreContext - e.g. if we try to get a copy of the CoreContext in - * the main thread while another thread is modifying it, we may end up blocking the - * main thread for an unacceptable duration. - * - * The mobile SDKs optimize for predictable worst-case performance in the main thread - * using async dispatch. If we adopted a similar approach: when an API call was - * handled on the main thread, rather than calling GetContext() and WriteEvent() - * synchronously, we'd instead enqueue a callback to be invoked on a background thread - * once a context and writer were ready. - * - * If we want to guarantee thread safety in this SDK, then using async dispatch may be - * a sensible choice - it would presumably ensure predictable main-thread performance - * overhead, at the cost of additional complexity, additional copying between threads, - * and raw speed in idealized single-threaded usage. - * - * Until we can profile and evaluate the tradeoffs, though, this straightforward, - * synchronous approach prevails. - */ - CoreContext GetContext() const; - /** * Performs a thread-safe write to the SDK's global CoreContext value, allowing a * feature to populate up-to-date state that the core or other features may access. @@ -161,14 +133,6 @@ class FeatureScope { */ void UpdateContext(const std::function& callback); - /** - * Enqueues an arbitrary event payload to be written to disk in the storage thread. - * - * NOTE: Given that most features inevitably need thread-safe access to the - * CoreContext, it may be worthwhile to use async dispatch (see above). - */ - bool WriteEvent(Block event, Block event_metadata) const; - /** * Executes a function on the context thread, providing it with a CoreContext * snapshot and an EventWriter for generating events. diff --git a/tests/impl/core/feature_test.cpp b/tests/impl/core/feature_test.cpp index 4e75ddc0..f87c64cf 100644 --- a/tests/impl/core/feature_test.cpp +++ b/tests/impl/core/feature_test.cpp @@ -28,18 +28,23 @@ class ChattyFeature : public MockFeature { ChattyFeature() : MockFeature(CreateFeatureId("HIHI"), "chatty") {} virtual void Start() override { - // Start() should be called once it's OK to write events - WriteEvent("hello"); + _scope->ExecuteOnContextThread( + [](const CoreContext&, const impl::EventWriter& writer) { writer("hello", {}); } + ); } virtual void Stop() override { - // Stop() should be called before it stops being OK to write events - WriteEvent("goodbye"); + _scope->ExecuteOnContextThread([](const CoreContext&, + const impl::EventWriter& writer) { + writer("goodbye", {}); + }); } }; TEST_CASE("Feature", "[unit]") { - SECTION("M produce events to storage W WriteEvent is called") { + SECTION( + "M produce events to storage W FeatureScope::ExecuteOnContextThread is called" + ) { // Given a running core with a registered feature const bool flush_http_requests = false; CoreTestHarness test = CoreTestHarness::Init(flush_http_requests); diff --git a/tests/impl/features/rum/scopes/action_test.cpp b/tests/impl/features/rum/scopes/action_test.cpp index 3ce426d7..d93f38e5 100644 --- a/tests/impl/features/rum/scopes/action_test.cpp +++ b/tests/impl/features/rum/scopes/action_test.cpp @@ -46,12 +46,8 @@ class ActionFixture { RumEventCapture event_capture; public: - CoreContext GetTestContext() { return event_capture.GetFeatureScope().GetContext(); } - EventWriter GetTestWriter() { - return [this](Block event, Block metadata) { - return event_capture.GetFeatureScope().WriteEvent(event, metadata); - }; - } + CoreContext GetTestContext() { return event_capture.GetContext(); } + EventWriter GetTestWriter() { return event_capture.GetWriter(); } ActionFixture() : config(APPLICATION_ID), deps(config, clock), diff --git a/tests/impl/features/rum/scopes/resource_test.cpp b/tests/impl/features/rum/scopes/resource_test.cpp index 88bc9737..58f028db 100644 --- a/tests/impl/features/rum/scopes/resource_test.cpp +++ b/tests/impl/features/rum/scopes/resource_test.cpp @@ -46,12 +46,8 @@ class ResourceFixture { RumEventCapture event_capture; public: - CoreContext GetTestContext() { return event_capture.GetFeatureScope().GetContext(); } - EventWriter GetTestWriter() { - return [this](Block event, Block metadata) { - return event_capture.GetFeatureScope().WriteEvent(event, metadata); - }; - } + CoreContext GetTestContext() { return event_capture.GetContext(); } + EventWriter GetTestWriter() { return event_capture.GetWriter(); } ResourceFixture() : config(APPLICATION_ID), deps(config, clock), diff --git a/tests/impl/features/rum/scopes/session_test.cpp b/tests/impl/features/rum/scopes/session_test.cpp index d8ef652f..05cc626e 100644 --- a/tests/impl/features/rum/scopes/session_test.cpp +++ b/tests/impl/features/rum/scopes/session_test.cpp @@ -431,12 +431,8 @@ class SessionEventFixture { RumEventCapture event_capture; public: - CoreContext GetTestContext() { return event_capture.GetFeatureScope().GetContext(); } - EventWriter GetTestWriter() { - return [this](Block event, Block metadata) { - return event_capture.GetFeatureScope().WriteEvent(event, metadata); - }; - } + CoreContext GetTestContext() { return event_capture.GetContext(); } + EventWriter GetTestWriter() { return event_capture.GetWriter(); } SessionEventFixture() : config(APPLICATION_ID), deps(config, clock), diff --git a/tests/impl/features/rum/scopes/view_test.cpp b/tests/impl/features/rum/scopes/view_test.cpp index 3354e698..d6c5a4a8 100644 --- a/tests/impl/features/rum/scopes/view_test.cpp +++ b/tests/impl/features/rum/scopes/view_test.cpp @@ -43,12 +43,8 @@ class ViewFixture { RumEventCapture event_capture; public: - CoreContext GetTestContext() { return event_capture.GetFeatureScope().GetContext(); } - EventWriter GetTestWriter() { - return [this](Block event, Block metadata) { - return event_capture.GetFeatureScope().WriteEvent(event, metadata); - }; - } + CoreContext GetTestContext() { return event_capture.GetContext(); } + EventWriter GetTestWriter() { return event_capture.GetWriter(); } ViewFixture() : config(APPLICATION_ID), deps(config, clock), diff --git a/tests/mock/feature.hpp b/tests/mock/feature.hpp index 4def0ec2..8dabfdcf 100644 --- a/tests/mock/feature.hpp +++ b/tests/mock/feature.hpp @@ -81,7 +81,14 @@ class MockFeature : public impl::Feature { * Allows tests to arbitrarily generate events. */ bool GenerateEvent(impl::Block event, impl::Block event_metadata = {}) { - return WriteEvent(event, event_metadata); + if (!_scope) return false; + std::string event_data(event.data(), event.size()); + std::string metadata(event_metadata.data(), event_metadata.size()); + _scope->ExecuteOnContextThread([event_data, metadata]( + const impl::CoreContext&, + const impl::EventWriter& writer + ) { writer(event_data, metadata); }); + return true; } /** diff --git a/tests/support/rum_event_capture.hpp b/tests/support/rum_event_capture.hpp index 7fa116b2..cf1cdd0a 100644 --- a/tests/support/rum_event_capture.hpp +++ b/tests/support/rum_event_capture.hpp @@ -42,6 +42,7 @@ class RumEventCapture { const char* view_id; CoreContextProvider context_provider; + EventGeneratedFunc _event_func; FeatureScope feature_scope; public: @@ -66,34 +67,34 @@ class RumEventCapture { MOCK_OS_INFO, MOCK_DEVICE_INFO )), + _event_func([this](Block event, Block event_metadata) { + // RUM implementation doesn't produce events with metadata + REQUIRE(event_metadata.empty()); + + // Require valid JSON object + auto obj = nlohmann::json::parse(event); + REQUIRE(obj.is_object()); + + // Validate IDs based on event type + if (obj.contains("application") && obj["application"].contains("id")) { + REQUIRE(obj["application"]["id"] == this->application_id); + } + if (obj.contains("session") && obj["session"].contains("id")) { + REQUIRE(obj["session"]["id"] == this->session_id); + } + if (this->view_id != nullptr && obj.contains("view") && + obj["view"].contains("id")) { + REQUIRE(obj["view"]["id"] == this->view_id); + } + + // Capture all events + all_events.emplace_back(std::move(obj)); + return true; + }), feature_scope( FeatureScope::CreateForTesting( context_provider, - [this](Block event, Block event_metadata) { - // RUM implementation doesn't produce events with metadata - REQUIRE(event_metadata.empty()); - - // Require valid JSON object - auto obj = nlohmann::json::parse(event); - REQUIRE(obj.is_object()); - - // Validate IDs based on event type - if (obj.contains("application") && - obj["application"].contains("id")) { - REQUIRE(obj["application"]["id"] == this->application_id); - } - if (obj.contains("session") && obj["session"].contains("id")) { - REQUIRE(obj["session"]["id"] == this->session_id); - } - if (this->view_id != nullptr && obj.contains("view") && - obj["view"].contains("id")) { - REQUIRE(obj["view"]["id"] == this->view_id); - } - - // Capture all events - all_events.emplace_back(std::move(obj)); - return true; - }, + _event_func, DiagnosticLogger( [&](const DiagnosticMessage& message) { switch (message.level) { @@ -122,6 +123,16 @@ class RumEventCapture { */ FeatureScope& GetFeatureScope() { return feature_scope; } + /** + * Returns the current CoreContext snapshot for use as a test argument. + */ + CoreContext GetContext() const { return context_provider.Get(); } + + /** + * Returns the EventWriter for use as a test argument. + */ + const EventGeneratedFunc& GetWriter() const { return _event_func; } + /** * Returns the full set of diagnostic messages that were emitted via this object's * FeatureScope since construction. From 37d09465d43b0c11a76130d37ac943d010ddefe7 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Wed, 11 Mar 2026 10:03:59 -0400 Subject: [PATCH 20/25] chore: Add tests for context-queue shutdown behavior --- tests/impl/core/feature_scope_test.cpp | 45 ++---------------- tests/impl/core/feature_test.cpp | 63 +++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 43 deletions(-) diff --git a/tests/impl/core/feature_scope_test.cpp b/tests/impl/core/feature_scope_test.cpp index 62dbedb8..52e24791 100644 --- a/tests/impl/core/feature_scope_test.cpp +++ b/tests/impl/core/feature_scope_test.cpp @@ -300,6 +300,9 @@ TEST_CASE("FeatureScope", "[unit][core]") { } SECTION("M drain queue and execute all enqueued functions W stopped") { + // TODO(RUM-15042): Currently the SDK flushes the context queue on shutdown; if this + // changes, this test will need to change or else use a test-only flush method + // Given a single feature with a FeatureScope FeatureState feature; FeatureScope scope = feature.CreateScope(context_provider, context_queue); @@ -335,45 +338,3 @@ TEST_CASE("FeatureScope", "[unit][core]") { } } } - -/* -TEST_CASE("FeatureScope shutdown safety", "[unit][core][shutdown]") { - SECTION("M not use FeatureScope after destruction W lambdas queued on shutdown") { - // Given a CoreContextProvider - CoreContextProvider context_provider(MOCK_CONTEXT); - - // And a context queue with a one-shot gate to hold the context thread - Queue> queue; - std::promise gate; - std::future gate_signal = gate.get_future(); - - // Start a context thread that processes the queue - std::thread context_thread([&]() { - while (auto thunk = queue.Pop()) { - (*thunk)(); - } - }); - - // Block the context thread by pushing a gating lambda first - queue.Push([&]() { gate_signal.wait(); }); - - // Now push real work via FeatureScope - these sit behind the gate - auto scope = std::make_unique(FeatureScope::Create( - context_provider, [](Block, Block) { return true; }, DiagnosticLogger{}, queue - )); - scope->ExecuteOnContextThread([](auto&, auto&) {}); - scope->ExecuteOnContextThread([](auto&, auto&) {}); - scope->UpdateContext([](CoreContext&) {}); - - // Destroy FeatureScope while those lambdas are still queued - scope.reset(); - - // Release the gate - context thread will now execute the lambdas - // with dangling `this` (ASAN will catch the use-after-free here) - gate.set_value(); - - queue.Stop(); - context_thread.join(); - } -} -*/ \ No newline at end of file diff --git a/tests/impl/core/feature_test.cpp b/tests/impl/core/feature_test.cpp index f87c64cf..e023e67f 100644 --- a/tests/impl/core/feature_test.cpp +++ b/tests/impl/core/feature_test.cpp @@ -7,12 +7,15 @@ #include "datadog/impl/core/feature.hpp" #include -#include +#include +#include #include +#include #include #include "mock/feature.hpp" #include "mock/tlv.hpp" +#include "support/catch.hpp" #include "support/core.hpp" #include "support/threading.hpp" @@ -21,6 +24,12 @@ using namespace datadog::impl; class CoolFeature : public MockFeature { public: CoolFeature() : MockFeature(CreateFeatureId("COOL"), "coolfeature") {} + + void BlockContextThread(std::future& gate_signal) { + _scope->ExecuteOnContextThread([&](const CoreContext&, EventGeneratedFunc) { + gate_signal.wait(); + }); + } }; class ChattyFeature : public MockFeature { @@ -247,4 +256,56 @@ TEST_CASE("Feature thread-safety", "[unit][core][thread-safety]") { REQUIRE(events[5000] == "main:00"); REQUIRE(events[5099] == "main:99"); } + + SECTION( + "M finish executing all queued functions W Core is stopped with a non-empty " + "context queue" + ) { + // TODO(RUM-15042): This test validates the existing behavior, where the SDK drains + // the context queue on shutdown, which can cause blocking shutdown time to scale + // with the number of SDK operations still pending. If we make shutdown + // non-blocking, this test will need to change, or else validate the new test-only + // flush-and-stop function. + + // Given a started core with a registered feature + CoreTestHarness test = CoreTestHarness::Init(); + auto feature = std::make_shared(); + REQUIRE(test.core.RegisterFeature(feature)); + REQUIRE(test.core.Start()); + + // And a promise that will block the context thread until we explicitly resolve it + std::promise gate; + std::future gate_signal = gate.get_future(); + + // When we enqueue a context-thread function that will block indefinitely, pausing + // context thread processing until we call gate.set_value() + feature->BlockContextThread(gate_signal); + + // And then we enqueue 500 events for write + for (uint64_t i = 0; i < 500; i++) { + feature->GenerateEvent("A"); + } + + // And spawn a background thread that will unblock the context thread (since our + // Stop() call below is blocking) + std::thread t{[&]() { + std::this_thread::sleep_for(std::chrono::microseconds(1)); + gate.set_value(); + }}; + + // And then we shut down the SDK + test.core.Stop(); + t.join(); + + // Then all events should have been generated and sent + size_t num_events = 0; + for (MockHttpRequest& req : test.client.requests) { + for (char c : req.body) { + if (c == 'A') { + num_events++; + } + } + } + REQUIRE(num_events == 500); + } } From 1dc05e97db44485d33219ceb708c9c7a94915f36 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Wed, 11 Mar 2026 10:07:13 -0400 Subject: [PATCH 21/25] fix: Fix shutdown ordering: stop context thread before stopping features --- src/datadog/impl/core/core.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/datadog/impl/core/core.cpp b/src/datadog/impl/core/core.cpp index 957dc818..63669b3a 100644 --- a/src/datadog/impl/core/core.cpp +++ b/src/datadog/impl/core/core.cpp @@ -395,11 +395,6 @@ void Core::Stop() { } _diagnostic_logger.Debug("Beginning Core shutdown"); - // Notify each registered feature that the core has stopped - for (const auto& feature : _features) { - feature.impl->OnCoreStopping(); - } - // If we were previously started, all background threads should be running DATADOG_ASSERT(_context_queue, "_context_queue is invalid on Stop"); DATADOG_ASSERT( @@ -419,13 +414,23 @@ void Core::Stop() { // Stop the context queue, then block until the context thread drains the queue and // exits. This ensures all pending feature work completes before we stop the storage - // thread. + // thread or tear down feature state. + // TODO(RUM-15042): Other SDKs abandon the context thread and shut down in a + // non-blocking fashion, without draining the context queue _context_queue->Stop(); if (_context_thread) { _diagnostic_logger.Debug("Joining on context thread"); _context_thread->join(); } _context_thread.reset(); + + // Notify each registered feature that the core has stopped, now that no more + // context-thread functions enqueued by those features may be running + for (const auto& feature : _features) { + feature.impl->OnCoreStopping(); + } + + // Destroy the context queue, now that no more features exist _context_queue.reset(); // Stop all queue processing, then block until the consumer thread drains the queue From 77dfaa9f1a39ff93f4b8e091f8d1cfba2cfe4cc6 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Wed, 11 Mar 2026 10:51:57 -0400 Subject: [PATCH 22/25] fix: Events may no longer be generated on Feature::Stop() This function is used to tear down feature state; it may not be used to generate events or perform any work on the context thread (which is now stopped before the call to Feature::Stop()). This is consistent with the behavior of the iOS SDK, which treats Stop() as an immediate, non-blocking shutdown, abandoning all work on the context thread rather than draining the queue. --- src/datadog/impl/core/feature.cpp | 5 +---- src/datadog/impl/core/feature.hpp | 19 ++++++++++--------- tests/impl/core/feature_test.cpp | 27 +++++++++------------------ 3 files changed, 20 insertions(+), 31 deletions(-) diff --git a/src/datadog/impl/core/feature.cpp b/src/datadog/impl/core/feature.cpp index 3baeae2f..955cbb8a 100644 --- a/src/datadog/impl/core/feature.cpp +++ b/src/datadog/impl/core/feature.cpp @@ -20,8 +20,7 @@ void Feature::OnCoreStarted(FeatureScope&& feature_scope) { } void Feature::OnCoreStopping() { - // Notify the feature that the core is about to stop, while it's still able to write - // events + // Notify the feature that the core is stopping Stop(); // Drop our FeatureScope; the Core is stopped so we must stop all work @@ -29,6 +28,4 @@ void Feature::OnCoreStopping() { _scope.reset(); } -bool Feature::IsRunning() const { return _scope.has_value(); } - } // namespace datadog::impl diff --git a/src/datadog/impl/core/feature.hpp b/src/datadog/impl/core/feature.hpp index e47ce170..6ee1d4e3 100644 --- a/src/datadog/impl/core/feature.hpp +++ b/src/datadog/impl/core/feature.hpp @@ -110,22 +110,23 @@ class Feature : public std::enable_shared_from_this { protected: /** * Called from the main thread when the SDK has finished starting up. This is the - * first point at which events may be generated. + * first point at which _scope is valid. + * + * Once Start() is called, the feature may enqueue work on the context thread via + * _scope->ExecuteOnContextThread(), generating events in the process, until Stop() is + * called. */ virtual void Start() {} /** - * Called from the main thread when the SDK is about to shut down. This is the last - * point at which events may be generated. + * Called from the main thread when the SDK is shutting down. This is the last point + * at which _scope is valid. + * + * Once Stop() is called, any work enqueued via _scope->ExecuteOnContextThread() will + * be ignored, and the feature may no longer generate events. */ virtual void Stop() {} - /** - * @returns whether the feature has received a call to Start() and has not yet - * received a call to Stop(). - */ - bool IsRunning() const; - public: /** * Called from the upload thread when a batch of events written to storage by this diff --git a/tests/impl/core/feature_test.cpp b/tests/impl/core/feature_test.cpp index e023e67f..eaa55088 100644 --- a/tests/impl/core/feature_test.cpp +++ b/tests/impl/core/feature_test.cpp @@ -41,13 +41,6 @@ class ChattyFeature : public MockFeature { [](const CoreContext&, const impl::EventWriter& writer) { writer("hello", {}); } ); } - - virtual void Stop() override { - _scope->ExecuteOnContextThread([](const CoreContext&, - const impl::EventWriter& writer) { - writer("goodbye", {}); - }); - } }; TEST_CASE("Feature", "[unit]") { @@ -111,9 +104,9 @@ TEST_CASE("Feature", "[unit]") { REQUIRE(!ok); } - SECTION("M be able to produce events W core is started or stopping") { + SECTION("M be able to produce events immediately W core is started") { // Given an initialized core with a registered feature that generates events in - // response to core start and stop + // response to core start const bool flush_http_requests = false; CoreTestHarness test = CoreTestHarness::Init(flush_http_requests); auto feature = std::make_shared(); @@ -124,15 +117,12 @@ TEST_CASE("Feature", "[unit]") { REQUIRE(feature->GenerateEvent("nice weather today")); test.core.Stop(); - // Then the resulting batch file should contain all events, including those - // generated on start and on stop, in the correct order + // Then the resulting batch file should contain both events: the one generated on + // start, and then the one we generated explicitly std::vector relpaths = test.storage.FindFiles("chatty/v1"); REQUIRE(relpaths.size() == 1); - std::string expected = MockTLVFile() - .AppendEvent("hello") - .AppendEvent("nice weather today") - .AppendEvent("goodbye") - .ToString(); + std::string expected = + MockTLVFile().AppendEvent("hello").AppendEvent("nice weather today").ToString(); REQUIRE(test.storage.Cat(relpaths.front()) == expected); } @@ -150,12 +140,13 @@ TEST_CASE("Feature", "[unit]") { REQUIRE(feature->GenerateEvent("nice weather today")); test.core.Stop(); - // Then core should have successfully uploaded our feature + // Then core should have successfully uploaded our feature's events from a batch + // file REQUIRE(test.client.requests.size() == 1); REQUIRE(test.storage.GetNumFilesDeleted() == 1); const MockHttpRequest& req = test.client.requests.front(); REQUIRE(!req.aborted); - REQUIRE(req.body == "hello,nice weather today,goodbye"); + REQUIRE(req.body == "hello,nice weather today"); // And the batch file should have been deleted on successful upload std::vector relpaths = test.storage.FindFiles("chatty/v1"); From 4e679de1c38b7c52b7093114fde8142c380b8378 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Wed, 11 Mar 2026 11:13:42 -0400 Subject: [PATCH 23/25] chore: Replace `EventGeneratedFunc` with `EventWriter` We don't need two separate definitions of the same function type; it's clearer to just use `EventWriter` everywhere. --- src/datadog/impl/core/core.cpp | 8 ++----- src/datadog/impl/core/feature_scope.cpp | 12 +++++------ src/datadog/impl/core/feature_scope.hpp | 22 +++++++------------ tests/impl/core/feature_scope_test.cpp | 28 ++++++++++++++----------- tests/impl/core/feature_test.cpp | 2 +- tests/support/feature.hpp | 4 ++-- tests/support/rum_event_capture.hpp | 4 ++-- 7 files changed, 36 insertions(+), 44 deletions(-) diff --git a/src/datadog/impl/core/core.cpp b/src/datadog/impl/core/core.cpp index 63669b3a..2d25e67a 100644 --- a/src/datadog/impl/core/core.cpp +++ b/src/datadog/impl/core/core.cpp @@ -372,16 +372,12 @@ bool Core::Start() { // FeatureScope interface that it can use to interoperate with the core for (const auto& feature : _features) { const FeatureId id = feature.id; - EventGeneratedFunc event_generated_func = - [this, id](Block event, Block event_metadata) -> bool { + EventWriter event_writer = [this, id](Block event, Block event_metadata) -> bool { return EnqueueStorageWrite(id, event, event_metadata); }; feature.impl->OnCoreStarted( FeatureScope::Create( - *_context_provider, - event_generated_func, - _diagnostic_logger, - *_context_queue + *_context_provider, event_writer, _diagnostic_logger, *_context_queue ) ); } diff --git a/src/datadog/impl/core/feature_scope.cpp b/src/datadog/impl/core/feature_scope.cpp index d8b67f50..ef286a97 100644 --- a/src/datadog/impl/core/feature_scope.cpp +++ b/src/datadog/impl/core/feature_scope.cpp @@ -12,13 +12,13 @@ namespace datadog::impl { FeatureScope::FeatureScope( CoreContextProvider& context_provider, - const EventGeneratedFunc& event_generated_func, + const EventWriter& event_writer, const DiagnosticLogger& in_diagnostic_logger, FeatureScope::ExecutionMode mode, Queue>* context_queue ) : _context_provider(&context_provider), - _event_generated_func(event_generated_func), + _event_generated_func(event_writer), _mode(mode), _context_queue(context_queue), diagnostic_logger(in_diagnostic_logger) { @@ -31,13 +31,13 @@ FeatureScope::FeatureScope( FeatureScope FeatureScope::Create( CoreContextProvider& context_provider, - const EventGeneratedFunc& event_generated_func, + const EventWriter& event_writer, const DiagnosticLogger& diagnostic_logger, Queue>& context_queue ) { return FeatureScope( context_provider, - event_generated_func, + event_writer, diagnostic_logger, FeatureScope::ExecutionMode::OnContextThread, &context_queue @@ -46,12 +46,12 @@ FeatureScope FeatureScope::Create( FeatureScope FeatureScope::CreateForTesting( CoreContextProvider& context_provider, - const EventGeneratedFunc& event_generated_func, + const EventWriter& event_writer, const DiagnosticLogger& diagnostic_logger ) { return FeatureScope( context_provider, - event_generated_func, + event_writer, diagnostic_logger, FeatureScope::ExecutionMode::Synchronous, nullptr diff --git a/src/datadog/impl/core/feature_scope.hpp b/src/datadog/impl/core/feature_scope.hpp index 6a3586f9..b24fa146 100644 --- a/src/datadog/impl/core/feature_scope.hpp +++ b/src/datadog/impl/core/feature_scope.hpp @@ -17,8 +17,7 @@ namespace datadog::impl { /** - * Callback invoked by a feature when it generates an event that needs to be enqueued - * for storage. + * Callback that writes an event to storage from the context thread. * * @param event Arbitrary bytes. Must be non-empty. Will be copied into the storage * queue. Eventually written to a batch with TLVBlockType::Event. @@ -27,13 +26,6 @@ namespace datadog::impl { * a block of type TLVBlockType::Metadata. * @returns whether the event was successfuly enqueued for storage. */ -using EventGeneratedFunc = std::function; - -/** - * Callback that writes an event to storage from the context thread. - * - * Returns whether the event was successfully enqueued. - */ using EventWriter = std::function; /** @@ -59,7 +51,7 @@ class FeatureScope { }; CoreContextProvider* _context_provider; - EventGeneratedFunc _event_generated_func; + EventWriter _event_generated_func; ExecutionMode _mode; Queue>* _context_queue; @@ -69,7 +61,7 @@ class FeatureScope { */ explicit FeatureScope( CoreContextProvider& context_provider, - const EventGeneratedFunc& event_generated_func, + const EventWriter& event_writer, const DiagnosticLogger& in_diagnostic_logger, ExecutionMode mode, Queue>* context_queue @@ -91,14 +83,14 @@ class FeatureScope { * execution on the SDK's context thread. * * @param context_provider Provides thread-safe access to CoreContext - * @param event_generated_func Callback invoked when features generate events + * @param event_writer Callback invoked when features generate events * @param diagnostic_logger Logger for diagnostic messages * @param context_queue Queue containing thunks to be executed serially by the context * thread; must outlive the FeatureScope */ static FeatureScope Create( CoreContextProvider& context_provider, - const EventGeneratedFunc& event_generated_func, + const EventWriter& event_writer, const DiagnosticLogger& diagnostic_logger, Queue>& context_queue ); @@ -109,12 +101,12 @@ class FeatureScope { * thread. * * @param context_provider Provides thread-safe access to CoreContext - * @param event_generated_func Callback invoked when features generate events + * @param event_writer Callback invoked when features generate events * @param diagnostic_logger Logger for diagnostic messages */ static FeatureScope CreateForTesting( CoreContextProvider& context_provider, - const EventGeneratedFunc& event_generated_func, + const EventWriter& event_writer, const DiagnosticLogger& diagnostic_logger ); diff --git a/tests/impl/core/feature_scope_test.cpp b/tests/impl/core/feature_scope_test.cpp index 52e24791..ffbd34e2 100644 --- a/tests/impl/core/feature_scope_test.cpp +++ b/tests/impl/core/feature_scope_test.cpp @@ -35,7 +35,7 @@ static const size_t MAX_EVENTS = 512; * Stand-in for a Feature that interacts with the FeatureScope in order to read * CoreContext, mutate CoreContext, and generate events. * - * Provides an EventGeneratedFunc that buffers the event into a thread-safe array with a + * Provides an EventWriter that buffers the event into a thread-safe array with a * fixed capacity. */ struct FeatureState { @@ -96,11 +96,15 @@ static void WriteContextValue(CoreContext& ctx, uint64_t value) { ctx.rum->session_id = new_session_id; } -static bool GenerateUInt64Event(const EventGeneratedFunc& func, uint64_t value) { +/** + * Serializes `value` as a string and passes it to the given EventWriter function, + * thereby producing an event whose binary value is a string-encoded uint64_t. + */ +static bool GenerateUInt64Event(const EventWriter& event_writer, uint64_t value) { char buf[20]; auto res = std::to_chars(buf, buf + sizeof(buf), value); REQUIRE(res.ec == std::errc{}); - return func(Block(buf, res.ptr - buf), {}); + return event_writer(Block(buf, res.ptr - buf), {}); } /** @@ -149,7 +153,7 @@ TEST_CASE("FeatureScope", "[unit][core]") { // context thread int num_executions = 0; auto main_thread_id = std::this_thread::get_id(); - scope.ExecuteOnContextThread([&](const CoreContext&, EventGeneratedFunc) { + scope.ExecuteOnContextThread([&](const CoreContext&, EventWriter) { num_executions++; // Then our function is executed on a background thread @@ -179,7 +183,7 @@ TEST_CASE("FeatureScope", "[unit][core]") { // And then we enqueue a read-only function that will run thereafter int num_executions = 0; - scope.ExecuteOnContextThread([&](const CoreContext& ctx, EventGeneratedFunc) { + scope.ExecuteOnContextThread([&](const CoreContext& ctx, EventWriter) { // Then the CoreContext snapshot passed to our second function reflects the // modifications made by the first REQUIRE(ReadContextValue(ctx) == 0xbeef); @@ -212,12 +216,12 @@ TEST_CASE("FeatureScope", "[unit][core]") { generation++; WriteContextValue(ctx, generation); }; - auto consume = [](const CoreContext& ctx, EventGeneratedFunc event_generated_func) { + auto consume = [](const CoreContext& ctx, EventWriter event_writer) { const uint64_t value = ReadContextValue(ctx); char buf[20]; auto res = std::to_chars(buf, buf + sizeof(buf), value); REQUIRE(res.ec == std::errc{}); - event_generated_func(Block(buf, res.ptr - buf), {}); + event_writer(Block(buf, res.ptr - buf), {}); }; mutate_scope.UpdateContext(mutate); // Stores generation 1 in context @@ -267,9 +271,9 @@ TEST_CASE("FeatureScope", "[unit][core]") { // On each iteration, enqueue a function that will run on the context thread and // produce an event with the latest value read from CoreContext scope.ExecuteOnContextThread([](const CoreContext& ctx, - EventGeneratedFunc event_generated_func) { + EventWriter event_writer) { // Produce an event whose payload is just a string version of our value - GenerateUInt64Event(event_generated_func, ReadContextValue(ctx)); + GenerateUInt64Event(event_writer, ReadContextValue(ctx)); }); } }); @@ -310,14 +314,14 @@ TEST_CASE("FeatureScope", "[unit][core]") { // And a promise that will block the context thread until we explicitly resolve it std::promise gate; std::future gate_signal = gate.get_future(); - scope.ExecuteOnContextThread([&](const CoreContext&, EventGeneratedFunc) { + scope.ExecuteOnContextThread([&](const CoreContext&, EventWriter) { gate_signal.wait(); }); // When we enqueue a bunch of functions to run on the context thread for (size_t i = 0; i < MAX_EVENTS; i++) { - scope.ExecuteOnContextThread([i](const CoreContext&, EventGeneratedFunc func) { - GenerateUInt64Event(func, i); + scope.ExecuteOnContextThread([i](const CoreContext&, EventWriter event_writer) { + GenerateUInt64Event(event_writer, i); }); } diff --git a/tests/impl/core/feature_test.cpp b/tests/impl/core/feature_test.cpp index eaa55088..2580eb92 100644 --- a/tests/impl/core/feature_test.cpp +++ b/tests/impl/core/feature_test.cpp @@ -26,7 +26,7 @@ class CoolFeature : public MockFeature { CoolFeature() : MockFeature(CreateFeatureId("COOL"), "coolfeature") {} void BlockContextThread(std::future& gate_signal) { - _scope->ExecuteOnContextThread([&](const CoreContext&, EventGeneratedFunc) { + _scope->ExecuteOnContextThread([&](const CoreContext&, EventWriter) { gate_signal.wait(); }); } diff --git a/tests/support/feature.hpp b/tests/support/feature.hpp index 4c1f27dd..16c7afa2 100644 --- a/tests/support/feature.hpp +++ b/tests/support/feature.hpp @@ -47,7 +47,7 @@ class FeatureTest { explicit FeatureTest(const CoreContext& context) : _context_provider(context) {} void Start(const std::shared_ptr& feature) { - auto event_generated_func = [this](Block event, Block event_metadata) { + auto event_writer = [this](Block event, Block event_metadata) { events.emplace_back(event, event_metadata); return true; }; @@ -57,7 +57,7 @@ class FeatureTest { feature->OnCoreStarted( FeatureScope::CreateForTesting( _context_provider, - event_generated_func, + event_writer, DiagnosticLogger(diagnostic_handler, DiagnosticLevel::Debug) ) ); diff --git a/tests/support/rum_event_capture.hpp b/tests/support/rum_event_capture.hpp index cf1cdd0a..58c46777 100644 --- a/tests/support/rum_event_capture.hpp +++ b/tests/support/rum_event_capture.hpp @@ -42,7 +42,7 @@ class RumEventCapture { const char* view_id; CoreContextProvider context_provider; - EventGeneratedFunc _event_func; + EventWriter _event_func; FeatureScope feature_scope; public: @@ -131,7 +131,7 @@ class RumEventCapture { /** * Returns the EventWriter for use as a test argument. */ - const EventGeneratedFunc& GetWriter() const { return _event_func; } + const EventWriter& GetWriter() const { return _event_func; } /** * Returns the full set of diagnostic messages that were emitted via this object's From e19810fd3a3df3555149e9047a8e2689ebe55f44 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Wed, 11 Mar 2026 11:29:17 -0400 Subject: [PATCH 24/25] chore: Remove CoreContextProvider arg from ContextThreadMain --- src/datadog/impl/core/context_thread.cpp | 4 +--- src/datadog/impl/core/context_thread.hpp | 18 ++++++++---------- src/datadog/impl/core/core.cpp | 5 +---- src/datadog/impl/features/rum/rum.cpp | 1 - tests/impl/core/context_thread_test.cpp | 4 ++-- tests/impl/core/feature_scope_test.cpp | 5 +---- 6 files changed, 13 insertions(+), 24 deletions(-) diff --git a/src/datadog/impl/core/context_thread.cpp b/src/datadog/impl/core/context_thread.cpp index 89aaa169..c15f202e 100644 --- a/src/datadog/impl/core/context_thread.cpp +++ b/src/datadog/impl/core/context_thread.cpp @@ -9,9 +9,7 @@ namespace datadog::impl { void ContextThreadMain( - const DiagnosticLogger& diagnostic_logger, - Queue>& queue, - CoreContextProvider& /* context_provider */ + const DiagnosticLogger& diagnostic_logger, Queue>& queue ) { diagnostic_logger.Debug("Context thread starting"); diff --git a/src/datadog/impl/core/context_thread.hpp b/src/datadog/impl/core/context_thread.hpp index c5a44835..525f97c7 100644 --- a/src/datadog/impl/core/context_thread.hpp +++ b/src/datadog/impl/core/context_thread.hpp @@ -17,23 +17,21 @@ namespace datadog::impl { /** * Entry point for the context thread. * - * The context thread processes functions submitted by features, providing each - * function with a snapshot of the CoreContext at execution time. This allows - * features to perform operations asynchronously without blocking their callers. + * 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. * - * The thread runs until the queue is stopped and drained, processing functions in - * FIFO order. + * 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. - * @param context_provider Non-owning reference to the CoreContextProvider that - * supplies context snapshots; guaranteed to outlive the thread. */ void ContextThreadMain( - const DiagnosticLogger& diagnostic_logger, - Queue>& queue, - CoreContextProvider& context_provider + const DiagnosticLogger& diagnostic_logger, Queue>& queue ); } // namespace datadog::impl diff --git a/src/datadog/impl/core/core.cpp b/src/datadog/impl/core/core.cpp index 2d25e67a..faa7c2bf 100644 --- a/src/datadog/impl/core/core.cpp +++ b/src/datadog/impl/core/core.cpp @@ -323,10 +323,7 @@ bool Core::Start() { // Start the context thread that will execute functions submitted by features DATADOG_ASSERT(!_context_thread, "_context_thread already exists on Start()"); _context_thread = std::thread( - ContextThreadMain, - std::ref(_diagnostic_logger), - std::ref(*_context_queue), - std::ref(*_context_provider) + ContextThreadMain, std::ref(_diagnostic_logger), std::ref(*_context_queue) ); // Initialize an upload scheduler to manage the timing of upload cycles for each diff --git a/src/datadog/impl/features/rum/rum.cpp b/src/datadog/impl/features/rum/rum.cpp index 50bc7590..41072663 100644 --- a/src/datadog/impl/features/rum/rum.cpp +++ b/src/datadog/impl/features/rum/rum.cpp @@ -52,7 +52,6 @@ std::optional Rum::UploadThread_PrepareReport( void Rum::Start() { // Fully reinitialize RUM application state to clear all sessions/views/etc. from // previous runs - // Fully reinitialize RUM application state _application = RumApplicationScope(_deps); // Dispatch SDKInit to start first session diff --git a/tests/impl/core/context_thread_test.cpp b/tests/impl/core/context_thread_test.cpp index 00ef3b36..596da205 100644 --- a/tests/impl/core/context_thread_test.cpp +++ b/tests/impl/core/context_thread_test.cpp @@ -30,7 +30,7 @@ TEST_CASE("ContextThreadMain", "[unit]") { SECTION("M log debug messages W started and stopped cleanly") { // When we run ContextThreadMain synchronously on an empty, stopped queue context_queue.Stop(); - ContextThreadMain(diagnostic_logger, context_queue, context_provider); + ContextThreadMain(diagnostic_logger, context_queue); // Then we should have exactly two debug messages that signal thread start and stop REQUIRE(diagnostics.debug.size() == 2); @@ -48,7 +48,7 @@ TEST_CASE("ContextThreadMain", "[unit]") { context_queue.Push([&x]() { x++; }); context_queue.Push([&x]() { x++; }); context_queue.Stop(); - ContextThreadMain(diagnostic_logger, context_queue, context_provider); + ContextThreadMain(diagnostic_logger, context_queue); // Then we should have no unexpected diagnostic output REQUIRE(diagnostics.debug.size() == 2); diff --git a/tests/impl/core/feature_scope_test.cpp b/tests/impl/core/feature_scope_test.cpp index ffbd34e2..179db6b1 100644 --- a/tests/impl/core/feature_scope_test.cpp +++ b/tests/impl/core/feature_scope_test.cpp @@ -134,10 +134,7 @@ TEST_CASE("FeatureScope", "[unit][core]") { // executes them serially in the background Queue> context_queue; std::thread context_thread{ - ContextThreadMain, - std::ref(diagnostic_logger), - std::ref(context_queue), - std::ref(context_provider) + ContextThreadMain, std::ref(diagnostic_logger), std::ref(context_queue) }; auto stop_context_thread = [&]() { context_queue.Stop(); From fcbdf93a46814c687b3f076c11aea4aff822db89 Mon Sep 17 00:00:00 2001 From: Alex Forsythe Date: Wed, 11 Mar 2026 11:53:16 -0400 Subject: [PATCH 25/25] fix: Reset RumFeatureContext etc. on Core::Start, not on Feature::Stop Previously, each feature that stored custom state in `CoreContext` (in practice, just `Rum`) was responsible for clearing its own `CoreContext` state in `Feature::Stop()` so that, for example, if you stop and SDK instance and then restart it, you won't accidentally use stale RUM IDs from its past life. That was a simple enough approach when `CoreContext` mutations happened synchronously, but now that we're using a context thread (which has to be shut down before `Feature::Stop()`), this approach no longer makes sense. It's much simpler to just have the Core itself reset the `CoreContext` value each time it starts. This guarantees that the SDK will always use a clean `CoreContext` on `Start()`, regardless of whether it's stopped and restarted, and without requiring any feature-specific cleanup logic or tricky synchronization on shutdown. --- src/datadog/impl/core/context.cpp | 2 + src/datadog/impl/core/context.hpp | 7 +++ src/datadog/impl/core/core.cpp | 5 ++ src/datadog/impl/features/rum/rum.cpp | 9 +--- tests/impl/features/logging/logger_test.cpp | 56 +++++++++++---------- tests/impl/features/rum/rum_test.cpp | 19 ++++--- tests/support/feature.hpp | 8 +++ 7 files changed, 67 insertions(+), 39 deletions(-) diff --git a/src/datadog/impl/core/context.cpp b/src/datadog/impl/core/context.cpp index 166425ba..cd16055c 100644 --- a/src/datadog/impl/core/context.cpp +++ b/src/datadog/impl/core/context.cpp @@ -175,6 +175,8 @@ CoreContext::CoreContext( ) : http(std::make_shared(config)), os(&os_info), device(&device_info) {} +void CoreContext::Reset() { rum.reset(); } + CoreContextProvider::CoreContextProvider(const CoreContext& context) : _context(context) {} diff --git a/src/datadog/impl/core/context.hpp b/src/datadog/impl/core/context.hpp index e3a6df80..d66c6407 100644 --- a/src/datadog/impl/core/context.hpp +++ b/src/datadog/impl/core/context.hpp @@ -143,6 +143,13 @@ struct CoreContext { * Additional context provided by the RUM feature, if in use. */ std::optional rum; + + /** + * Resets all mutable feature-specific context fields to their default (absent) state. + * Called by Core at the start of each run to ensure features never inherit stale + * context from a previous run. + */ + void Reset(); }; /** diff --git a/src/datadog/impl/core/core.cpp b/src/datadog/impl/core/core.cpp index faa7c2bf..ac976702 100644 --- a/src/datadog/impl/core/core.cpp +++ b/src/datadog/impl/core/core.cpp @@ -299,6 +299,11 @@ bool Core::Start() { return false; } + // Reset any feature-specific context left over from a previous run, so that features + // start clean rather than inheriting stale state from the prior session + DATADOG_ASSERT(_context_provider, "_context_provider is null on Start()"); + _context_provider->Update([](CoreContext& ctx) { ctx.Reset(); }); + // Initialize a thread-safe queue that features can write to whenever they produce // events that need to be written to disk DATADOG_ASSERT(!_storage_queue, "_storage_queue already exists on Start()"); diff --git a/src/datadog/impl/features/rum/rum.cpp b/src/datadog/impl/features/rum/rum.cpp index 41072663..2e1ec7bb 100644 --- a/src/datadog/impl/features/rum/rum.cpp +++ b/src/datadog/impl/features/rum/rum.cpp @@ -59,15 +59,10 @@ void Rum::Start() { }; void Rum::Stop() { - // On Core shutdown, ensure that any RUM context is purged, so that if the SDK is - // restarted it won't inherit old state - if (_scope) { - _scope->UpdateContext([](CoreContext& ctx) { ctx.rum.reset(); }); - } - // Note: _application will be destroyed when Rum is destroyed (after Core joins // the context thread). In-flight lambdas can safely access _application as long as - // weak_ptr.lock() succeeds. + // weak_ptr.lock() succeeds. CoreContext is reset by Core::Start() at the top of each + // new run, so no context mutation is needed here. } void Rum::AddAttribute(std::string_view name, const Attribute& value) { diff --git a/tests/impl/features/logging/logger_test.cpp b/tests/impl/features/logging/logger_test.cpp index 1ef31893..f4126f8e 100644 --- a/tests/impl/features/logging/logger_test.cpp +++ b/tests/impl/features/logging/logger_test.cpp @@ -58,17 +58,19 @@ TEST_CASE("Logger RUM enrichment", "[unit][logging]") { auto logging = std::make_shared(clock, "mock-service", "1.0.0"); auto logger = logging->CreateLogger(LoggerConfig()); - // And a context that includes RUM session details - CoreContext context(CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO); - context.rum = RumFeatureContext(); - context.rum->application_id = uuid_9916; - context.rum->session_id = uuid_d927; - context.rum->view_id = uuid_e27c; - context.rum->action_id = uuid_cfa4; - - // When we emit a log event while the SDK is running - FeatureTest test(context); + // When the SDK starts, then RUM publishes its context (simulating the async update + // that RUM enqueues after OnCoreStarted) + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); test.Start(logging); + test.UpdateContext([&](CoreContext& ctx) { + ctx.rum = RumFeatureContext(); + ctx.rum->application_id = uuid_9916; + ctx.rum->session_id = uuid_d927; + ctx.rum->view_id = uuid_e27c; + ctx.rum->action_id = uuid_cfa4; + }); + + // And we emit a log event logger->EmitLogEvent(LogLevel::Info, "hello"); test.Stop(logging); @@ -89,16 +91,17 @@ TEST_CASE("Logger RUM enrichment", "[unit][logging]") { auto logging = std::make_shared(clock, "mock-service", "1.0.0"); auto logger = logging->CreateLogger(LoggerConfig()); - // And a context that includes a RUM application ID and session ID, but no view or - // action ID - CoreContext context(CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO); - context.rum = RumFeatureContext(); - context.rum->application_id = uuid_9916; - context.rum->session_id = uuid_d927; - - // When we emit a log event while the SDK is running - FeatureTest test(context); + // When the SDK starts, then RUM publishes a context with an application ID and + // session ID but no view or action ID + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); test.Start(logging); + test.UpdateContext([&](CoreContext& ctx) { + ctx.rum = RumFeatureContext(); + ctx.rum->application_id = uuid_9916; + ctx.rum->session_id = uuid_d927; + }); + + // And we emit a log event logger->EmitLogEvent(LogLevel::Info, "hello"); test.Stop(logging); @@ -119,18 +122,19 @@ TEST_CASE("Logger RUM enrichment", "[unit][logging]") { auto logging = std::make_shared(clock, "mock-service", "1.0.0"); auto logger = logging->CreateLogger(LoggerConfig()); - // And a context that includes a RUM application ID, but no session - CoreContext context(CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO); - context.rum = RumFeatureContext(); - context.rum->application_id = uuid_9916; - // And user attributes named 'application_id' and 'session_id' logger->AddAttribute("application_id", Attribute::String("user-application-id")); logger->AddAttribute("session_id", Attribute::String("user-session-id")); - // When we emit a log event while the SDK is running - FeatureTest test(context); + // When the SDK starts, then RUM publishes a context with only an application ID + FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); test.Start(logging); + test.UpdateContext([&](CoreContext& ctx) { + ctx.rum = RumFeatureContext(); + ctx.rum->application_id = uuid_9916; + }); + + // And we emit a log event logger->EmitLogEvent(LogLevel::Info, "hello"); test.Stop(logging); diff --git a/tests/impl/features/rum/rum_test.cpp b/tests/impl/features/rum/rum_test.cpp index 86e9c972..eaeecdd8 100644 --- a/tests/impl/features/rum/rum_test.cpp +++ b/tests/impl/features/rum/rum_test.cpp @@ -54,21 +54,28 @@ TEST_CASE("Rum context population", "[unit][rum]") { REQUIRE(context.rum->action_id == UUID::Zero); } - SECTION("M clear RumFeatureContext W SDK stops") { - // Given a running RUM feature which has populated a RumFeatureContext + SECTION("M clear RumFeatureContext W SDK restarts") { + // Given a RUM feature that has been started, producing a session MockClock clock; FeatureTest test(CoreContext{CORE_CONFIG, MOCK_OS_INFO, MOCK_DEVICE_INFO}); clock.FreezeAtMilliseconds(1700000000000); auto rum = std::make_shared(RUM_CONFIG, clock); test.Start(rum); REQUIRE(test.GetContextSync().rum.has_value() == true); + const UUID initial_session_id = test.GetContextSync().rum->session_id; + REQUIRE(initial_session_id != UUID::Zero); - // When RUM is notified that the SDK is shutting down + // When the SDK is stopped and then restarted test.Stop(rum); + test.Start(rum); - // Then the RumFeatureContext is cleared, ensuring that if the SDK is restarted from - // the same instance, our original state doesn't leak in - REQUIRE(test.GetContextSync().rum.has_value() == false); + // Then the RumFeatureContext reflects a fresh new session, not the stale state + // from the previous run + CoreContext context = test.GetContextSync(); + REQUIRE(context.rum.has_value() == true); + REQUIRE(context.rum->application_id == APPLICATION_ID); + REQUIRE(context.rum->session_id != UUID::Zero); + REQUIRE(context.rum->session_id != initial_session_id); } SECTION("M change session_id W session expires and is replaced") { diff --git a/tests/support/feature.hpp b/tests/support/feature.hpp index 16c7afa2..29282def 100644 --- a/tests/support/feature.hpp +++ b/tests/support/feature.hpp @@ -47,6 +47,10 @@ class FeatureTest { explicit FeatureTest(const CoreContext& context) : _context_provider(context) {} void Start(const std::shared_ptr& feature) { + // Mirror Core::Start() by resetting feature context before each run, so tests + // observe the same clean-slate guarantee that production code provides + _context_provider.Update([](CoreContext& ctx) { ctx.Reset(); }); + auto event_writer = [this](Block event, Block event_metadata) { events.emplace_back(event, event_metadata); return true; @@ -65,5 +69,9 @@ class FeatureTest { void Stop(const std::shared_ptr& feature) { feature->OnCoreStopping(); } + void UpdateContext(const std::function& callback) { + _context_provider.Update(callback); + } + inline CoreContext GetContextSync() const { return _context_provider.Get(); } };