From d0819eadcedff6f9d728e8ef7370d297bac1a97f Mon Sep 17 00:00:00 2001
From: Brett McBride <brett@deakin.edu.au>
Date: Tue, 18 Jun 2024 18:37:50 +1000
Subject: [PATCH] move dropped warning into toSpanData, add trace+span id
 moving the warning removes an extra call to toSpanData, thus some overhead.
 Outside of tests, this function is only called from span processors once

---
 src/SDK/Trace/Span.php            | 23 +++++++++++++----------
 tests/Unit/SDK/Trace/SpanTest.php | 17 ++++++++++-------
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/SDK/Trace/Span.php b/src/SDK/Trace/Span.php
index 4196ff96f..2eee38e09 100644
--- a/src/SDK/Trace/Span.php
+++ b/src/SDK/Trace/Span.php
@@ -251,15 +251,6 @@ public function end(int $endEpochNanos = null): void
             return;
         }
 
-        $spanData = $this->toSpanData();
-        if ($spanData->getTotalDroppedLinks() || $spanData->getTotalDroppedEvents() || $spanData->getAttributes()->getDroppedAttributesCount()) {
-            self::logWarning('Dropped span attributes, links or events', [
-                'attributes' => $spanData->getAttributes()->getDroppedAttributesCount(),
-                'links' => $spanData->getTotalDroppedLinks(),
-                'events' => $spanData->getTotalDroppedEvents(),
-            ]);
-        }
-
         $this->endEpochNanos = $endEpochNanos ?? Clock::getDefault()->now();
         $this->hasEnded = true;
 
@@ -289,7 +280,7 @@ public function hasEnded(): bool
 
     public function toSpanData(): SpanDataInterface
     {
-        return new ImmutableSpan(
+        $spanData = new ImmutableSpan(
             $this,
             $this->name,
             $this->links,
@@ -301,6 +292,18 @@ public function toSpanData(): SpanDataInterface
             $this->endEpochNanos,
             $this->hasEnded
         );
+
+        if ($spanData->getTotalDroppedLinks() || $spanData->getTotalDroppedEvents() || $spanData->getAttributes()->getDroppedAttributesCount()) {
+            self::logWarning('Dropped span attributes, links or events', [
+                'trace_id' => $spanData->getTraceId(),
+                'span_id' => $spanData->getSpanId(),
+                'attributes' => $spanData->getAttributes()->getDroppedAttributesCount(),
+                'links' => $spanData->getTotalDroppedLinks(),
+                'events' => $spanData->getTotalDroppedEvents(),
+            ]);
+        }
+
+        return $spanData;
     }
 
     /** @inheritDoc */
diff --git a/tests/Unit/SDK/Trace/SpanTest.php b/tests/Unit/SDK/Trace/SpanTest.php
index 810dd2370..a17dd1554 100644
--- a/tests/Unit/SDK/Trace/SpanTest.php
+++ b/tests/Unit/SDK/Trace/SpanTest.php
@@ -943,15 +943,18 @@ private function assertSpanData(
 
     private function expectDropped(int $attributes, int $events, int $links): void
     {
-        $this->logWriter->expects($this->once())->method('write')->with(
+        $this->logWriter->expects($this->atLeastOnce())->method('write')->with(
             $this->anything(),
             $this->stringContains('Dropped span attributes'),
-            $this->equalTo([
-                'attributes' => $attributes,
-                'links' => $links,
-                'events' => $events,
-                'source' => 'OpenTelemetry\\SDK\\Trace\\Span',
-            ])
+            $this->callback(function (array $context) use ($attributes, $events, $links) {
+                $this->assertSame($context['attributes'], $attributes);
+                $this->assertSame($context['events'], $events);
+                $this->assertSame($context['links'], $links);
+                $this->assertNotNull($context['trace_id']);
+                $this->assertNotNull($context['span_id']);
+
+                return true;
+            }),
         );
     }
 }