diff --git a/cfg/xrpld-example.cfg b/cfg/xrpld-example.cfg index 90f6c4f086..a476830280 100644 --- a/cfg/xrpld-example.cfg +++ b/cfg/xrpld-example.cfg @@ -1751,19 +1751,21 @@ validators.txt # # Maximum number of spans in one export request. A batch is sent once # this many spans have queued up, or once batch_delay_ms has elapsed, -# whichever happens first. Default: 512. +# whichever happens first. Must be at least 1 and must not exceed +# max_queue_size. Default: 512. # # batch_delay_ms=5000 # # Longest a queued span waits before its batch is exported, in # milliseconds. Lower it for fresher traces at the cost of more -# export requests. Default: 5000 (5 seconds). +# export requests. Must be at least 1. Default: 5000 (5 seconds). # # max_queue_size=2048 # # Maximum number of spans held in memory awaiting export. Spans are # dropped once the queue is full, so raise this if the collector is -# slow or briefly unreachable. Default: 2048. +# slow or briefly unreachable. Must be at least 1 and at least as +# large as batch_size. Default: 2048. # # trace_rpc=1 # diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp index a165d0ad5a..63c48c9a72 100644 --- a/src/libxrpl/telemetry/SpanGuard.cpp +++ b/src/libxrpl/telemetry/SpanGuard.cpp @@ -77,7 +77,10 @@ SpanContext::SpanContext(std::shared_ptr impl) : impl_(std::move(impl)) bool SpanContext::isValid() const noexcept { - return impl_ != nullptr; + // Holding a Context is not proof of holding a span. GetCurrent() hands back + // an empty Context on a thread with no active span, and threadLocalContext() + // wraps that too. Ask the Context for its span instead of trusting impl_. + return impl_ != nullptr && otel_trace::GetSpan(impl_->ctx)->GetContext().IsValid(); } // ===== SpanGuard::Impl ==================================================== diff --git a/src/libxrpl/telemetry/TelemetryConfig.cpp b/src/libxrpl/telemetry/TelemetryConfig.cpp index 4cbbbf2a98..4f3698383c 100644 --- a/src/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/libxrpl/telemetry/TelemetryConfig.cpp @@ -8,11 +8,15 @@ * See cfg/xrpld-example.cfg for the full list of available options. */ +#include #include #include #include #include +#include +#include +#include #include namespace xrpl::telemetry { @@ -60,6 +64,71 @@ constexpr std::uint32_t batchDelayMs = 5000u; constexpr std::uint32_t maxQueueSize = 2048u; } // namespace dflt +/** + * Smallest accepted value for the three batch settings. + * + * All three size a queue or a timer, so zero is meaningless for every one of + * them. The OTel BatchSpanProcessor takes them as given and does not validate, + * so the config parser is the only place a nonsense value can be rejected. + */ +constexpr std::uint32_t kMinBatchSetting = 1u; + +/** + * Section name used in error messages, so the operator knows where to look. + */ +constexpr char const* kSectionLabel = "[telemetry]"; + +/** + * Read a config value and reject anything outside minValue..UINT32_MAX. + * + * Section::get() lets boost::bad_lexical_cast escape. That derives from + * std::bad_cast, not std::runtime_error, so a mistyped value gives the operator + * a bare "bad cast" naming no key. Wrap it and rethrow with the key name. + * + * @param section The [telemetry] section to read from. + * @param name Key to read, as documented in cfg/xrpld-example.cfg. + * @param absentValue Value returned when the key is absent. + * @param minValue Smallest accepted value. + * @return The configured value, or absentValue if the key is absent. + * @note Throws std::runtime_error for a value that is not a whole number, and + * for one out of range, with a different message for each. + */ +[[nodiscard]] std::uint32_t +readBounded( + Section const& section, + char const* name, + std::uint32_t absentValue, + std::uint32_t minValue) +{ + // Read as signed. boost::lexical_cast to an unsigned type wraps a leading + // minus instead of failing ("-1" yields 4294967295), so reading signed is + // the only way to see a negative value and reject it below. + std::optional parsed; + try + { + parsed = section.get(name); + } + catch (...) + { + Throw( + std::string("Invalid value '") + name + "' in " + kSectionLabel + + ": must be a whole number."); + } + + if (!parsed) + return absentValue; + + constexpr auto maxValue = static_cast(std::numeric_limits::max()); + if (*parsed < static_cast(minValue) || *parsed > maxValue) + { + Throw( + std::string("Invalid value '") + name + "' in " + kSectionLabel + ": must be between " + + std::to_string(minValue) + " and " + std::to_string(maxValue) + "."); + } + + return static_cast(*parsed); +} + /** * Derive a human-readable network type label from the numeric network ID. * @param networkId The network identifier from [network_id] config. @@ -108,10 +177,22 @@ makeTelemetrySetup( // traces; volume reduction is delegated to the collector's tail sampling. // setup.samplingRatio is a const member fixed at 1.0; nothing to parse. - setup.batchSize = section.valueOr(key::batchSize, dflt::batchSize); + setup.batchSize = readBounded(section, key::batchSize, dflt::batchSize, kMinBatchSetting); setup.batchDelay = std::chrono::milliseconds{ - section.valueOr(key::batchDelayMs, dflt::batchDelayMs)}; - setup.maxQueueSize = section.valueOr(key::maxQueueSize, dflt::maxQueueSize); + readBounded(section, key::batchDelayMs, dflt::batchDelayMs, kMinBatchSetting)}; + setup.maxQueueSize = + readBounded(section, key::maxQueueSize, dflt::maxQueueSize, kMinBatchSetting); + + // The OTel SDK documents max_export_batch_size <= max_queue_size as a + // precondition of BatchSpanProcessorOptions and does not enforce it, so + // reject the pair here rather than hand the SDK a state it forbids. + if (setup.batchSize > setup.maxQueueSize) + { + Throw( + std::string("Invalid value '") + key::batchSize + "' in " + kSectionLabel + + ": must not exceed '" + key::maxQueueSize + "' (" + std::to_string(setup.maxQueueSize) + + ")."); + } setup.networkId = networkId; setup.networkType = networkTypeFromId(networkId);