From 8cb8f6b6c6a6f7ebae319cbfa570c9617be88912 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 17 Sep 2026 16:54:18 +0100 Subject: [PATCH] build: drop the second telemetry default and its stale docs This branch rewrote the CMakeLists telemetry block and the build doc. Both now describe a CMake option that no longer exists: the Conan option is the switch and the generated toolchain carries it into CMake. The comment also recorded the state of the change rather than the behaviour of the code. The doc's "Building without telemetry" section told readers to pass -Dtelemetry=OFF to CMake as well, which would override the toolchain rather than follow it. --- CMakeLists.txt | 19 ++++++++----------- docs/build/telemetry.md | 17 ++++++----------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 49fc873685..1006c723c8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -141,21 +141,18 @@ if(rocksdb) endif() # OpenTelemetry distributed tracing (optional). -# When ON, links against opentelemetry-cpp and defines XRPL_ENABLE_TELEMETRY so +# When on, links against opentelemetry-cpp and defines XRPL_ENABLE_TELEMETRY so # that SpanGuard factory methods produce real OTel spans. -# When OFF, all tracing code compiles to no-ops with zero overhead and +# When off, all tracing code compiles to no-ops with zero overhead and # opentelemetry-cpp is not needed at all. # -# The value below is temporarily ON so that CI compiles the telemetry code -# paths while this feature is in review. OFF is the intended shipped default; -# flipping it back is tracked as a separate change. Do not rely on the current -# value - select it explicitly with cmake -Dtelemetry=ON|OFF or -# conan install -o telemetry=True|False. +# There is no CMake option. The one switch is `conan install -o telemetry=`, +# which decides whether opentelemetry-cpp is fetched and sets the variable read +# below through the generated toolchain. # -# -DXRPL_ENABLE_TELEMETRY=OFF does not turn anything off: that name is only a -# compile definition added below, not a CMake option, so CMake just lists it as -# an unused variable at the end of configuration. -option(telemetry "Enable OpenTelemetry tracing" ON) +# -DXRPL_ENABLE_TELEMETRY=OFF turns nothing off: that name is only a compile +# definition added below, so CMake lists it as an unused variable at the end of +# configuration. if(telemetry) find_package(opentelemetry-cpp CONFIG REQUIRED) add_compile_definitions(XRPL_ENABLE_TELEMETRY) diff --git a/docs/build/telemetry.md b/docs/build/telemetry.md index 14f7d1d5a9..134cca5f63 100644 --- a/docs/build/telemetry.md +++ b/docs/build/telemetry.md @@ -32,14 +32,10 @@ such as Grafana Tempo. Telemetry is gated twice — once at compile time and once at runtime: -- **Compile time**: The Conan option `telemetry` and CMake option `telemetry` decide - whether the OTel SDK is linked in and `XRPL_ENABLE_TELEMETRY` is defined. +- **Compile time**: The Conan option `telemetry` must be `True`. It is the only switch: there is no CMake option, because `conan install` writes the value into the generated toolchain and CMake reads it from there. When off, all `SpanGuard` calls compile to inline no-ops (defined in `SpanGuard.h`) with zero overhead — no OTel SDK dependency required. - The option is currently `True`/`ON` on the telemetry branches so that CI builds and - exercises the instrumented code; **`False`/`OFF` is the intended default once this - feature is merged.** Pass the value you want explicitly rather than relying on the - default. + Pass the value you want explicitly rather than relying on the default. - **Runtime**: Telemetry is **off by default** — the `[telemetry]` config section must set `enabled=1`. When disabled at runtime, a no-op implementation is used even in a build that has the SDK compiled in. @@ -110,10 +106,9 @@ cmake --build . --parallel $(nproc) ## Building without telemetry -Pass `-o telemetry=False` to `conan install`, and `-Dtelemetry=OFF` to CMake if you -configure without the Conan-generated toolchain. Do not just omit the option — it then -resolves to whatever the current default is, and that default is `True` on the -telemetry branches. +Pass `-o telemetry=False` to `conan install`. That is the whole switch: CMake takes the +value from the generated toolchain, so there is no CMake flag to pass as well. Do not just +omit the option — it then resolves to whatever the recipe's current default is. The `opentelemetry-cpp` dependency will not be downloaded, the `XRPL_ENABLE_TELEMETRY` preprocessor define will not be set, @@ -124,7 +119,7 @@ The resulting binary is identical to one built before telemetry support was adde > CMake option — it is only a compile definition added when `telemetry` is on. Passing it > on the command line leaves telemetry compiled in; CMake merely lists it at the end of > configuration under `Manually-specified variables were not used by the project`. -> Use `-Dtelemetry=OFF`. +> Use `-o telemetry=False` on `conan install`. ## Troubleshooting