This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new a759d90318 GH-47040: [C++] Refine reset of Span to be reusable (#47004)
a759d90318 is described below
commit a759d903183b8bd4060677154b15a0c61656d372
Author: ZENOTME <[email protected]>
AuthorDate: Fri Jul 18 11:36:54 2025 +0800
GH-47040: [C++] Refine reset of Span to be reusable (#47004)
### Rationale for this change
Original reset will cause Span can't be used again, e.g.
```
Span span;
span.reset();
span.valid(); // crash
RewrapSpan(span, ..); // crash
```
Instead of reset the pointer to SpanImpl, maybe we should reset content
inside SpanImpl.
### What changes are included in this PR?
Add reset function in SpanImpl and reimplement reset of Span
### Are these changes tested?
No
### Are there any user-facing changes?
No
* GitHub Issue: #47040
Authored-by: ZENOTME <[email protected]>
Signed-off-by: David Li <[email protected]>
---
cpp/src/arrow/util/tracing.cc | 4 +++-
cpp/src/arrow/util/tracing_internal.h | 1 +
cpp/src/arrow/util/tracing_test.cc | 27 +++++++++++++++++++++++++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/cpp/src/arrow/util/tracing.cc b/cpp/src/arrow/util/tracing.cc
index f4c18f1236..18257eced7 100644
--- a/cpp/src/arrow/util/tracing.cc
+++ b/cpp/src/arrow/util/tracing.cc
@@ -37,7 +37,9 @@ bool Span::valid() const {
return
static_cast<::arrow::internal::tracing::SpanImpl*>(details.get())->valid();
}
-void Span::reset() { details.reset(); }
+void Span::reset() {
+ static_cast<::arrow::internal::tracing::SpanImpl*>(details.get())->reset();
+}
#else
diff --git a/cpp/src/arrow/util/tracing_internal.h
b/cpp/src/arrow/util/tracing_internal.h
index 6ed731599a..3696e6e003 100644
--- a/cpp/src/arrow/util/tracing_internal.h
+++ b/cpp/src/arrow/util/tracing_internal.h
@@ -110,6 +110,7 @@ class SpanImpl : public ::arrow::util::tracing::SpanDetails
{
public:
~SpanImpl() override = default;
bool valid() const { return ot_span != nullptr; }
+ void reset() { ot_span = nullptr; }
opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> ot_span;
};
diff --git a/cpp/src/arrow/util/tracing_test.cc
b/cpp/src/arrow/util/tracing_test.cc
index 08d737ddfd..b4f67b42d3 100644
--- a/cpp/src/arrow/util/tracing_test.cc
+++ b/cpp/src/arrow/util/tracing_test.cc
@@ -46,6 +46,33 @@ TEST(Tracing, OtLifetime) {
}));
}
+// This test checks that the Span valid invariant is maintained:
+// 1. Span is invalid before START_SPAN
+// 2. Span is valid after START_SPAN
+// 3. Span is invalid after reset
+// 4. Span can be restarted after reset
+TEST(Tracing, ValidInvariant) {
+ Span span;
+
+ EXPECT_FALSE(span.valid());
+
+ START_SPAN(span, "TestSpan");
+
+ EXPECT_TRUE(span.valid());
+
+ span.reset();
+
+ EXPECT_FALSE(span.valid());
+
+ span.reset();
+
+ EXPECT_FALSE(span.valid());
+ {
+ START_SPAN(span, "TestSpan2");
+ EXPECT_TRUE(span.valid());
+ }
+}
+
#endif
} // namespace tracing
