nostd::shared_ptr cannot be safely casted

[Brandon-Kimberly]

In implementing our design for the metrics API and SDK we ran into a problem regarding nostd::shared_ptr and we are unsure how to proceed from here.

The OpenTelemetry specification states here that functions and components related to the querying and exporting of metric data (such as aggregators, controller, etc.) are to be handled in the SDK; the API merely provides the components and functionality necessary to create metric instruments and capture measurements.

However, the API is also the user facing portion of the code. This means that any time we are passed an argument to a function by the user, that component only has access to functions defined in the API, as well as any virtual functions that may be overriden in derived classes. So it seems we are left with no choice but to explicitly downcast.

One such case where we need to do this is in the controller. When the user creates a controller, they will pass a unique pointer to an instance of the API Meter class. The controller will need to downcast this to an SDK meter to be able to access the Collect() function which checkpoints aggregators and sends metric data to the processor. Essentially, there are functions which we need from each component which we are not allowed to declare in the API and thus must be in the SDK according to the spec.

Possible Solutions

1. Add functionality to the nostd::shared_ptr class to support dynamic_pointer_cast and static_pointer_cast. This would allow us to do the casting that we need in a memory-safe manner. In order to implement this, I believe that a new constructor would need to be added to the nostd::shared_ptr class called the aliasing constructor.
2. Change the OpenTelemetry specification to allow functions and components related to the querying and exporting of metric data to live in the API. This would allow us to add virtual functions to these classes that we can use to collect and export with and eliminates the need for explicit casting. This is a large change to the specification which would require substantial debate outside the C++ repo.
3. Use dynamic_cast instead of dynamic_pointer_cast but this leads to a problem: If we do this then, for at least a brief time, we would have a raw pointer and a smart pointer pointing to the same memory. This is dangerous as it can can easily lead to seg-faults and, as is the case for the meter class, make the code very unstable.

My recommendation would be to use solution 1 given it is the cleanest implementation and most optimal choice. However, if there is another way to solve this issue that we are missing please let us know!

We would greatly appreciate any advice or help on solving this issue as it is currently blocking us from completing a working version of the metrics API and SDK.

cc: @alolita @reyang @maxgolov

[pyohannes]

I had a quick look at the PR link you gave.

One other possible option would be to just obtain the raw pointer from the shared pointer (via the get() method). You can then safely cast and use the raw pointer while you're still holding on to a copy of the nostd::shared_ptr object at the same time.

[Brandon-Kimberly]

@pyohannes Hi Johannes! We're currently trying to implement nostd::dynamic_pointer_cast for nostd::shared_ptr and it seems like we need to access the underlying std::shared_ptr in the shared_ptr_wrapper. We need to access this as we need to pass it to std::shared_ptr's aliasing constructor.

For now, we have added a getter function to the wrapper called GetSharedPtr() like so:

However, I'm sure this is not allowed because of the requirement to be ABI stable.

I want to be able to do r.wrapper().CopyTo(buffer_) but that is not allowed as the nostd::shared_ptrs are pointing to different types (even though one is a derived type of the other).

It seems that the functionality we need may already be implemented in the wrapper class but we're just not quite sure how to use it. Is there any way you could help us out with this?

[Brandon-Kimberly]

Update for those looking at this issue in the future:

We are not looking to add advanced functionality to nostd::shared_ptr if there are possible workarounds to doing so. In our case, we found a workaround that avoids the need to cast nostd::shared_ptrs.