build: default telemetry off, for the perf-iac compiled-out control run

[pratikmankawde]

PR to test telemetry-off builds on CI.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[pratikmankawde]

Severity counts: 1 BLOCKING - 1 SHOULD-FIX - 1 NIT.

Review of the telemetry-off draft at 5f9f7c2b71. Small and focused; the mechanism is right and the options: input on build-deps is a clean way to pass a Conan host option.

The blocking concern is CI coverage rather than correctness - see the comment on CMakeLists.txt. In short, telemetry-validation.yml is now the only leg that enables telemetry, and it has no pull_request trigger and a paths: filter that excludes the instrumented call sites, so after this flip no PR check compiles the #ifdef XRPL_ENABLE_TELEMETRY blocks and clang-tidy does not analyse them either.

SHOULD-FIX - Medium - a doc contradicts the code after this change (not on a diff line, so pointing at it here): docs/build/telemetry.md:39-42 still reads "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." Both halves are false on this branch once the two defaults flip. That sentence was introduced on phase-9 (3153f3ef56) and is correct on phase-9 and phase-10 - the branch that makes it wrong is this one, so the fix belongs here rather than upstream.

What I checked and found correct:

• The two defaults now agree (CMakeLists.txt:160 OFF, conanfile.py:60 False), which is the stated reason for the change and is genuinely better than the previous split: a bare cmake .. no longer enables telemetry against a dependency Conan never fetched.
• The workflow passes the option on both sides, and the comment explains that the Conan option is what fetches opentelemetry-cpp while the CMake option is what makes find_package require it. That is accurate.
• Nothing else in the tree relies on the old default. grep -rn "telemetry" .github/workflows/ finds only this workflow, reusable-check-otel-naming.yml (Python-only checks, no compile) and on-pr.yml's paths list.
• The code is all still present and check-otel-naming still validates the name layers on every PR. What is lost is the compiler and clang-tidy, which is what the inline comment is about.

[pratikmankawde]

[BLOCKING · High] — with both defaults off, no pull-request check compiles the #ifdef XRPL_ENABLE_TELEMETRY code any more.

XRPL_ENABLE_TELEMETRY is defined only inside if(telemetry) two lines below, so with the option off every guarded block is preprocessed away. Flipping this together with conanfile.py:60 means nothing in the PR pipeline builds that code:

• .github/scripts/strategy-matrix/linux.json contains no telemetry option, so every build-test leg configures with it off.
• .github/workflows/reusable-clang-tidy.yml:72 calls ./.github/actions/build-deps without the new options: input, so clang-tidy's compile database is built with telemetry off and the guarded regions are not analysed at all.
• The only leg that passes &:telemetry=True is telemetry-validation.yml, and it triggers on workflow_dispatch and push — there is no pull_request trigger.
• That workflow's paths: filter also excludes the instrumented call sites: it matches **/*SpanNames.h and the three telemetry module directories, but nothing under src/xrpld/app/**, src/xrpld/overlay/** or src/libxrpl/core/**.

Concretely: a change that adds or edits a span or metric call inside PeerImp.cpp or LedgerMaster.cpp would compile cleanly on every PR check with the guarded code never parsed, and would not trigger the one workflow that would have caught it. Before this change both defaults were on, so the whole matrix type-checked it.

The comment this replaces said exactly that — "temporarily ON so that CI compiles the telemetry code paths while this feature is in review" — and it is being removed while the feature is still in review. If the intent is only the perf-iac compiled-out control run, this branch should stay a draft and not merge; if the flip is meant to ship, it needs the telemetry option added to at least one build-test matrix leg and to the clang-tidy leg in the same change, otherwise the chain loses its compile coverage at the moment it stops being exercised.

[pratikmankawde]

[NIT · Low] — the unquoted expansion enables filename globbing as well as the word splitting the comment intends.

The comment above says word splitting is deliberate, which is right, but leaving ${EXTRA_OPTIONS} unquoted also subjects each word to pathname expansion. &:telemetry=True contains no glob characters so nothing is wrong today; a future value containing *, ? or [ would be silently rewritten against the runner's working directory, and the failure would look like a bad Conan option rather than a shell expansion.

read -ra extra <<<"$EXTRA_OPTIONS" splits on whitespace without globbing and keeps the intent explicit. A local set -f around the loop is the one-line alternative.