[GitHub] [arrow] lidavidm commented on a diff in pull request #12702: ARROW-15062: [C++] Add memory information to current spans

[GitBox <gi...@apache.org>]

lidavidm commented on code in PR #12702:
URL: https://github.com/apache/arrow/pull/12702#discussion_r854064388


##########
cpp/src/arrow/util/tracing_internal.h:
##########
@@ -114,23 +117,38 @@ class SpanImpl {
 opentelemetry::trace::StartSpanOptions SpanOptionsWithParent(
     const util::tracing::Span& parent_span);
 
-#define START_SPAN(target_span, ...)                                                \
-  auto opentelemetry_scope##__LINE__ =                                              \
-      ::arrow::internal::tracing::GetTracer()->WithActiveSpan(                      \
-          target_span                                                               \
-              .Set(::arrow::util::tracing::Span::Impl{                              \
-                  ::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__)}) \
-              .span)
+#define FIRST_ARG(first, ...) first
+
+#define START_COMPUTE_SPAN(target_span, ...) \
+  START_SPAN(target_span, __VA_ARGS__);      \
+  START_SPAN(target_span, FIRST_ARG(__VA_ARGS__), {GET_MEMORY_POOL_INFO});

Review Comment:
   Yeah, this seems to create two spans, and immediately closes the first one. I don't think this is what we want.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org