From: Otto Moerbeek Date: Tue, 24 Jun 2025 07:21:41 +0000 (+0200) Subject: Process review comments by rgacogne and Habbie X-Git-Tag: rec-5.3.0-alpha1^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=239b00d7babdd9dbd1f6f2c2907977e7d6f201cb;p=thirdparty%2Fpdns.git Process review comments by rgacogne and Habbie Signed-off-by: Otto Moerbeek --- diff --git a/pdns/protozero-trace.hh b/pdns/protozero-trace.hh index ffa73c7c16..aa3b6d6d79 100644 --- a/pdns/protozero-trace.hh +++ b/pdns/protozero-trace.hh @@ -37,6 +37,8 @@ namespace pdns::trace { +// https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/common/v1/common.proto + struct AnyValue; struct ArrayValue; struct KeyValue; @@ -169,7 +171,8 @@ struct KeyValueList } }; -struct AnyValue : public std::variant> +using NoValue = char; +struct AnyValue : public std::variant> { void encode(protozero::pbf_writer& writer) const; static AnyValue decode(protozero::pbf_reader& reader); diff --git a/pdns/recursordist/docs/performance.rst b/pdns/recursordist/docs/performance.rst index f5e3cdfa4c..de7d9fa0ed 100644 --- a/pdns/recursordist/docs/performance.rst +++ b/pdns/recursordist/docs/performance.rst @@ -412,11 +412,11 @@ There is no packet cache hit, so SyncRes is called which does a couple of outgoi OpenTelemetry Traces ^^^^^^^^^^^^^^^^^^^^ -OpenTelemetry Traces can be generated by setting :ref:`setting-yaml-recursor.event_trace_enabled` or by using the :doc:`rec_control ` subcommand ``set-event-trace-enabled``. -:program:`Recursor` will set the ``openTelemetryData`` field of ``dnsmessage.proto`` messages crated to contain Protobuf encoded data. +OpenTelemetry Traces are generated by setting :ref:`setting-yaml-recursor.event_trace_enabled` or by using the :doc:`rec_control ` subcommand ``set-event-trace-enabled``. +:program:`Recursor` will set the ``openTelemetryData`` field of ``dnsmessage.proto`` messages generated to contain OpenTelemetry Traces, encoded as Protobuf data. The encoding used is defined in https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/trace/v1/trace.proto. -The example decoder in https://github.com/PowerDNS/pdns/blob/master/contrib/ProtobufLogger.py can show a JSON representation of the data and optionally submit the data (in Protobuf format) to an OpenTelemetry trace collector using a POST to a URL given on the command line. +The example decoder in https://github.com/PowerDNS/pdns/blob/master/contrib/ProtobufLogger.py displays a JSON representation of the data and optionally submits the data (in Protobuf format) to an OpenTelemetry trace collector using a POST to a URL given on the command line. An example, encoded in JSON: diff --git a/pdns/recursordist/rec-eventtrace.cc b/pdns/recursordist/rec-eventtrace.cc index 5c63e946e0..52c9fe7601 100644 --- a/pdns/recursordist/rec-eventtrace.cc +++ b/pdns/recursordist/rec-eventtrace.cc @@ -111,8 +111,8 @@ std::vector RecEventTrace::convertToOT(const Span& span) cons } else { // It's a close event - if (ids.find(event.d_matching) != ids.end()) { - auto& work = ret.at(ids.at(event.d_matching)); + if (const auto match = ids.find(event.d_matching); match != ids.end()) { + auto& work = ret.at(match->second); addValue(event, work, false); work.end_time_unix_nano = static_cast(event.d_ts + diff); spanIDs.emplace_back(work.span_id); diff --git a/pdns/recursordist/rec-eventtrace.hh b/pdns/recursordist/rec-eventtrace.hh index d5a8e1e291..1623b0c2ab 100644 --- a/pdns/recursordist/rec-eventtrace.hh +++ b/pdns/recursordist/rec-eventtrace.hh @@ -275,6 +275,9 @@ public: return old; } + // The EventScope class is used to close (add an end event) automatically upon the scope object + // going out of scope. It is also possible to manually close it, specifying a value to be registered + // at the close event. In that case the dt call will become a no-op. class EventScope { public: @@ -282,12 +285,13 @@ public: d_eventTrace(eventTrace), d_oldParent(oldParent) { - if (d_eventTrace.enabled()) { + if (d_eventTrace.enabled() && !d_eventTrace.d_events.empty()) { d_event = d_eventTrace.d_events.back().d_event; d_match = d_eventTrace.d_events.size() - 1; } } + // Only int64_t for now needed, might become a template in the future. void close(int64_t val) { if (!d_eventTrace.enabled() || d_closed) { @@ -300,6 +304,8 @@ public: ~EventScope() { + // If the dt is called after an explicit close(), value does not matter. + // Otherwise, it signals a implicit close, e.g. an exception was thrown close(-1); } EventScope(const EventScope&) = delete; diff --git a/pdns/recursordist/test-protozero-trace.cc b/pdns/recursordist/test-protozero-trace.cc index 2ecf860103..68c2561609 100644 --- a/pdns/recursordist/test-protozero-trace.cc +++ b/pdns/recursordist/test-protozero-trace.cc @@ -60,7 +60,7 @@ static void testAny(const T& testcase) protozero::pbf_reader reader{data}; pdns::trace::AnyValue value = pdns::trace::AnyValue::decode(reader); - if (!std::holds_alternative(value)) { + if (!std::holds_alternative(value)) { BOOST_CHECK(testcase == std::get(value)); } else { @@ -116,14 +116,14 @@ BOOST_AUTO_TEST_CASE(traces1) .span_id = {0xEE, 0xE1, 0x9B, 0x7E, 0xC3, 0xC1, 0xB1, 0x74}, .parent_span_id = {0xEE, 0xE1, 0x9B, 0x7E, 0xC3, 0xC1, 0xB1, 0x73}, .name = "I'm a server span", + .kind = pdns::trace::Span::SpanKind::SPAN_KINSERVER, .start_time_unix_nano = 1544712660000000000UL, .end_time_unix_nano = 1544712661000000000UL, - .kind = pdns::trace::Span::SpanKind::SPAN_KINSERVER, .attributes = {{"my.span.attr", {"some value"}}}}; pdns::trace::InstrumentationScope scope = {"my.library", "1.0.0", {{"my.scope.attribute", {"some scope attribute"}}}}; pdns::trace::ScopeSpans scopespans = {.scope = scope, .spans = {span}}; pdns::trace::Resource res = {.attributes = {{"service.name", {"my.service"}}}}; - pdns::trace::ResourceSpans resspans = {{res}, .scope_spans = {scopespans}}; + pdns::trace::ResourceSpans resspans = {.resource = res, .scope_spans = {scopespans}}; pdns::trace::TracesData traces = {.resource_spans = {resspans}}; std::string data;