Add 'Thread Safety' documentation

docs/README.md

@@ -2,6 +2,7 @@
 
 This directory contains documentation of the Datadog C++ Tracer, including:
 
-- [Design](design.md)
 - [Conventions](conventions.md)
+- [Design](design.md)
 - [Development Processes](development.md)
+- [Thread Safety](thread-safety.md)

docs/thread-safety.md

@@ -0,0 +1,136 @@
+# Datadog C++ Tracer Thread Safety
+
+## Architecture Principles
+
+- **Immutability**. The `Tracer` and the objects passed into a `TraceSegment` at construction are
+set once and never mutated afterward.
+- **Delegation**. All mutable shared state is in an explicit set of classes that each owns a
+  `std::mutex`:
+  - `CerrLogger`
+  - `ConfigManager`
+  - `Curl`
+  - `DatadogAgent`
+  - `SpanSampler::SynchronizedLimiter`
+  - `Telemetry`
+  - `ThreadedEventScheduler`
+  - `TraceSampler`
+  - `TraceSegment`
+- `Span`/`SpanData`/`Tracer` carry no internal lock. `Span` is move-only and non-reassignable. The
+  contract is: **one `Span`, one owner thread at a time**. Concurrency is meant to happen between
+  sibling `Span`s of the same `TraceSegment`.
+- **Use `std::mutex`, not `std::atomic`**:
+  - Every synchronized class uses plain `std::mutex` + `lock_guard`/`unique_lock` (no usage of
+    `std::atomic`), for simplicity.
+  - No logging while holding a process-wide singleton's lock, because of potentially slow custom
+    `Logger`.
+- **The default pluggable interfaces add extra threads.** The default `HTTPClient`, which is `Curl`,
+  and the default `EventScheduler`, which is `ThreadedEventScheduler`, add each one a dedicated
+  thread.
+- `Fork` (such as Nginx/Apache pre-fork workers). Because background threads are not automatically
+  fork-safe, the embedder must construct the `Tracer` (and therefore any default
+  `Curl`/`ThreadedEventScheduler`) strictly after `fork()`.
+- **`Collector` sharing across multiple `Tracer`s**. This is the intended way to fan many
+  threads/`Tracer`s into one flush pipeline. This is synchronized via `DatadogAgent`'s own mutex.
+- **Three process-wide singletons, exceptions to the independence of `Tracer`s**:
+  - `telemetry::instance()`: the first `Tracer` (of the process) configures the `Telemetry` used by
+    all `Tracers`.
+  - `root_session_id::get_or_init()` is meant to be caller-coordinated. Integrations (notably Nginx
+    and Apache) should set this in the master process before workers fork so all `Tracer`s share the
+    same root.
+  - `OtelCtxRegistration` is a mutex-guarded singleton publishing the OpenTelemetry process context.
+    Each `Tracer` registers/unregisters on construction/destruction. The first `Tracer`'s fields
+    win. `service_instance_id` is published only while all live `Tracer`s agree on it.
+- **Shutdown discipline**. The `DatadogAgent` destructor cancels scheduled tasks and waits for
+  outstanding HTTP requests. `ThreadedEventScheduler`'s cancellation closure blocks until any
+  in-flight callback finishes. `Curl`'s destructor joins its thread. This avoids callbacks firing
+  into a destroyed object. A custom `HTTPClient`/`EventScheduler` must replicate this or accept
+  trace loss on shutdown.
+- **AppSec/WAF-style thread-pool offload**. This Nginx feature stresses the core library's
+  unsynchronized `Span` (see details below).
+
+## Take-aways for Library Users
+
+- It is safe to share one `Tracer` across many threads. `create_span()`/`extract_span()` can be
+  called concurrently. This is the primary supported model.
+- It is safe to create/finish sibling `Span`s of the same `TraceSegment` concurrently across
+  threads.
+- It is unsafe to use a single `Span` object from two threads at once. Either transfer ownership
+  completely (moves are supported) or build your own handoff protocol (see, for example,
+  `nginx-datadog`'s WAF thread-pool integration: atomic release/acquire flag + swapping out the
+  request's event handlers so the main thread can't touch it mid-flight).
+- Never construct a `Tracer` before your process forks. Construct it after, in each child.
+- Multiple `Tracer`s in one process share one telemetry pipeline.
+- Multiple `Tracer`s in one process end up with a `root_session_id` decided by the first `Tracer`
+  constructed (later values are silently ignored). Pass the same explicit
+  `TracerConfig::root_session_id` to every `Tracer` (for example, Nginx and Apache both compute it
+  once pre-fork, then pass it to each worker's `Tracer`).
+
+- Multiple `Tracer`s in one process publish one shared OpenTelemetry process context. The first
+  `Tracer`'s fields win. The shared `runtime_id` is published only while every `Tracer` agrees on
+  it.
+- If you supply your own `Clock`/`Collector`/`EventScheduler`/`HTTPClient`/`IDGenerator`/`Logger`,
+  you must ensure it is safe to be called from multiple threads because the library will call it
+  from whatever threads its other pluggable pieces run on.
+
+## Web Reverse Proxies Integrations
+
+The Datadog C++ Tracer was notably designed with the following three integrations in mind. Thus,
+they serve both as examples of different threading models and as projects for validating changes to
+the Tracer.
+
+### Datadog Nginx Module
+
+See [nginx-datadog](https://github.com/DataDog/nginx-datadog).
+
+Process / Thread Model:
+
+- Nginx has a **master process** which forks into several **worker processes**.
+- Each worker process handles many connections at once in a **single thread** (with one exception:
+  the optional security/WAF analysis runs in a side thread pool).
+- Each worker process creates its `Tracer`.
+
+It has a custom `NgxEventScheduler`, which runs on the worker's own event loop.
+
+It has a custom logger locking.
+
+The `root_session_id` is set explicitly, generated once pre-fork.
+
+The WAF thread pool mutates a `Span` from a non-owning thread (in `Context::run_waf_start()` ,
+`Context::run_waf_req_post()` and `Context::do_on_main_log_request()`). It is safe by an ad hoc
+protocol: handler swap (`Context::replace_handlers()`), and `std::atomic<bool> ran_on_thread_`
+release (`Context::handle()`) / acquire (`Context::complete()`).
+
+### Datadog Apache Httpd Module
+
+See [httpd-datadog](https://github.com/DataDog/httpd-datadog).
+
+Process / Thread Model:
+
+- Apache has a **master process** which forks into several **child processes**.
+- Depending on the configuration, the child processes can be single-threaded or **multi-threaded**.
+- Each child process creates its `Tracer`, shared by every thread in it.
+
+The `root_session_id` and `runtime_id` are set explicitly pre-fork.
+
+It has a custom logger locking.
+
+The `Span`s are allocated on the heap and tied to the request's Apache Pre-Request (APR) memory pool
+(and so automatically deleted when the request finishes).
+
+### Datadog Envoy Extension
+
+See
+[envoyproxy/envoy/source/extensions/tracers/datadog](https://github.com/envoyproxy/envoy/tree/main/source/extensions/tracers/datadog).
+
+Process / Thread Model:
+
+- Envoy has a **single process**, with several **worker threads**.
+- Each worker thread creates its `Tracer`.
+
+It uses a custom `AgentHTTPClient` and a custom `EventScheduler`. They are bound to the owning
+`Dispatcher`, with no extra thread.
+
+It uses Envoy’s own logging.
+
+Envoy is the only integration where `OtelCtxRegistration`'s multi-Tracer bookkeeping is actually
+exercised.

src/datadog/limiter.h

@@ -1,12 +1,10 @@
 #pragma once
 
-// This component provides a `class`, `Limiter`, that is an implementation of
-// the [token bucket][1] rate limiter.
+// The `Limiter` class is an implementation of the [token
+// bucket](https://en.wikipedia.org/wiki/Token_bucket) rate limiter.
 //
 // `Limiter` is used by the `TraceSampler` and the `SpanSampler` to enforce
 // their respective `max_per_second` configuration parameters.
-//
-// [1]: https://en.wikipedia.org/wiki/Token_bucket
 
 #include <datadog/clock.h>
 #include <datadog/rate.h>

src/datadog/random.cpp

@@ -22,7 +22,7 @@ class Uint64Generator {
     // If a process links to this library and then calls `fork`, the
     // `generator_` in the parent and child processes will produce the exact
     // same sequence of values, which is bad.
-    // A subsequent call to `exec` would remedy this, but nginx in particular
+    // A subsequent call to `exec` would remedy this, but Nginx in particular
     // does not call `exec` after forking its worker processes.
     // So, we use `at_fork_in_child` to re-seed `generator_` in the child
     // process after `fork`.

src/datadog/remote_config/remote_config.h

@@ -1,11 +1,11 @@
 #pragma once
 
 // Remote Configuration is a Datadog capability that allows a user to remotely
-// configure and change the behaviour of the tracing library.
+// configure and change the behavior of the tracing library.
 // The current implementation is restricted to Application Performance
 // Monitoring features.
 //
-// The `RemoteConfigurationManager` class implement the protocol to query,
+// The `RemoteConfigurationManager` class implements the protocol to query,
 // process and verify configuration from a remote source. It is also
 // responsible for handling configuration updates received from a remote source
 // and maintains the state of applied configuration.

src/datadog/telemetry/telemetry_impl.h

@@ -21,18 +21,17 @@ namespace datadog::telemetry {
 
 using MetricSnapshot = std::vector<std::pair<std::time_t, uint64_t>>;
 
-/// The telemetry class is responsible for handling internal telemetry data to
-/// track Datadog product usage. It _can_ collect and report logs and metrics.
-///
-/// NOTE(@dmehala): The current implementation can lead a significant amount
-/// of overhead if the mutext is highly disputed. Unless this is proven to be
-/// indeed a bottleneck, I'll embrace KISS principle. However, in a future
-/// iteration we could use multiple producer single consumer queue or
-/// lock-free queue.
+// The `Telemetry` class is responsible for handling internal telemetry data to
+// track Datadog product usage. It _can_ collect and report logs and metrics.
+//
+// The current implementation can lead a significant amount of overhead if the
+// mutex is highly disputed. Unless this is proven to be indeed a bottleneck, we
+// embrace KISS principle. However, in a future iteration we could use multiple
+// producers - single consumer queue or lock-free queue.
 class Telemetry final : public std::enable_shared_from_this<Telemetry> {
-  /// Configuration object containing the validated settings for telemetry
+  // Configuration object containing the validated settings for telemetry
   FinalizedConfiguration config_;
-  /// Shared pointer to the user logger instance.
+  // Shared pointer to the user logger instance
   std::shared_ptr<tracing::Logger> logger_;
   std::vector<tracing::EventScheduler::Cancel> tasks_;
   tracing::HTTPClient::URL telemetry_endpoint_;
@@ -41,23 +40,23 @@ class Telemetry final : public std::enable_shared_from_this<Telemetry> {
   tracing::Clock clock_;
   std::shared_ptr<tracing::EventScheduler> scheduler_;
 
-  /// Counter
+  // Counter
   std::mutex counter_mutex_;
   std::unordered_map<MetricContext<Counter>, uint64_t> counters_;
   std::unordered_map<MetricContext<Counter>, MetricSnapshot> counters_snapshot_;
 
-  /// Rate
+  // Rate
   std::mutex rate_mutex_;
   std::unordered_map<MetricContext<Rate>, uint64_t> rates_;
   std::unordered_map<MetricContext<Rate>, MetricSnapshot> rates_snapshot_;
 
-  /// Distribution
-  /// TODO: split distribution in array of N element?
+  // Distribution
+  // TODO: split distribution in array of N element?
   std::mutex distributions_mutex_;
   std::unordered_map<MetricContext<Distribution>, std::vector<uint64_t>>
       distributions_;
 
-  /// Configuration
+  // Configuration
   std::vector<tracing::ConfigMetadata> configuration_snapshot_;
 
   std::mutex log_mutex_;
@@ -97,15 +96,11 @@ class Telemetry final : public std::enable_shared_from_this<Telemetry> {
             tracing::Clock clock = tracing::default_clock);
 
  public:
-  /// Capture and report internal error message to Datadog.
-  ///
-  /// @param message The error message.
+  // Capture and report internal error message to Datadog.
   void log_error(std::string message);
   void log_error(std::string message, std::string stacktrace);
 
-  /// capture and report internal warning message to Datadog.
-  ///
-  /// @param message The warning message to log.
+  // Capture and report internal warning message to Datadog.
   void log_warning(std::string message);
 
   void send_configuration_change();
@@ -119,7 +114,7 @@ class Telemetry final : public std::enable_shared_from_this<Telemetry> {
   // After this call the Telemetry object is inert and safe to destroy.
   void shutdown();
 
-  /// Counter
+  // Counter
   void increment_counter(const Counter& counter);
   void increment_counter(const Counter& counter,
                          const std::vector<std::string>& tags);
@@ -130,12 +125,12 @@ class Telemetry final : public std::enable_shared_from_this<Telemetry> {
   void set_counter(const Counter& counter, const std::vector<std::string>& tags,
                    uint64_t value);
 
-  /// Rate
+  // Rate
   void set_rate(const Rate& rate, uint64_t value);
   void set_rate(const Rate& rate, const std::vector<std::string>& tags,
                 uint64_t value);
 
-  /// Distribution
+  // Distribution
   void add_datapoint(const Distribution& distribution, uint64_t value);
   void add_datapoint(const Distribution& distribution,
                      const std::vector<std::string>& tags, uint64_t value);