C API aspect can return object with a destructor which is run after the C API call

[rroelke]

My work on CORE-290 intends to use opentelemetry tracing into the core library. When we introduce tracing, we will want to record each C API call as a span under which nested operations occur.

The C API calls can be intercepted via aspect template (#4430). A build process which compiles TileDB core from source can use this aspect to inject code which runs prior to each C API call.

One use of this is in the TileDB-Server repository, which also wants to add opentelemetry tracing to each C API call. However, this implementation is lacking because the aspects do not allow a span to encompass the whole C API call. The instrumentation linked starts and ends the span before invoking the API. It cannot do any better because the aspect call template function return value is not used, so a non-trivial value returned would have its destructor run right away.

This pull request modifies the template invoking the aspect call function to bind its result to a value on the stack. This runs the destructor after returning from the C API call, which will allow the aspect call function to return a tracing span object.

There is no tracing in this pull request, but the added test demonstrates the principle, modifying the existing logging hook test into a "tracing hook" which pushes messages onto a simple log before and after each C API call.

---

TYPE: NO_HISTORY
DESC: Enable C API aspect templates to return a non-trivial

[davisp]

+1

[teo-tsirpanis]

Add static_assert that WITH_HOOK is either logger or tracer? Or not, it's test code.

[teo-tsirpanis]

Would this work? Users will not have to add quote marks to the define. Or not, it's test code.

Suggested change

	#ifndef WITH_HOOK
	#define WITH_HOOK ""
	#endif
	enum class WhichHook { None, Logger, Tracer };
	constexpr WhichHook get_constexpr_which_hook() {
	constexpr std::string_view view(WITH_HOOK);
	#ifndef WITH_HOOK
	#define WITH_HOOK
	#endif
	enum class WhichHook { None, Logger, Tracer };
	constexpr WhichHook get_constexpr_which_hook() {
	constexpr std::string_view view(#WITH_HOOK);

[teo-tsirpanis]

Seems excessive to compile multiple files twice just to make a static_assert.

[teo-tsirpanis]

On second thought, seems even more excessive to add 200 lines of test code just to check that the destructor of an object gets called at the right place. 🤔

[rroelke]

Seems excessive to compile multiple files twice just to make a static_assert.

It's a bit more than that - the tests in unit_capi_hook.cc switch based on which_hook and these files add the static assertions that it compiled according to expectation

seems even more excessive to add 200 lines of test code just to check that the destructor of an object gets called at the right place

I don't agree, for the tracing spans to work properly it is a necessity that the destructors are called in the place we expect. This aspect stuff makes it pretty painful.