review: rename finish_with_time → set_end_time, use -1 sentinel

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) <noreply@anthropic.com>

Author: adeshkumar1

binding/c/include/datadog/c/tracer.h

@@ -44,6 +44,10 @@ typedef enum {
   DD_OPT_INTEGRATION_VERSION = 5
 } dd_tracer_option;
 
+// Sentinel for start_time_ns / dd_span_set_end_time meaning "use the
+// current time." Chosen so 0 (1970-01-01) remains a valid timestamp.
+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 +56,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 ns; DD_TRACE_CURRENT_TIME for now
 } dd_span_options_t;
 
 // Error codes returned by the C binding.
@@ -181,12 +185,14 @@ 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. 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 ns)
+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.
 //

binding/c/src/tracer.cpp

@@ -76,7 +76,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 +265,30 @@ 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<dd::Span *>(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<dd::Span *>(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::system_clock::duration>(
-            std::chrono::nanoseconds(end_time_ns)));
-    end_tick =
-        start.tick +
-        (end_wall > start.wall
-             ? std::chrono::duration_cast<dd::Duration>(end_wall - start.wall)
-             : dd::Duration::zero());
-  }
-  span->set_end_time(end_tick > start.tick ? end_tick : start.tick);
+  const auto end_wall = std::chrono::system_clock::time_point(
+      std::chrono::round<std::chrono::system_clock::duration>(
+          std::chrono::nanoseconds(end_time_ns)));
+  const auto duration =
+      end_wall > start.wall
+          ? std::chrono::duration_cast<dd::Duration>(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,

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::nanoseconds>(
-          (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");
 }