lidavidm commented on a change in pull request #12408:
URL: https://github.com/apache/arrow/pull/12408#discussion_r805186057
##########
File path: cpp/src/arrow/util/tracing_internal.h
##########
@@ -101,6 +101,17 @@ AsyncGenerator<T> WrapAsyncGenerator(AsyncGenerator<T>
wrapped,
};
}
+class OtHandle {
+ public:
+
OtHandle(opentelemetry::nostd::shared_ptr<opentelemetry::context::RuntimeContextStorage>
Review comment:
Should this be a private constructor?
##########
File path: cpp/src/arrow/util/tracing_internal.cc
##########
@@ -176,8 +176,23 @@ otel::trace::TracerProvider* GetTracerProvider() {
static nostd::shared_ptr<otel::trace::TracerProvider> provider =
InitializeTracing();
return provider.get();
}
+
+struct StorageSingleton {
+ StorageSingleton() : storage_(otel::context::GetDefaultStorage()) {
+ otel::context::RuntimeContext::SetRuntimeContextStorage(storage_);
Review comment:
Right, this should generally be left to the application. For us, I
suppose that's technically each individual test; that may get tedious, though.
##########
File path: cpp/src/arrow/util/tracing_internal.cc
##########
@@ -176,8 +176,23 @@ otel::trace::TracerProvider* GetTracerProvider() {
static nostd::shared_ptr<otel::trace::TracerProvider> provider =
InitializeTracing();
return provider.get();
}
+
+struct StorageSingleton {
+ StorageSingleton() : storage_(otel::context::GetDefaultStorage()) {
+ otel::context::RuntimeContext::SetRuntimeContextStorage(storage_);
Review comment:
Maybe we should file an upstream issue? If we could access the upstream
shared_ptr, we could keep references to it ourselves and be independent of what
the application wants to do:
https://github.com/open-telemetry/opentelemetry-cpp/blob/3a3bf25289901079534b1cabe14e9c4fb3b35968/api/include/opentelemetry/context/runtime_context.h#L154
A brief reproduction using a thread and TSAN might be enough to convince
them.
##########
File path: cpp/src/arrow/util/tracing_internal.h
##########
@@ -101,6 +101,17 @@ AsyncGenerator<T> WrapAsyncGenerator(AsyncGenerator<T>
wrapped,
};
}
+class OtHandle {
Review comment:
Can we add a brief docstring for the class?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
