Avoid pointer to reference to temporaries in SetResource API

[maxgolov]

Repro Steps

• Compile with standard library (STL) classes.
• Run all tests: 335 tests passed; 10 tests failed (all related to SetResource API).

@lalitb - this is problematic spot. It won't work too well with temporaries because it's assigning a pointer to reference.

If Resource is compiled with std::variant. And a temporary is passed to API, e.g.

SetResource("key", "value")

It'd break.. Sorry, I found it just now. Haven't noticed during code review originally.. :( Now I tagged the spot in review below:

#706 (comment)_

Exact Problematic Test Spot

Same issue across all 10 tests.

How to Fix

We need to avoid a pointer to temporary here. Since Resource values are not changing often, perhaps a copy is more appropriate than a pointer?

[lalitb]

This is not related to resource sdk implementation, but the result of the implicit conversion of const char * (or any pointer type ) to bool while storing string literal to std::variant as specified in C++17 working draft:
https://timsong-cpp.github.io/cppwp/n4659/conv#bool

Boolean conversions :
A prvalue of arithmetic, unscoped enumeration, pointer, or pointer to member type can be converted to a prvalue of type bool. A zero value, null pointer value, or null member pointer value is converted to false; any other value is converted to true. For direct-initialization, a prvalue of type std​::​nullptr_­t can be converted to a prvalue of type bool; the resulting value is false.

There is a proposal to consider this conversion as narrowing, and so not allowed in C++20 :
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0608r3.html

It seems that our default nostd::variant handles this conversion properly.

So, the problem will come wherever we use OwnedAttributeValue without explicit std::string conversion - right now it is used in ostream-exporter ( through span-data), and resource sdk ( to store properties). The solution as of now is to explicitly convert any string literal into std::string before storing it in OwnedAttributeValue.

I think we need to finalize one variant type support ( #572 ), as supporting multiple variants is going to complicate the implementation.

[maxgolov]

Same issue exists with both recent gcc (Ubuntu 20.04-LTS) and latest Visual Studio 2019 std::variant. It is also there in absl::variant implementation. We can't wait for C++20 update until this feature is released widely, as it would take years for the customers to get to that recent compiler / STL update that would have the fix. I'm proposing a fix that works in #771