From a2cb4416e38ab6482629b730f64d68c4604c813e Mon Sep 17 00:00:00 2001 From: Adesh Kumar Date: Fri, 17 Apr 2026 14:12:50 -0400 Subject: [PATCH 1/2] feat(c-binding): accept explicit span timestamps Add `start_time_ns` to `dd_span_options_t` and introduce `dd_span_finish_with_time(end_time_ns)`. Both accept wall-clock nanoseconds since the UNIX epoch; 0 preserves today's "use current time" behaviour so existing callers are unaffected. The conversion from wall-ns to the tracer's internal `TimePoint` derives the steady_clock tick from the current system/steady offset, so duration math stays correct regardless of which endpoint is explicit and which is implicit. Motivated by FFI callers (e.g. the Kong plugin) that capture timestamps in their host runtime before crossing the FFI boundary, where using "now" inside the binding would lose the pre-FFI portion of the span and bake in call-overhead skew. Co-Authored-By: Claude Opus 4.7 (1M context) --- binding/c/include/datadog/c/tracer.h | 9 ++++++ binding/c/src/tracer.cpp | 41 ++++++++++++++++++++++++-- binding/c/test/test_c_binding.cpp | 43 ++++++++++++++++++++++++++-- 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/binding/c/include/datadog/c/tracer.h b/binding/c/include/datadog/c/tracer.h index 655d334d..2a7fbb3c 100644 --- a/binding/c/include/datadog/c/tracer.h +++ b/binding/c/include/datadog/c/tracer.h @@ -1,6 +1,7 @@ #pragma once #include +#include #if defined(_WIN32) #if defined(DD_TRACE_C_BUILDING) @@ -51,6 +52,7 @@ typedef struct { const char* service_type; const char* environment; const char* version; + int64_t start_time_ns; // UNIX-epoch nanoseconds; 0 = use current time } dd_span_options_t; // Error codes returned by the C binding. @@ -179,6 +181,13 @@ DD_TRACE_C_API dd_span_t* dd_span_create_child(dd_span_t* span_handle, // @param span_handle Span handle DD_TRACE_C_API void dd_span_finish(dd_span_t* span_handle); +// Finish a span using an explicit end time. No-op if span_handle is NULL. +// +// @param span_handle Span handle +// @param end_time_ns Wall-clock end time (UNIX-epoch ns); 0 = current time +DD_TRACE_C_API void dd_span_finish_with_time(dd_span_t* span_handle, + int64_t end_time_ns); + // Get the trace ID as a zero-padded hex string. // // @param span_handle Span handle diff --git a/binding/c/src/tracer.cpp b/binding/c/src/tracer.cpp index 2a81a40f..3c685a29 100644 --- a/binding/c/src/tracer.cpp +++ b/binding/c/src/tracer.cpp @@ -44,6 +44,18 @@ class ContextWriter : public dd::DictWriter { } }; +// tick is derived from the current system/steady offset so that duration +// (end.tick - start.tick) stays correct across FFI-supplied timestamps. +dd::TimePoint wall_ns_to_timepoint(int64_t wall_ns) { + const auto wall = std::chrono::system_clock::time_point( + std::chrono::round( + std::chrono::nanoseconds(wall_ns))); + const auto now_wall = std::chrono::system_clock::now(); + const auto now_tick = std::chrono::steady_clock::now(); + return {wall, + now_tick - std::chrono::duration_cast(now_wall - wall)}; +} + dd::SpanConfig make_span_config(dd_span_options_t options) { dd::SpanConfig span_config; if (options.name != nullptr) { @@ -64,6 +76,9 @@ dd::SpanConfig make_span_config(dd_span_options_t options) { if (options.version != nullptr) { span_config.version = options.version; } + if (options.start_time_ns != 0) { + span_config.start = wall_ns_to_timepoint(options.start_time_ns); + } return span_config; } @@ -250,11 +265,33 @@ dd_span_t *dd_span_create_child(dd_span_t *span_handle, } void dd_span_finish(dd_span_t *span_handle) { + dd_span_finish_with_time(span_handle, 0); +} + +void dd_span_finish_with_time(dd_span_t *span_handle, int64_t end_time_ns) { if (span_handle == nullptr) { return; } - reinterpret_cast(span_handle) - ->set_end_time(std::chrono::steady_clock::now()); + auto *span = reinterpret_cast(span_handle); + const auto start = span->start_time(); + // Explicit path anchors to start.wall so NTP adjustments during the + // span's lifetime don't skew duration; implicit path uses steady now. + // Both clamp to start.tick so a negative duration never ships + // (serialization casts duration to uint64_t). + std::chrono::steady_clock::time_point end_tick; + if (end_time_ns == 0) { + end_tick = std::chrono::steady_clock::now(); + } else { + const auto end_wall = std::chrono::system_clock::time_point( + std::chrono::round( + std::chrono::nanoseconds(end_time_ns))); + end_tick = + start.tick + + (end_wall > start.wall + ? std::chrono::duration_cast(end_wall - start.wall) + : dd::Duration::zero()); + } + span->set_end_time(end_tick > start.tick ? end_tick : start.tick); } int dd_span_get_trace_id(dd_span_t *span_handle, char *buffer, diff --git a/binding/c/test/test_c_binding.cpp b/binding/c/test/test_c_binding.cpp index 7b93516b..04bf7149 100644 --- a/binding/c/test/test_c_binding.cpp +++ b/binding/c/test/test_c_binding.cpp @@ -91,7 +91,11 @@ TEST_CASE("tracer new propagates error", "[c_binding]") { TEST_CASE("span create, tag, finish, free", "[c_binding]") { auto ctx = make_tracer(); - auto *span = dd_tracer_create_span(ctx.tracer, {.name = "test.op"}); + const int64_t start_ns = 1'700'000'000'000'000'000LL; + const int64_t end_ns = start_ns + 42'000'000LL; + + auto *span = dd_tracer_create_span( + ctx.tracer, {.name = "test.op", .start_time_ns = start_ns}); REQUIRE(span != nullptr); dd_span_set_tag(span, "http.method", "GET"); @@ -100,7 +104,7 @@ TEST_CASE("span create, tag, finish, free", "[c_binding]") { dd_span_set_error(span, 1); dd_span_set_error_message(span, "something broke"); - dd_span_finish(span); + dd_span_finish_with_time(span, end_ns); dd_span_free(span); const auto &sd = ctx.collector->first_span(); @@ -109,6 +113,40 @@ TEST_CASE("span create, tag, finish, free", "[c_binding]") { CHECK(sd.service == "user-service"); CHECK(sd.error == true); CHECK(sd.tags.at("error.message") == "something broke"); + + CHECK(std::chrono::duration_cast( + sd.start.wall.time_since_epoch()) + .count() == start_ns); + CHECK(std::chrono::abs(sd.duration - + std::chrono::nanoseconds(end_ns - start_ns)) < + std::chrono::milliseconds(1)); +} + +TEST_CASE("finish clamps negative durations to zero", "[c_binding]") { + auto ctx = make_tracer(); + + // Explicit end before explicit start. + const int64_t past_ns = 1'700'000'000'000'000'000LL; + auto *s1 = dd_tracer_create_span( + ctx.tracer, {.name = "explicit", .start_time_ns = past_ns}); + dd_span_finish_with_time(s1, past_ns - 1'000'000LL); + dd_span_free(s1); + + // Sentinel finish (steady_clock::now()) with a start projected into + // the future — exercises the implicit-path clamp. + const int64_t future_ns = + std::chrono::duration_cast( + (std::chrono::system_clock::now() + std::chrono::hours(1)) + .time_since_epoch()) + .count(); + auto *s2 = dd_tracer_create_span( + ctx.tracer, {.name = "implicit", .start_time_ns = future_ns}); + dd_span_finish(s2); + dd_span_free(s2); + + for (const auto &chunk : ctx.collector->chunks) + for (const auto &span : chunk) + CHECK(span->duration >= std::chrono::nanoseconds(0)); } TEST_CASE("create span with resource", "[c_binding]") { @@ -253,6 +291,7 @@ TEST_CASE("null arguments do not crash", "[c_binding]") { dd_span_set_error_message(nullptr, "msg"); dd_span_inject(nullptr, test_header_writer); dd_span_finish(nullptr); + dd_span_finish_with_time(nullptr, 0); dd_span_set_resource(nullptr, "res"); dd_span_set_service(nullptr, "svc"); } From 616f1e5801a273fcca4b53daa48724fdba172c6f Mon Sep 17 00:00:00 2001 From: Adesh Kumar Date: Mon, 20 Apr 2026 13:17:14 -0400 Subject: [PATCH 2/2] =?UTF-8?q?review:=20rename=20finish=5Fwith=5Ftime=20?= =?UTF-8?q?=E2=86=92=20set=5Fend=5Ftime,=20use=20-1=20sentinel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review feedback from @dmehala: 1. Rename dd_span_finish_with_time → dd_span_set_end_time to match the C++ Span::set_end_time and the existing dd_span_set_* naming family. 2. Expose DD_TRACE_CURRENT_TIME (-1) as the "use current time" sentinel for start_time_ns, so 0 (1970-01-01) stays a valid timestamp. 3. Drop the sentinel branch in dd_span_set_end_time; callers wanting the current end time use dd_span_finish, and this also removes the implicit-path clamp that was only defending against a future-start caller bug. Co-Authored-By: Claude Opus 4.7 (1M context) --- binding/c/include/datadog/c/tracer.h | 18 +++++++--- binding/c/src/tracer.cpp | 51 ++++++++++++++-------------- binding/c/test/test_c_binding.cpp | 34 ++++++------------- 3 files changed, 49 insertions(+), 54 deletions(-) diff --git a/binding/c/include/datadog/c/tracer.h b/binding/c/include/datadog/c/tracer.h index 2a7fbb3c..4c52f362 100644 --- a/binding/c/include/datadog/c/tracer.h +++ b/binding/c/include/datadog/c/tracer.h @@ -44,6 +44,9 @@ typedef enum { DD_OPT_INTEGRATION_VERSION = 5 } dd_tracer_option; +// Sentinel for start_time_ns meaning "use the current time." +static const int64_t DD_TRACE_CURRENT_TIME = -1; + // Options for creating a span. Unset fields default to NULL. typedef struct { const char* name; @@ -52,7 +55,7 @@ typedef struct { const char* service_type; const char* environment; const char* version; - int64_t start_time_ns; // UNIX-epoch nanoseconds; 0 = use current time + int64_t start_time_ns; // Unix-epoch nanoseconds or DD_TRACE_CURRENT_TIME. } dd_span_options_t; // Error codes returned by the C binding. @@ -181,12 +184,17 @@ DD_TRACE_C_API dd_span_t* dd_span_create_child(dd_span_t* span_handle, // @param span_handle Span handle DD_TRACE_C_API void dd_span_finish(dd_span_t* span_handle); -// Finish a span using an explicit end time. No-op if span_handle is NULL. +// Set the end time of a span from an explicit wall-clock timestamp. The +// span itself is finished when dd_span_free destroys it. To use the +// current time, call dd_span_finish instead. If end_time_ns is earlier +// than the span's start time, the resulting duration is clamped to zero. +// No-op if span_handle is NULL. // // @param span_handle Span handle -// @param end_time_ns Wall-clock end time (UNIX-epoch ns); 0 = current time -DD_TRACE_C_API void dd_span_finish_with_time(dd_span_t* span_handle, - int64_t end_time_ns); +// @param end_time_ns Wall-clock end time (Unix-epoch nanoseconds). +// DD_TRACE_CURRENT_TIME does not apply here. +DD_TRACE_C_API void dd_span_set_end_time(dd_span_t* span_handle, + int64_t end_time_ns); // Get the trace ID as a zero-padded hex string. // diff --git a/binding/c/src/tracer.cpp b/binding/c/src/tracer.cpp index 3c685a29..812f7d54 100644 --- a/binding/c/src/tracer.cpp +++ b/binding/c/src/tracer.cpp @@ -44,12 +44,18 @@ class ContextWriter : public dd::DictWriter { } }; -// tick is derived from the current system/steady offset so that duration -// (end.tick - start.tick) stays correct across FFI-supplied timestamps. -dd::TimePoint wall_ns_to_timepoint(int64_t wall_ns) { - const auto wall = std::chrono::system_clock::time_point( +std::chrono::system_clock::time_point wall_ns_to_system_time(int64_t wall_ns) { + return std::chrono::system_clock::time_point( std::chrono::round( std::chrono::nanoseconds(wall_ns))); +} + +// Create a TimePoint from a wall-clock timestamp in nanoseconds. The tick is +// derived from the wall clock using the current offset between the system and +// the steady clocks. This keeps duration computed from ticks accurate when +// the start time is supplied via FFI. +dd::TimePoint wall_ns_to_timepoint(int64_t wall_ns) { + const auto wall = wall_ns_to_system_time(wall_ns); const auto now_wall = std::chrono::system_clock::now(); const auto now_tick = std::chrono::steady_clock::now(); return {wall, @@ -76,7 +82,7 @@ dd::SpanConfig make_span_config(dd_span_options_t options) { if (options.version != nullptr) { span_config.version = options.version; } - if (options.start_time_ns != 0) { + if (options.start_time_ns != DD_TRACE_CURRENT_TIME) { span_config.start = wall_ns_to_timepoint(options.start_time_ns); } return span_config; @@ -265,33 +271,28 @@ dd_span_t *dd_span_create_child(dd_span_t *span_handle, } void dd_span_finish(dd_span_t *span_handle) { - dd_span_finish_with_time(span_handle, 0); + if (span_handle == nullptr) { + return; + } + reinterpret_cast(span_handle) + ->set_end_time(std::chrono::steady_clock::now()); } -void dd_span_finish_with_time(dd_span_t *span_handle, int64_t end_time_ns) { +void dd_span_set_end_time(dd_span_t *span_handle, int64_t end_time_ns) { if (span_handle == nullptr) { return; } auto *span = reinterpret_cast(span_handle); + // Anchor end.tick to start.tick + wall delta so NTP adjustments during the + // span's lifetime don't skew duration. Clamp to start.tick on negative + // deltas — serialization casts duration to uint64_t. const auto start = span->start_time(); - // Explicit path anchors to start.wall so NTP adjustments during the - // span's lifetime don't skew duration; implicit path uses steady now. - // Both clamp to start.tick so a negative duration never ships - // (serialization casts duration to uint64_t). - std::chrono::steady_clock::time_point end_tick; - if (end_time_ns == 0) { - end_tick = std::chrono::steady_clock::now(); - } else { - const auto end_wall = std::chrono::system_clock::time_point( - std::chrono::round( - std::chrono::nanoseconds(end_time_ns))); - end_tick = - start.tick + - (end_wall > start.wall - ? std::chrono::duration_cast(end_wall - start.wall) - : dd::Duration::zero()); - } - span->set_end_time(end_tick > start.tick ? end_tick : start.tick); + const auto end_wall = wall_ns_to_system_time(end_time_ns); + const auto duration = + end_wall > start.wall + ? std::chrono::duration_cast(end_wall - start.wall) + : dd::Duration::zero(); + span->set_end_time(start.tick + duration); } int dd_span_get_trace_id(dd_span_t *span_handle, char *buffer, diff --git a/binding/c/test/test_c_binding.cpp b/binding/c/test/test_c_binding.cpp index 04bf7149..db102bb8 100644 --- a/binding/c/test/test_c_binding.cpp +++ b/binding/c/test/test_c_binding.cpp @@ -104,7 +104,7 @@ TEST_CASE("span create, tag, finish, free", "[c_binding]") { dd_span_set_error(span, 1); dd_span_set_error_message(span, "something broke"); - dd_span_finish_with_time(span, end_ns); + dd_span_set_end_time(span, end_ns); dd_span_free(span); const auto &sd = ctx.collector->first_span(); @@ -122,31 +122,17 @@ TEST_CASE("span create, tag, finish, free", "[c_binding]") { std::chrono::milliseconds(1)); } -TEST_CASE("finish clamps negative durations to zero", "[c_binding]") { +TEST_CASE("set_end_time before start clamps to zero duration", "[c_binding]") { auto ctx = make_tracer(); - // Explicit end before explicit start. const int64_t past_ns = 1'700'000'000'000'000'000LL; - auto *s1 = dd_tracer_create_span( - ctx.tracer, {.name = "explicit", .start_time_ns = past_ns}); - dd_span_finish_with_time(s1, past_ns - 1'000'000LL); - dd_span_free(s1); - - // Sentinel finish (steady_clock::now()) with a start projected into - // the future — exercises the implicit-path clamp. - const int64_t future_ns = - std::chrono::duration_cast( - (std::chrono::system_clock::now() + std::chrono::hours(1)) - .time_since_epoch()) - .count(); - auto *s2 = dd_tracer_create_span( - ctx.tracer, {.name = "implicit", .start_time_ns = future_ns}); - dd_span_finish(s2); - dd_span_free(s2); - - for (const auto &chunk : ctx.collector->chunks) - for (const auto &span : chunk) - CHECK(span->duration >= std::chrono::nanoseconds(0)); + auto *span = dd_tracer_create_span( + ctx.tracer, {.name = "reversed", .start_time_ns = past_ns}); + REQUIRE(span != nullptr); + dd_span_set_end_time(span, past_ns - 1'000'000LL); + dd_span_free(span); + + CHECK(ctx.collector->first_span().duration >= std::chrono::nanoseconds(0)); } TEST_CASE("create span with resource", "[c_binding]") { @@ -291,7 +277,7 @@ TEST_CASE("null arguments do not crash", "[c_binding]") { dd_span_set_error_message(nullptr, "msg"); dd_span_inject(nullptr, test_header_writer); dd_span_finish(nullptr); - dd_span_finish_with_time(nullptr, 0); + dd_span_set_end_time(nullptr, 0); dd_span_set_resource(nullptr, "res"); dd_span_set_service(nullptr, "svc"); }