From ae15db60ad9c56b87dfdf48d450d2f75f3e2efbc Mon Sep 17 00:00:00 2001
From: Christophe Marchal <christophe@getonce.com>
Date: Tue, 21 Aug 2018 15:39:17 +0100
Subject: [PATCH] Stop overriding completion_time when finishing span (#60)

* Stop overriding completion_time when finishing span
---
 lib/spandex.ex     | 10 ++++++++--
 test/span_test.exs | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/lib/spandex.ex b/lib/spandex.ex
index c45c539..1fa4ee7 100644
--- a/lib/spandex.ex
+++ b/lib/spandex.ex
@@ -131,7 +131,7 @@ defmodule Spandex do
         {:error, :no_trace_context}
 
       %Trace{spans: spans, stack: stack} ->
-        unfinished_spans = Enum.map(stack, &update_or_keep(&1, completion_time: adapter.now()))
+        unfinished_spans = Enum.map(stack, &ensure_completion_time_set(&1, adapter))
 
         sender = opts[:sender] || adapter.default_sender()
 
@@ -157,13 +157,19 @@ defmodule Spandex do
         {:error, :no_span_context}
 
       %Trace{stack: [span | tail], spans: spans} = trace ->
-        finished_span = update_or_keep(span, completion_time: adapter.now())
+        finished_span = ensure_completion_time_set(span, adapter)
 
         strategy.put_trace(opts[:tracer], %{trace | stack: tail, spans: [finished_span | spans]})
         {:ok, finished_span}
     end
   end
 
+  defp ensure_completion_time_set(%Span{completion_time: nil} = span, adapter) do
+    update_or_keep(span, completion_time: adapter.now())
+  end
+
+  defp ensure_completion_time_set(%Span{} = span, _adapter), do: span
+
   def span_error(_error, _stacktrace, :disabled), do: {:error, :disabled}
 
   def span_error(exception, stacktrace, opts) do
diff --git a/test/span_test.exs b/test/span_test.exs
index 76af194..a902e18 100644
--- a/test/span_test.exs
+++ b/test/span_test.exs
@@ -14,4 +14,24 @@ defmodule Spandex.Test.SpanTest do
 
     assert(span.service == :special_service)
   end
+
+  test "finishing a span does not override the completion time" do
+    completion_time = :os.system_time(:nano_seconds)
+    Tracer.start_trace("my_trace")
+    Tracer.update_span(service: :my_app, type: :web, completion_time: completion_time)
+    Tracer.finish_span()
+    Tracer.finish_trace()
+
+    span = Util.find_span("my_trace")
+    assert(span.completion_time == completion_time)
+  end
+
+  test "unfinished spans should have a completion time after trace finishes" do
+    Tracer.start_trace("my_trace")
+    Tracer.update_span(service: :my_app, type: :web)
+    Tracer.finish_trace()
+    span = Util.find_span("my_trace")
+
+    assert(span.completion_time != nil)
+  end
 end