Add a backport of std::string_view

[rnburn]

Adds a minimal backport of std::string_view.

Provides a subset of the std::string_view interface. I'm open to porting over additional methods (e.g. the algorithmic member functions) if there's demand for them, but I thought I'd start with something simple.

The PR uses a nostd subdirectory and namespace so that this and other future backports won't clash with opentelemetry's api names.

Note: It uses catch2 so as to have some test coverage, but I'll change if there's consensus to use a different testing framework.

[bogdandrutu]

Do we want to use absl for these kind of stuff instead of copying the code?

[bogdandrutu]

@rnburn don't know if you already made a decision but I think absl has a lot of useful stuff that it may worth using it, and if you are going to use that library, it has a string_view port.

[rnburn]

@bogdandrutu - It certainly does, but we're aiming to have no external dependencies for the API

[bogdandrutu]

You may want to have something like absl which uses std::string_view if available https://github.com/abseil/abseil-cpp/blob/master/absl/strings/string_view.h#L33 this way users of c++17 don't have to manually transform an std::string_view to otel::nostd::string_view.

[rnburn]

@bogdandrutu - Yeah, I was thinking about that. We want to be careful, though. One of our goals is to have a stable ABI to facilitate the dynamic loading of tracer, so we don't want to use data types that can have different layouts for different implementations of the standard c++ library (for example std::string)

That said - I would expect the standard c++ libraries are using the same layout for std::string_view.

We could also provide a preprocessor switch with implicit conversions for std::string_view if the compiler supports it.

[bogdandrutu]

Are all these requirements documented anywhere? I think the dynamic loading of the implementation is a stretch goal, and I would not sacrifice usability to achieve that goal. Also I would be very interested to know how you plan to ship the library (or maybe go with something like absl which forces everyone to build the library).

[rnburn]

@bogdandrutu - We don't have all the requirements documented yet.

But dynamic loading is definitely not a stretch goal. It's already commonly used with the opentracing api (see, for example, https://github.com/opentracing-contrib/nginx-opentracing) and it's essential for making tracing available to projects that can't pull in large external dependencies or don't want to link in multiple tracer implementations.

We some care, we can achieve an API that's provides both ABI stability and usability.

The plan is to make the API and SDK implementations available in multiple forms (e.g. bazel, cmake, etc.)

We're also probably going to provide pre-built binaries for architectures like x86-64 so that you can drop in a tracer implementation without requiring a build.

[bogdandrutu]

But dynamic loading is definitely not a stretch goal. It's already commonly used with the opentracing api (see, for example, https://github.com/opentracing-contrib/nginx-opentracing) and it's essential for making tracing available to projects that can't pull in large external dependencies or don't want to link in multiple tracer implementations.

@rnburn I think one of the solution for the nginx example is to use the standard SDK with a agent/collector binary that exports to the backend of choice, which is easier to achieve and less error prone. I think that this should be our recommended way for projects that ship pre-compiled binaries.

We're also probably going to provide pre-built binaries for architectures like x86-64 so that you can drop in a tracer implementation without requiring a build.

Is this a requirement, I would encourage to not do this to avoid any problems.

[bogdandrutu]

Why do you need this file?

[rnburn]

It might be useful for a test that wants to define its own main, but I removed for now so that there's only catch2_with_main

[bogdandrutu]

I don't see the main function, do I miss anything? If it is based on the macro you can just do it in the build rule to set the macro.

[rnburn]

It's from the macro, yes.

You might have a situation like this

// a_test_helper.cc
#include "opentelemetry/3rd_party/catch.hpp"
... <define some helper functions>

// a_test.cc
#include "a_test_helper.h"
#include "opentelemetry/3rd_party/catch.hpp"
... <define tests>

If the macro definition gets propagated to both source files when they get compiled, you'll have two mains.

[rnburn]

I think one of the solution for the nginx example is to use the standard SDK with a agent/collector binary that exports to the backend of choice, which is easier to achieve and less error prone. I think that this should be our recommended way for projects that ship pre-compiled binaries.

We want this to be usable for vendors that don't want to use a standard tracer implementation.

And if the standard tracer implementation has a large dependency chain with something like grpc, protobuf, etc, many projects won't be able to link to the standard implementation directly because it would bring in too many external dependencies.

The plan is to have the API be minimal, easy to vendor as part of your source tree, and allow you to work with many different tracer providers.

Projects can still, of course, link in full tracer implementations if they prefer.

Is this a requirement, I would encourage to not do this to avoid any problems.

Prebuilt binaries would be something that vendors provide. We haven't decided yet whether to supply them as part of otel-cpp, but that's something we can debate at a later time.

It is definitely possible to supply prebuilt portable binaries provided you set up the linking properly.

[g-easy]

catch2

Can we use googletest like Envoy uses?

[g-easy]

Please clang-format :)

[g-easy]

"bazel test ..." succeeds for me but cmake doesn't:

$ cmake --build .build
Scanning dependencies of target string_view_test
[ 50%] Building CXX object api/test/nostd/CMakeFiles/string_view_test.dir/string_view_test.cc.o
$HOME/src/opentelemetry-cpp/api/test/nostd/string_view_test.cc:1:10: fatal error: opentelemetry/nostd/string_view.h: No such file or directory
 #include "opentelemetry/nostd/string_view.h"

[rnburn]

Which version of cmake do you have? It worked with cmake 3.15 when I tested on os x.

I'm wondering if this way of specifying header-only libraries I used is only supported in newer versions

add_library(opentelemetry_api INTERFACE)
target_include_directories(opentelemetry_api INTERFACE include
  $<INSTALL_INTERFACE:include/opentelemetry>)

[g-easy]

3.13.4, which is what Debian stable ships link

More generally, I was hoping cmake_minimum_required(...) would handle compatibility issues. :)

FYI discussion about cmake requirements

[rnburn]

I clang-formatted.

Could you try testing again with cmake? I think I forgot to add a library to the test executable.

[g-easy]

Getting closer:

[ 50%] Building CXX object api/test/nostd/CMakeFiles/string_view_test.dir/string_view_test.cc.o
[100%] Linking CXX executable string_view_test
/usr/bin/ld: /usr/lib/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/libgtest.a(gtest-all.cc.o): in function `testing::internal::UnitTestImpl::~UnitTestImpl()':
(.text+0x10df1): undefined reference to `pthread_getspecific'

[rnburn]

I added pthreads to the linking

[g-easy]

That fixed it! Thanks Ryan!