You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/20 10:23:42 UTC

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

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


##########
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});
+
+#define START_COMPUTE_SPAN_WITH_PARENT(target_span, parent_span, ...)      \
+  START_SPAN_WITH_PARENT(target_span, parent_span, __VA_ARGS__);           \
+  START_SPAN_WITH_PARENT(target_span, parent_span, FIRST_ARG(__VA_ARGS__), \
+                         {GET_MEMORY_POOL_INFO});
+
+#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);                                                               \
+  }

Review Comment:
   Yes, we shouldn't create the OT scope in a C++ scope here, because the OT scope object controls how long a span is active.



-- 
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