Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation

src/libxrpl/consensus/Consensus.cpp

@@ -13,8 +13,8 @@
 
 namespace xrpl {
 
-bool
-shouldCloseLedger(
+LedgerCloseReason
+whyCloseLedger(
     bool anyTransactions,
     std::size_t prevProposers,
     std::size_t proposersClosed,
@@ -47,7 +47,7 @@ shouldCloseLedger(
 
         JLOG(j.warn()) << ss.str();
         CLOG(clog) << "closing ledger: " << ss.str() << ". ";
-        return true;
+        return LedgerCloseReason::Anomaly;
     }
 
     if ((proposersClosed + proposersValidated) > (prevProposers / 2))
@@ -55,14 +55,16 @@ shouldCloseLedger(
         // If more than half of the network has closed, we close
         JLOG(j.trace()) << "Others have closed";
         CLOG(clog) << "closing ledger because enough others have already. ";
-        return true;
+        return LedgerCloseReason::OthersClosed;
     }
 
     if (!anyTransactions)
     {
         // Only close at the end of the idle interval
         CLOG(clog) << "no transactions, returning. ";
-        return timeSincePrevClose >= idleInterval;  // normal idle
+        return timeSincePrevClose >= idleInterval  // normal idle
+            ? LedgerCloseReason::Idle
+            : LedgerCloseReason::KeepOpen;
     }
 
     // Preserve minimum ledger open time
@@ -70,7 +72,7 @@ shouldCloseLedger(
     {
         JLOG(j.debug()) << "Must wait minimum time before closing";
         CLOG(clog) << "not closing because under ledgerMIN_CLOSE. ";
-        return false;
+        return LedgerCloseReason::KeepOpen;
     }
 
     // Don't let this ledger close more than twice as fast as the previous
@@ -80,12 +82,40 @@ shouldCloseLedger(
     {
         JLOG(j.debug()) << "Ledger has not been open long enough";
         CLOG(clog) << "not closing because not open long enough. ";
-        return false;
+        return LedgerCloseReason::KeepOpen;
     }
 
     // Close the ledger
     CLOG(clog) << "no reason to not close. ";
-    return true;
+    return LedgerCloseReason::Normal;
+}
+
+bool
+shouldCloseLedger(
+    bool anyTransactions,
+    std::size_t prevProposers,
+    std::size_t proposersClosed,
+    std::size_t proposersValidated,
+    std::chrono::milliseconds prevRoundTime,
+    std::chrono::milliseconds timeSincePrevClose,
+    std::chrono::milliseconds openTime,
+    std::chrono::milliseconds idleInterval,
+    ConsensusParms const& parms,
+    beast::Journal j,
+    std::unique_ptr<std::stringstream> const& clog)
+{
+    return whyCloseLedger(
+               anyTransactions,
+               prevProposers,
+               proposersClosed,
+               proposersValidated,
+               prevRoundTime,
+               timeSincePrevClose,
+               openTime,
+               idleInterval,
+               parms,
+               j,
+               clog) != LedgerCloseReason::KeepOpen;
 }
 
 bool

src/libxrpl/telemetry/TelemetryConfig.cpp

@@ -8,6 +8,7 @@
  * See cfg/xrpld-example.cfg for the full list of available options.
  */
 
+#include <xrpl/basics/FileUtilities.h>
 #include <xrpl/basics/contract.h>
 #include <xrpl/config/BasicConfig.h>
 #include <xrpl/telemetry/Telemetry.h>
@@ -16,6 +17,7 @@
 #include <cstdint>
 #include <stdexcept>
 #include <string>
+#include <system_error>
 
 namespace xrpl::telemetry {
 
@@ -64,6 +66,35 @@ constexpr std::uint32_t batchDelayMs = 5000u;
 constexpr std::uint32_t maxQueueSize = 2048u;
 }  // namespace dflt
 
+/**
+ * Throw unless the given path names a file this process can read.
+ *
+ * An empty path means the option is unset, which every caller allows. Reading
+ * the file proves it is both present and readable; testing existence alone
+ * would miss a permissions problem. The contents are discarded — nothing here
+ * checks that they parse as PEM.
+ *
+ * @param path       Path taken from the config, possibly empty.
+ * @param configKey  Config key the path came from, named in the message. Not
+ * called `key`, which would hide the `key` namespace above.
+ * @throws std::runtime_error  If the path is non-empty and cannot be read.
+ */
+void
+requireReadableFile(std::string const& path, char const* configKey)
+{
+    if (path.empty())
+        return;
+
+    std::error_code ec;
+    getFileContents(ec, path);
+    if (ec)
+    {
+        Throw<std::runtime_error>(
+            std::string{"[telemetry] "} + configKey + " cannot be read: " + path + " - " +
+            ec.message());
+    }
+}
+
 }  // namespace
 
 /**
@@ -143,6 +174,18 @@ makeTelemetrySetup(
                 "[telemetry] tls_client_cert/tls_client_key require use_tls=1 "
                 "(set use_tls=1 to enable mutual TLS, or remove the cert paths).");
         }
+
+        // Still inside the enabled branch. The exporter opens these files only
+        // when TLS is on, so check them only then: a bad path behind use_tls=0
+        // stops nothing. Checking here turns what would otherwise surface much
+        // later as an opaque handshake failure into a startup error naming the
+        // key. Each path is optional; an empty one is skipped.
+        if (setup.useTls)
+        {
+            requireReadableFile(setup.tlsCertPath, key::tlsCaCert);
+            requireReadableFile(setup.tlsClientCertPath, key::tlsClientCert);
+            requireReadableFile(setup.tlsClientKeyPath, key::tlsClientKey);
+        }
     }
 
     // Head sampling is intentionally fixed at 1.0 (sample everything) and is

src/tests/libxrpl/consensus/Consensus.cpp

@@ -66,6 +66,33 @@ shouldCloseLedger(
         clog);
 }
 
+LedgerCloseReason
+whyCloseLedger(
+    bool anyTransactions,
+    std::size_t prevProposers,
+    std::size_t proposersClosed,
+    std::size_t proposersValidated,
+    std::chrono::milliseconds prevRoundTime,
+    std::chrono::milliseconds timeSincePrevClose,
+    std::chrono::milliseconds openTime,
+    std::chrono::milliseconds idleInterval,
+    ConsensusParms const& parms,
+    std::unique_ptr<std::stringstream> const& clog = {})
+{
+    return xrpl::whyCloseLedger(
+        anyTransactions,
+        prevProposers,
+        proposersClosed,
+        proposersValidated,
+        prevRoundTime,
+        timeSincePrevClose,
+        openTime,
+        idleInterval,
+        parms,
+        journal(),
+        clog);
+}
+
 ConsensusState
 checkConsensus(
     std::size_t prevProposers,
@@ -219,6 +246,82 @@ TEST(ConsensusTest, should_close_ledger)
     EXPECT_TRUE(shouldCloseLedger(true, 10, 0, 0, 10s, 10s, 10s, 10s, p));
 }
 
+TEST(ConsensusTest, why_close_ledger_reports_the_deciding_branch)
+{
+    using namespace std::chrono_literals;
+    SCOPED_TRACE("why close ledger");
+
+    // Same input vectors as should_close_ledger above, pinned to the reason
+    // rather than the bool, so a branch that starts returning the wrong
+    // reason is caught even though the close/no-close verdict is unchanged.
+    ConsensusParms const p{};
+
+    // Bizarre times forcibly close. These vectors ALSO satisfy the
+    // others-closed condition (10+10 > 10/2), so they pin the precedence: the
+    // anomaly check runs first.
+    EXPECT_EQ(whyCloseLedger(true, 10, 10, 10, -10s, 10s, 1s, 1s, p), LedgerCloseReason::Anomaly);
+    EXPECT_EQ(whyCloseLedger(true, 10, 10, 10, 100h, 10s, 1s, 1s, p), LedgerCloseReason::Anomaly);
+    EXPECT_EQ(whyCloseLedger(true, 10, 10, 10, 10s, 100h, 1s, 1s, p), LedgerCloseReason::Anomaly);
+
+    // Rest of network has closed: 3 closed + 5 validated > 10/2.
+    EXPECT_EQ(
+        whyCloseLedger(true, 10, 3, 5, 10s, 10s, 10s, 10s, p), LedgerCloseReason::OthersClosed);
+
+    // No transactions: keep open until the idle interval elapses, then close
+    // as idle rather than as a normal close.
+    EXPECT_EQ(whyCloseLedger(false, 10, 0, 0, 1s, 1s, 1s, 10s, p), LedgerCloseReason::KeepOpen);
+    EXPECT_EQ(whyCloseLedger(false, 10, 0, 0, 1s, 10s, 1s, 10s, p), LedgerCloseReason::Idle);
+
+    // Under ledgerMinClose (2s). prevRoundTime is 2s so prevRoundTime/2 is 1s
+    // and openTime is NOT under it -- this vector isolates the min-close
+    // branch, which the 10s variant does not (there openTime < 5s trips the
+    // too-fast branch as well, so deleting min-close entirely still passes).
+    EXPECT_EQ(whyCloseLedger(true, 10, 0, 0, 2s, 10s, 1s, 10s, p), LedgerCloseReason::KeepOpen);
+
+    // Past ledgerMinClose but under prevRoundTime/2 (5s), so still too fast.
+    EXPECT_EQ(whyCloseLedger(true, 10, 0, 0, 10s, 10s, 3s, 10s, p), LedgerCloseReason::KeepOpen);
+
+    // Both minimum-open constraints satisfied.
+    EXPECT_EQ(whyCloseLedger(true, 10, 0, 0, 10s, 10s, 10s, 10s, p), LedgerCloseReason::Normal);
+}
+
+TEST(ConsensusTest, why_close_ledger_others_closed_boundary_is_exclusive)
+{
+    using namespace std::chrono_literals;
+    SCOPED_TRACE("others-closed boundary");
+
+    // The branch is `(closed + validated) > prevProposers / 2`, strict. With
+    // prevProposers 10 the threshold is 5, so 5 must NOT close and 6 must.
+    // Flipping > to >= would otherwise go unnoticed.
+    ConsensusParms const p{};
+
+    EXPECT_EQ(whyCloseLedger(true, 10, 3, 2, 10s, 10s, 10s, 10s, p), LedgerCloseReason::Normal);
+    EXPECT_EQ(
+        whyCloseLedger(true, 10, 3, 3, 10s, 10s, 10s, 10s, p), LedgerCloseReason::OthersClosed);
+
+    // Integer truncation: 11/2 is 5, so 5 still does not close.
+    EXPECT_EQ(whyCloseLedger(true, 11, 3, 2, 10s, 10s, 10s, 10s, p), LedgerCloseReason::Normal);
+
+    // Others-closed outranks both the no-transactions and the minimum-open
+    // branches, which would otherwise return KeepOpen for these inputs.
+    EXPECT_EQ(whyCloseLedger(false, 10, 3, 5, 1s, 1s, 1s, 10s, p), LedgerCloseReason::OthersClosed);
+}
+
+TEST(ConsensusTest, why_close_ledger_idle_boundary_is_inclusive)
+{
+    using namespace std::chrono_literals;
+    SCOPED_TRACE("idle boundary");
+
+    // The idle path closes on `timeSincePrevClose >= idleInterval`. One
+    // millisecond either side of the boundary, to pin the comparison as
+    // inclusive rather than strict.
+    ConsensusParms const p{};
+
+    EXPECT_EQ(whyCloseLedger(false, 10, 0, 0, 1s, 9999ms, 1s, 10s, p), LedgerCloseReason::KeepOpen);
+    EXPECT_EQ(whyCloseLedger(false, 10, 0, 0, 1s, 10s, 1s, 10s, p), LedgerCloseReason::Idle);
+    EXPECT_EQ(whyCloseLedger(false, 10, 0, 0, 1s, 10001ms, 1s, 10s, p), LedgerCloseReason::Idle);
+}
+
 TEST(ConsensusTest, check_consensus)
 {
     using namespace std::chrono_literals;

src/tests/libxrpl/telemetry/ConsensusSpanNames.cpp

@@ -0,0 +1,124 @@
+#include <xrpl/consensus/ConsensusSpanNames.h>
+
+#include <xrpl/consensus/ConsensusParms.h>
+
+#include <gtest/gtest.h>
+
+#include <string_view>
+
+/**
+ * Contract tests for the consensus phase-span attribute constants.
+ *
+ *  The keys in ConsensusSpanNames.h are the single source of truth (L1) that
+ *  `.github/scripts/otel-naming/check_otel_naming.py` derives its valid key
+ *  set from, and that the collector's spanmetrics dimensions, the Tempo span
+ *  filters and the Grafana dashboards query by literal string. A silent rename
+ *  here compiles cleanly but blanks panels, so these tests pin the wire values.
+ *  They need no telemetry runtime and run in every build.
+ *
+ *  Scope: the attributes carried by `consensus.phase.open` and
+ *  `consensus.establish`. The round-level attrs are covered by the
+ *  pre-existing key set and are deliberately NOT duplicated onto the phase
+ *  children (a child span does not inherit parent attributes, but copying
+ *  `ledger_seq` down would store the same value twice per trace).
+ */
+
+using namespace xrpl::telemetry::consensus::span;
+
+TEST(ConsensusSpanNames, phase_open_start_attribute_keys)
+{
+    // Set once when the open-phase span is created in startRoundInternal().
+    EXPECT_EQ(std::string_view(attr::startReason), "start_reason");
+    EXPECT_EQ(std::string_view(attr::previousCloseAgree), "previous_close_agree");
+    EXPECT_EQ(std::string_view(attr::peerPositionsAtOpen), "peer_positions_at_open");
+    EXPECT_EQ(std::string_view(attr::earlyCloseTriggered), "early_close_triggered");
+}
+
+TEST(ConsensusSpanNames, phase_open_end_attribute_keys)
+{
+    // Existing end-of-phase metadata, pinned alongside the new key so a rename
+    // of either shows up here.
+    EXPECT_EQ(std::string_view(attr::openDurationMs), "open_duration_ms");
+    EXPECT_EQ(std::string_view(attr::peerPositionsAtClose), "peer_positions_at_close");
+    EXPECT_EQ(std::string_view(attr::txSetsAcquired), "tx_sets_acquired");
+    EXPECT_EQ(std::string_view(attr::closeReason), "close_reason");
+    EXPECT_EQ(std::string_view(attr::proposersValidated), "proposers_validated");
+}
+
+TEST(ConsensusSpanNames, close_reason_values_are_the_close_paths)
+{
+    // One per branch of whyCloseLedger() that closes the ledger. keep_open is
+    // never emitted (the attribute is only set on the closing path) but is
+    // labelled rather than left blank so the mapping is total.
+    EXPECT_EQ(std::string_view(val::closeKeepOpen), "keep_open");
+    EXPECT_EQ(std::string_view(val::closeAnomaly), "anomaly");
+    EXPECT_EQ(std::string_view(val::closeOthersClosed), "others_closed");
+    EXPECT_EQ(std::string_view(val::closeIdle), "idle");
+    EXPECT_EQ(std::string_view(val::closeNormal), "normal");
+}
+
+TEST(ConsensusSpanNames, close_reason_label_maps_every_enum_state)
+{
+    // A missed branch would attribute a close to the wrong cause, which is the
+    // whole point of the attribute, so every enumerator is asserted.
+    EXPECT_EQ(closeReasonLabel(xrpl::LedgerCloseReason::KeepOpen), "keep_open");
+    EXPECT_EQ(closeReasonLabel(xrpl::LedgerCloseReason::Anomaly), "anomaly");
+    EXPECT_EQ(closeReasonLabel(xrpl::LedgerCloseReason::OthersClosed), "others_closed");
+    EXPECT_EQ(closeReasonLabel(xrpl::LedgerCloseReason::Idle), "idle");
+    EXPECT_EQ(closeReasonLabel(xrpl::LedgerCloseReason::Normal), "normal");
+}
+
+TEST(ConsensusSpanNames, close_reason_label_is_usable_at_compile_time)
+{
+    static_assert(
+        closeReasonLabel(xrpl::LedgerCloseReason::Idle) == "idle",
+        "closeReasonLabel must be constexpr-evaluable");
+    SUCCEED();
+}
+
+TEST(ConsensusSpanNames, establish_attribute_keys)
+{
+    // Qualified: DisputedTx tracks a second, per-transaction avalanche.
+    EXPECT_EQ(std::string_view(attr::closeTimeAvalancheState), "close_time_avalanche_state");
+}
+
+TEST(ConsensusSpanNames, start_reason_values_are_the_two_entry_paths)
+{
+    // startRoundInternal() is entered fresh, or re-entered by handleWrongLedger
+    // after acquiring the correct prior ledger. A round that recovers emits a
+    // SECOND consensus.phase.open span, so the label is what tells them apart.
+    EXPECT_EQ(std::string_view(val::startInitial), "initial");
+    EXPECT_EQ(std::string_view(val::startRecovered), "recovered");
+}
+
+TEST(ConsensusSpanNames, avalanche_state_values_match_the_parms_enum)
+{
+    EXPECT_EQ(std::string_view(val::avalancheInit), "init");
+    EXPECT_EQ(std::string_view(val::avalancheMid), "mid");
+    EXPECT_EQ(std::string_view(val::avalancheLate), "late");
+    EXPECT_EQ(std::string_view(val::avalancheStuck), "stuck");
+    EXPECT_EQ(std::string_view(val::unknown), "unknown");
+}
+
+TEST(ConsensusSpanNames, avalanche_state_label_maps_every_enum_state)
+{
+    // A missed branch here would silently report the wrong convergence regime
+    // for the round, so every enumerator is asserted explicitly rather than
+    // round-tripped through a table.
+    using AvalancheState = xrpl::ConsensusParms::AvalancheState;
+
+    EXPECT_EQ(avalancheStateLabel(AvalancheState::Init), "init");
+    EXPECT_EQ(avalancheStateLabel(AvalancheState::Mid), "mid");
+    EXPECT_EQ(avalancheStateLabel(AvalancheState::Late), "late");
+    EXPECT_EQ(avalancheStateLabel(AvalancheState::Stuck), "stuck");
+}
+
+TEST(ConsensusSpanNames, avalanche_state_label_is_usable_at_compile_time)
+{
+    // The mapping is consteval-safe so the label costs nothing at the call
+    // site in endEstablishTracing().
+    static_assert(
+        avalancheStateLabel(xrpl::ConsensusParms::AvalancheState::Stuck) == "stuck",
+        "avalancheStateLabel must be constexpr-evaluable");
+    SUCCEED();
+}

src/tests/libxrpl/telemetry/TelemetryConfig.cpp

@@ -1,3 +1,4 @@
+#include <xrpl/basics/FileUtilities.h>
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/config/BasicConfig.h>
 #include <xrpl/telemetry/Telemetry.h>
@@ -5,10 +6,13 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <fstream>
 #include <stdexcept>
+#include <string>
 
 using namespace xrpl;
 
+using ::testing::AllOf;
 using ::testing::HasSubstr;
 using ::testing::ThrowsMessage;
 
@@ -25,15 +29,18 @@ namespace {
  * or an unexpected throw. Tests that never set the key are unaffected. One
  * source of truth still keeps the two files from drifting apart.
  *
- * clientCert and clientKey are the paths written to those keys. They are
+ * clientCert and clientKey are the paths written to those keys. They name files
+ * that do not exist, so they suit only the cases the readability check cannot
+ * reach: telemetry off, or use_tls off. A case with enabled=1 and use_tls=1
+ * must write real files with writeCertFile() below instead. They are
  * declared as `char const*` so they pass to Section::set() (which takes
  * `std::string const&`) and compare against the parsed std::string members
  * without an explicit conversion, exactly as a literal would.
  *
- * pairingError and useTlsError are message fragments. Both guards throw
- * std::runtime_error, so the exception type alone cannot tell them apart.
- * Each fragment occurs in exactly one of the two messages, so matching it
- * proves which guard fired.
+ * pairingError, useTlsError and readError are message fragments. All three
+ * guards throw std::runtime_error, so the exception type alone cannot tell
+ * them apart. Each fragment occurs in exactly one of the three messages, so
+ * matching it proves which guard fired.
  */
 namespace mtls {
 constexpr char const* keyClientCert = "tls_client_cert";
@@ -42,6 +49,7 @@ constexpr char const* clientCert = "/etc/ssl/client.pem";
 constexpr char const* clientKey = "/etc/ssl/client.key";
 constexpr char const* pairingError = "must be set together";
 constexpr char const* useTlsError = "require use_tls=1";
+constexpr char const* readError = "cannot be read";
 
 /**
  * Build a [telemetry] section carrying only the `enabled` key.
@@ -75,6 +83,27 @@ parseSection(Section const& section)
 {
     return telemetry::makeTelemetrySetup(section, "nHUtest123", "2.0.0", 0);
 }
+
+/**
+ * Write a placeholder certificate file at the given path.
+ *
+ * The parser only needs the file to exist and be readable, so the contents are
+ * irrelevant — nothing checks that they parse as PEM. The stream state is
+ * asserted, so a failed write shows up as a setup failure here rather than as a
+ * confusing failure in the case under test.
+ *
+ * @param path  Where to write the file, typically from TempDir::file().
+ * @return The same path, ready to pass to Section::set().
+ */
+std::string
+writeCertFile(std::string const& path)
+{
+    std::ofstream out{path};
+    out << "placeholder\n";
+    out.close();
+    EXPECT_TRUE(out.good()) << "could not create " << path;
+    return path;
+}
 }  // namespace mtls
 
 }  // namespace
@@ -121,6 +150,10 @@ TEST(TelemetryConfig, parse_empty_section)
 
 TEST(TelemetryConfig, parse_full_section)
 {
+    // The CA path has to name a real file: with enabled=1 and use_tls=1 the
+    // parser opens it, so a placeholder path would make this case throw.
+    TempDir const dir;
+    auto const caCert = mtls::writeCertFile(dir.file("ca.pem"));
     Section section;
     section.set("enabled", "1");
     section.set("service_name", "my-rippled");
@@ -128,7 +161,7 @@ TEST(TelemetryConfig, parse_full_section)
     section.set("exporter", "otlp_http");
     section.set("endpoint", "http://collector:4318/v1/traces");
     section.set("use_tls", "1");
-    section.set("tls_ca_cert", "/etc/ssl/ca.pem");
+    section.set("tls_ca_cert", caCert);
     section.set("batch_size", "256");
     section.set("batch_delay_ms", "3000");
     section.set("max_queue_size", "4096");
@@ -145,7 +178,7 @@ TEST(TelemetryConfig, parse_full_section)
     EXPECT_EQ(setup.serviceInstanceId, "custom-id");
     EXPECT_EQ(setup.exporterEndpoint, "http://collector:4318/v1/traces");
     EXPECT_TRUE(setup.useTls);
-    EXPECT_EQ(setup.tlsCertPath, "/etc/ssl/ca.pem");
+    EXPECT_EQ(setup.tlsCertPath, caCert);
     EXPECT_EQ(setup.batchSize, 256u);
     EXPECT_EQ(setup.batchDelay, std::chrono::milliseconds{3000});
     EXPECT_EQ(setup.maxQueueSize, 4096u);
@@ -158,17 +191,24 @@ TEST(TelemetryConfig, parse_full_section)
 
 TEST(TelemetryConfig, mtls_cert_and_key_both_set)
 {
-    // Telemetry on and use_tls=1, so both guards run and neither may fire.
+    // Telemetry on and use_tls=1, so all three checks run and none may fire.
+    // Both paths have to name real files, because the parser opens them here.
+    // No CA bundle is set, which is the case this covers: mTLS against a
+    // collector whose certificate the system CA store already vouches for.
+    TempDir const dir;
+    auto const cert = mtls::writeCertFile(dir.file("client.pem"));
+    auto const key = mtls::writeCertFile(dir.file("client.key"));
     Section section = mtls::makeSection(true);
     section.set("use_tls", "1");
-    section.set(mtls::keyClientCert, mtls::clientCert);
-    section.set(mtls::keyClientKey, mtls::clientKey);
+    section.set(mtls::keyClientCert, cert);
+    section.set(mtls::keyClientKey, key);
 
     auto const setup = mtls::parseSection(section);
     EXPECT_TRUE(setup.enabled);
     EXPECT_TRUE(setup.useTls);
-    EXPECT_EQ(setup.tlsClientCertPath, mtls::clientCert);
-    EXPECT_EQ(setup.tlsClientKeyPath, mtls::clientKey);
+    EXPECT_TRUE(setup.tlsCertPath.empty());
+    EXPECT_EQ(setup.tlsClientCertPath, cert);
+    EXPECT_EQ(setup.tlsClientKeyPath, key);
 }
 
 TEST(TelemetryConfig, mtls_cert_without_key_throws)
@@ -252,20 +292,141 @@ TEST(TelemetryConfig, mtls_default_no_client_tls_is_accepted)
 
 TEST(TelemetryConfig, mtls_neither_set_is_one_way_tls)
 {
-    // Telemetry is on so the guards run, and this config must pass both:
-    // one-way TLS with a CA bundle and no client certificate.
+    // Telemetry is on so the checks run, and this config must pass all of
+    // them: one-way TLS with a CA bundle and no client certificate. The CA
+    // path has to name a real file, because the parser opens it here.
+    TempDir const dir;
+    auto const caCert = mtls::writeCertFile(dir.file("ca.pem"));
     Section section = mtls::makeSection(true);
     section.set("use_tls", "1");
-    section.set("tls_ca_cert", "/etc/ssl/ca.pem");
+    section.set("tls_ca_cert", caCert);
 
     auto const setup = mtls::parseSection(section);
     EXPECT_TRUE(setup.enabled);
     EXPECT_TRUE(setup.useTls);
-    EXPECT_EQ(setup.tlsCertPath, "/etc/ssl/ca.pem");
+    EXPECT_EQ(setup.tlsCertPath, caCert);
     EXPECT_TRUE(setup.tlsClientCertPath.empty());
     EXPECT_TRUE(setup.tlsClientKeyPath.empty());
 }
 
+TEST(TelemetryConfig, tls_missing_client_cert_file_throws)
+{
+    // Both client paths are set and use_tls=1, so neither contradiction guard
+    // can fire and the readability check is the only reachable throw. Only the
+    // certificate is absent, so the message must name that key and that path.
+    //
+    // This case and the two below use an absent file. A file that exists but
+    // denies read permission is deliberately not covered: a test process
+    // running as root reads it anyway, so the case would not be reliable.
+    TempDir const dir;
+    auto const absentCert = dir.file("absent.pem");
+    Section section = mtls::makeSection(true);
+    section.set("use_tls", "1");
+    section.set(mtls::keyClientCert, absentCert);
+    section.set(mtls::keyClientKey, mtls::writeCertFile(dir.file("k.pem")));
+
+    EXPECT_THAT(
+        [&section] { mtls::parseSection(section); },
+        ThrowsMessage<std::runtime_error>(AllOf(
+            HasSubstr(mtls::readError), HasSubstr(mtls::keyClientCert), HasSubstr(absentCert))));
+}
+
+TEST(TelemetryConfig, tls_missing_client_key_file_throws)
+{
+    // The mirror image of the case above: the certificate is readable and only
+    // the private key is absent, so the key's name must appear instead.
+    TempDir const dir;
+    auto const absentKey = dir.file("absent.key");
+    Section section = mtls::makeSection(true);
+    section.set("use_tls", "1");
+    section.set(mtls::keyClientCert, mtls::writeCertFile(dir.file("c.pem")));
+    section.set(mtls::keyClientKey, absentKey);
+
+    EXPECT_THAT(
+        [&section] { mtls::parseSection(section); },
+        ThrowsMessage<std::runtime_error>(AllOf(
+            HasSubstr(mtls::readError), HasSubstr(mtls::keyClientKey), HasSubstr(absentKey))));
+}
+
+TEST(TelemetryConfig, tls_missing_ca_cert_file_throws)
+{
+    // One-way TLS with no client certificate, so the CA bundle is the only
+    // path checked.
+    TempDir const dir;
+    auto const absentCa = dir.file("absent-ca.pem");
+    Section section = mtls::makeSection(true);
+    section.set("use_tls", "1");
+    section.set("tls_ca_cert", absentCa);
+
+    EXPECT_THAT(
+        [&section] { mtls::parseSection(section); },
+        ThrowsMessage<std::runtime_error>(
+            AllOf(HasSubstr(mtls::readError), HasSubstr("tls_ca_cert"), HasSubstr(absentCa))));
+}
+
+TEST(TelemetryConfig, tls_readable_files_are_accepted)
+{
+    // Full mTLS with all three files present and readable: parsing must
+    // succeed and keep every path verbatim.
+    TempDir const dir;
+    auto const ca = mtls::writeCertFile(dir.file("ca.pem"));
+    auto const cert = mtls::writeCertFile(dir.file("c.pem"));
+    auto const key = mtls::writeCertFile(dir.file("k.pem"));
+    Section section = mtls::makeSection(true);
+    section.set("use_tls", "1");
+    section.set("tls_ca_cert", ca);
+    section.set(mtls::keyClientCert, cert);
+    section.set(mtls::keyClientKey, key);
+
+    telemetry::Telemetry::Setup setup;
+    ASSERT_NO_THROW(setup = mtls::parseSection(section));
+    EXPECT_TRUE(setup.enabled);
+    EXPECT_TRUE(setup.useTls);
+    EXPECT_EQ(setup.tlsCertPath, ca);
+    EXPECT_EQ(setup.tlsClientCertPath, cert);
+    EXPECT_EQ(setup.tlsClientKeyPath, key);
+}
+
+TEST(TelemetryConfig, tls_paths_not_checked_when_telemetry_disabled)
+{
+    // Telemetry off, so the files are never opened and absent paths must not
+    // stop the node from booting. use_tls stays 1 here, so the `enabled` gate
+    // is the only thing that can be suppressing the check.
+    TempDir const dir;
+    auto const absentCert = dir.file("absent.pem");
+    auto const absentKey = dir.file("absent.key");
+    Section section = mtls::makeSection(false);
+    section.set("use_tls", "1");
+    section.set(mtls::keyClientCert, absentCert);
+    section.set(mtls::keyClientKey, absentKey);
+
+    telemetry::Telemetry::Setup setup;
+    ASSERT_NO_THROW(setup = mtls::parseSection(section));
+    EXPECT_FALSE(setup.enabled);
+    EXPECT_TRUE(setup.useTls);
+    EXPECT_EQ(setup.tlsClientCertPath, absentCert);
+    EXPECT_EQ(setup.tlsClientKeyPath, absentKey);
+}
+
+TEST(TelemetryConfig, tls_ca_cert_not_checked_when_use_tls_off)
+{
+    // With TLS off the exporter never reads the CA path, so a missing file
+    // must not stop startup. Telemetry stays on here, so the use_tls gate is
+    // the only thing that can be suppressing the check. The client-cert keys
+    // cannot be used for this case: they trip the use_tls contradiction guard
+    // before any file is opened.
+    TempDir const dir;
+    auto const absentCa = dir.file("absent-ca.pem");
+    Section section = mtls::makeSection(true);
+    section.set("tls_ca_cert", absentCa);
+
+    telemetry::Telemetry::Setup setup;
+    ASSERT_NO_THROW(setup = mtls::parseSection(section));
+    EXPECT_TRUE(setup.enabled);
+    EXPECT_FALSE(setup.useTls);
+    EXPECT_EQ(setup.tlsCertPath, absentCa);
+}
+
 TEST(TelemetryConfig, null_telemetry_factory)
 {
     telemetry::Telemetry::Setup setup;