mirror of
https://github.com/XRPLF/rippled.git
synced 2026-09-26 15:10:12 +00:00
fix(telemetry): reject invalid [telemetry] batch settings and make isValid() honest
Two review findings on the telemetry library. SpanContext::isValid() returned impl_ != nullptr, so it answered true for a context holding no span. threadLocalContext() wraps whatever GetCurrent() returns, and that is an empty Context on a thread with no active span, which contradicted the documented "invalid context if none is active". It now asks the Context for its span. childSpan(name, ctx) is the one caller whose behaviour changes: a context with no span used to produce a new root span, and now returns a null guard as its @return already promised. The three batch settings went to the OTel BatchSpanProcessor unchecked. Three ways that failed: zero was accepted for all of them; batch_size could exceed max_queue_size, which the SDK documents as a precondition and does not enforce; and a mistyped value let boost::bad_lexical_cast escape, which derives from std::bad_cast rather than std::runtime_error, so the operator saw a bare "bad cast" naming no key. Reading unsigned also turned "-1" into 4294967295 instead of failing, so the value is parsed signed and negatives are rejected. xrpld-example.cfg now states the ranges.
This commit is contained in:
@@ -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
|
||||
#
|
||||
|
||||
@@ -77,7 +77,10 @@ SpanContext::SpanContext(std::shared_ptr<Impl> 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 ====================================================
|
||||
|
||||
@@ -8,11 +8,15 @@
|
||||
* See cfg/xrpld-example.cfg for the full list of available options.
|
||||
*/
|
||||
|
||||
#include <xrpl/basics/contract.h>
|
||||
#include <xrpl/config/BasicConfig.h>
|
||||
#include <xrpl/telemetry/Telemetry.h>
|
||||
|
||||
#include <chrono>
|
||||
#include <cstdint>
|
||||
#include <limits>
|
||||
#include <optional>
|
||||
#include <stdexcept>
|
||||
#include <string>
|
||||
|
||||
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<std::int64_t> parsed;
|
||||
try
|
||||
{
|
||||
parsed = section.get<std::int64_t>(name);
|
||||
}
|
||||
catch (...)
|
||||
{
|
||||
Throw<std::runtime_error>(
|
||||
std::string("Invalid value '") + name + "' in " + kSectionLabel +
|
||||
": must be a whole number.");
|
||||
}
|
||||
|
||||
if (!parsed)
|
||||
return absentValue;
|
||||
|
||||
constexpr auto maxValue = static_cast<std::int64_t>(std::numeric_limits<std::uint32_t>::max());
|
||||
if (*parsed < static_cast<std::int64_t>(minValue) || *parsed > maxValue)
|
||||
{
|
||||
Throw<std::runtime_error>(
|
||||
std::string("Invalid value '") + name + "' in " + kSectionLabel + ": must be between " +
|
||||
std::to_string(minValue) + " and " + std::to_string(maxValue) + ".");
|
||||
}
|
||||
|
||||
return static_cast<std::uint32_t>(*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<std::uint32_t>(key::batchSize, dflt::batchSize);
|
||||
setup.batchSize = readBounded(section, key::batchSize, dflt::batchSize, kMinBatchSetting);
|
||||
setup.batchDelay = std::chrono::milliseconds{
|
||||
section.valueOr<std::uint32_t>(key::batchDelayMs, dflt::batchDelayMs)};
|
||||
setup.maxQueueSize = section.valueOr<std::uint32_t>(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::runtime_error>(
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user