Skip to content

Commit a759d90

Browse files
authored
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 <st810918843@gmail.com> Signed-off-by: David Li <li.davidm96@gmail.com>
1 parent c8fe268 commit a759d90

3 files changed

Lines changed: 31 additions & 1 deletion

File tree

cpp/src/arrow/util/tracing.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ bool Span::valid() const {
3737
return static_cast<::arrow::internal::tracing::SpanImpl*>(details.get())->valid();
3838
}
3939

40-
void Span::reset() { details.reset(); }
40+
void Span::reset() {
41+
static_cast<::arrow::internal::tracing::SpanImpl*>(details.get())->reset();
42+
}
4143

4244
#else
4345

cpp/src/arrow/util/tracing_internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ class SpanImpl : public ::arrow::util::tracing::SpanDetails {
110110
public:
111111
~SpanImpl() override = default;
112112
bool valid() const { return ot_span != nullptr; }
113+
void reset() { ot_span = nullptr; }
113114
opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> ot_span;
114115
};
115116

cpp/src/arrow/util/tracing_test.cc

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,33 @@ TEST(Tracing, OtLifetime) {
4646
}));
4747
}
4848

49+
// This test checks that the Span valid invariant is maintained:
50+
// 1. Span is invalid before START_SPAN
51+
// 2. Span is valid after START_SPAN
52+
// 3. Span is invalid after reset
53+
// 4. Span can be restarted after reset
54+
TEST(Tracing, ValidInvariant) {
55+
Span span;
56+
57+
EXPECT_FALSE(span.valid());
58+
59+
START_SPAN(span, "TestSpan");
60+
61+
EXPECT_TRUE(span.valid());
62+
63+
span.reset();
64+
65+
EXPECT_FALSE(span.valid());
66+
67+
span.reset();
68+
69+
EXPECT_FALSE(span.valid());
70+
{
71+
START_SPAN(span, "TestSpan2");
72+
EXPECT_TRUE(span.valid());
73+
}
74+
}
75+
4976
#endif
5077

5178
} // namespace tracing

0 commit comments

Comments
 (0)