diff --git a/docs/README.md b/docs/README.md index f9ff5d69e..3967d2d05 100644 --- a/docs/README.md +++ b/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) diff --git a/docs/thread-safety.md b/docs/thread-safety.md new file mode 100644 index 000000000..9a3885c70 --- /dev/null +++ b/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 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. diff --git a/src/datadog/limiter.h b/src/datadog/limiter.h index ddddb5efb..b0cbb95a4 100644 --- a/src/datadog/limiter.h +++ b/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 #include diff --git a/src/datadog/random.cpp b/src/datadog/random.cpp index 78f52e65f..6d52df538 100644 --- a/src/datadog/random.cpp +++ b/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`. diff --git a/src/datadog/remote_config/remote_config.h b/src/datadog/remote_config/remote_config.h index b63a428be..380dda8dd 100644 --- a/src/datadog/remote_config/remote_config.h +++ b/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. diff --git a/src/datadog/telemetry/telemetry_impl.h b/src/datadog/telemetry/telemetry_impl.h index 59503565e..428ab60c7 100644 --- a/src/datadog/telemetry/telemetry_impl.h +++ b/src/datadog/telemetry/telemetry_impl.h @@ -21,18 +21,17 @@ namespace datadog::telemetry { using MetricSnapshot = std::vector>; -/// 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 { - /// 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 logger_; std::vector tasks_; tracing::HTTPClient::URL telemetry_endpoint_; @@ -41,23 +40,23 @@ class Telemetry final : public std::enable_shared_from_this { tracing::Clock clock_; std::shared_ptr scheduler_; - /// Counter + // Counter std::mutex counter_mutex_; std::unordered_map, uint64_t> counters_; std::unordered_map, MetricSnapshot> counters_snapshot_; - /// Rate + // Rate std::mutex rate_mutex_; std::unordered_map, uint64_t> rates_; std::unordered_map, 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, std::vector> distributions_; - /// Configuration + // Configuration std::vector configuration_snapshot_; std::mutex log_mutex_; @@ -97,15 +96,11 @@ class Telemetry final : public std::enable_shared_from_this { 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 { // 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& tags); @@ -130,12 +125,12 @@ class Telemetry final : public std::enable_shared_from_this { void set_counter(const Counter& counter, const std::vector& tags, uint64_t value); - /// Rate + // Rate void set_rate(const Rate& rate, uint64_t value); void set_rate(const Rate& rate, const std::vector& tags, uint64_t value); - /// Distribution + // Distribution void add_datapoint(const Distribution& distribution, uint64_t value); void add_datapoint(const Distribution& distribution, const std::vector& tags, uint64_t value);