Skip to content

Change the implementation of ResourceAttributes to use common::AttributeValue #817

Description

@maxgolov

Problem Statement

The issue is that presently ResourceAttributes uses common::OwnedAttributeValue here :

using ResourceAttributes =
    std::unordered_map<std::string, opentelemetry::sdk::common::OwnedAttributeValue>;

Now if we build with Abseil Variant or with C++17 std::variant ( #572 ), then it is impossible to properly initialize Resource Attribute with const char *. Because it gets converted to bool instead of being treated as string. In essence, it stores invalid value of the attribute. That is happening because OwnedAttributeValue variant does not define const char * as one of valid values. See the issue described here ( #733 ).

Proposed Solution

As discussed, we would like to avoid adding const char * to OwnedAttributeValue.. That implies now that ResourceAttributes should intake API common::AttributeValue instead in its constructor. And that class should have the const char *:

using AttributeValue =
    nostd::variant<bool,
                   int32_t,
                   int64_t,
                   uint32_t,
                   double,
                   const char *,
                   nostd::string_view,
                   nostd::span<const bool>,
                   nostd::span<const int32_t>,
                   nostd::span<const int64_t>,
                   nostd::span<const uint32_t>,
                   nostd::span<const double>,
                   nostd::span<const nostd::string_view>,
                   // Not currently supported by the specification, but reserved for future use.
                   // Added to provide support for all primitive C++ types.
                   uint64_t,
                   // Not currently supported by the specification, but reserved for future use.
                   // Added to provide support for all primitive C++ types.
                   nostd::span<const uint64_t>,
                   // Not currently supported by the specification, but reserved for future use.
                   // See https://github.com/open-telemetry/opentelemetry-specification/issues/780
                   nostd::span<const uint8_t>>;

Note common::AttributeValue allows const char *. Subsequently the collection of ResourceAttributes internally may still be backed by OwnedAttributeValue. We'd have to apply a transform of incoming ABI/API-safe collection - for each key-value from AttributeValue (that may contain const char *) to OwnedAttributeValue within the implementation of the Resource Attributes collection (that would reassign all non-owning const char * into owning std::string within).

Other Considerations / Future Work

This would also pave a way to potentially expose Resource Attributes either via API. And/or via ABI-stable interface - to be populated by instrumented application / library, by invoking SetResource / SetResources methods from vendor-provided prebuilt OpenTelemetry SDK DLL/Shared instrumentation library. This prebuilt library approach would only be applicable to later versions of SDK, when we get to solve various shared library integration questions in v1.1, i.e. already after v1.0.

@lalitb @pyohannes

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions