opentelemetry::v1::nostd::string_view is not compatible with std::string_view

[nic-godz]

The problem with the internal opentelemetry::v1::nostd::string_view only being compatible with std::string and not std::string_view is really obvious when using opentelemetry::context::propagation::TextMapCarrier

By using TextMapCarrier as a base class you need to implement "virtual nostd::string_view Get(nostd::string_view key) const noexcept = 0;" for instance. My experience is that C++ HTTP libraries are commonly based on std::string_view in order to avoid unnecessary copying of strings. Boost Beast for instance uses std::string_view for header parameters.

I know that it is possible to copy data/strings from a std::map<std::string_view, std::string_view> into a std::map<std::string, std::string>. But doing that for every request is a bad design. Am I missing something here? Is there a way of using the internal string_view implementation together with the std version? Why not use std::string_view in opentelemetry?

Maybe the TextMapCarrier base class should be a template instead. But I guess it will affect the TextMapPropagator as well then.

What's your take on this issue? I doubt other third party libraries will implement support for opentelemetry::v1::nostd::string_view.

[nic-godz]

I created a specific Boost Beast request carrier:

template <typename T>
class BoostBeastRequestCarrier : public opentelemetry::context::propagation::TextMapCarrier
{
public:
    BoostBeastRequestCarrier(boost::beast::http::request<T>& request) :
    request_(request),
    traceparent_(request_.find(std::string(opentelemetry::trace::propagation::kTraceParent))->value()),
    tracestate_(request_.find(std::string(opentelemetry::trace::propagation::kTraceState))->value())
    {}
    virtual opentelemetry::nostd::string_view Get(opentelemetry::nostd::string_view key) const noexcept override
    {
        if (key == opentelemetry::trace::propagation::kTraceParent)
        {
            return traceparent_;
        }
        else if (key == opentelemetry::trace::propagation::kTraceState)
        {
            return tracestate_;
        }
        return "";
    }
    virtual void Set(opentelemetry::nostd::string_view key, opentelemetry::nostd::string_view value) noexcept override
    {
        request_.set(std::string(key), std::string(value));
        if (key == opentelemetry::trace::propagation::kTraceParent)
        {
            traceparent_ = std::string(value);
        }
        else if (key == opentelemetry::trace::propagation::kTraceState)
        {
            tracestate_ = std::string(value);
        }
    }
    boost::beast::http::request<T>& request_;
    // Needed as temp buffer as opentelemetry::nostd::string_view is not compatible with std::string_view:
    std::string traceparent_;
    std::string tracestate_;
};

Would be nice to avoid all creation of temporary std::string:s. Apart from that, as you can see, really easy to create custom carriers. Clean interface.

[marcalff]

Thanks for the report.

Per file api/include/opentelemetry/nostd/string_view.h, the following are missing and probably could help:

• a nostd::string_view() constructor that takes a std::string_view as input
• a std::string_view operator, to convert nostd::string_view to std::string_view.

The same exist for std::string.

Now, about removing nostd classes entirely and using std instead to avoid making intermediate copies:

• it is desirable, yes
• it is not feasible to fix or change, due to constraints on the C++ ABI for the opentelemetry library.

[github-actions[bot]]

This issue was marked as stale due to lack of activity.

[nic-godz]

Any updates on this issue in terms of supporting std::string_view in the nostd::string_view ctor?

[github-actions[bot]]

This issue was marked as stale due to lack of activity.