importing gsl::span if std::span is not available

[esigo]

Fixes #813 (issue)

Changes

If the WITH_STL cmake option is enabled, the build will use std::span if available (for C++20), and fall back to nostd::span instead of gsl::span (or perhaps fail the build). Uses gsl::span only if explicitly specified the WITH_GSL build option.

For significant contributions please make sure you have completed the following items:

• CHANGELOG.md updated for non-trivial changes
• Unit tests have been added
• Changes in public API reviewed

[codecov]

Codecov Report

Merging #1167 (7350a45) into main (2a821fd) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1167   +/-   ##
=======================================
  Coverage   93.29%   93.29%           
=======================================
  Files         174      174           
  Lines        6404     6404           
=======================================
  Hits         5974     5974           
  Misses        430      430           

Impacted Files	Coverage Δ
...k/instrumentationlibrary/instrumentation_library.h	100.00% <ø> (ø)

[lalitb]

Why is this check required?

[esigo]

because __cpp_lib_span is defined for C++17 on g++-10:g++-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0.
I use these CMake options for build:
-DCMAKE_CXX_COMPILER=g++-10 -DCMAKE_CXX_STANDARD=17

[lalitb]

Which should be ok right? If the HAVE_CPP_STDLIB option is specified, and the standard library provides the implementation ( stable or experimental) it should get selected. What is the behavior now with these changes on g++-10/C++17 - compilation failure as std::span is not available?

[esigo]

The behavior for g++-10/C++17 is that nostd::span will be picked if the WITH_GSL option was not used. I think we shouldn't use std::span if the user explicitly asks for C++17.
Shall I enable std::span if C++17 was used and std::span is available?

[lalitb]

Just to clarify- with this PR, GSL library will only be used if the WITH_GSL flag is enabled?

[esigo]

yes it is enabled only when the user explicitly asks for it.

[lalitb]

Ok, I was thinking about this part in cmake:

opentelemetry-cpp/api/CMakeLists.txt

Lines 48 to 51 in 05d43d6

	if(WITH_STL)
	message("Building with standard library types...")
	target_compile_definitions(opentelemetry_api INTERFACE HAVE_CPP_STDLIB
	HAVE_GSL)

As it looks we are enabling HAVE_GSL macro with WITH_STL option, which would mean try using gsl::span from submodule if STL doesn't have std::span implementation.

[esigo]

that is covered in

opentelemetry-cpp/api/include/opentelemetry/std/span.h

Lines 29 to 31 in bc1b469

	# if !__has_include(<gsl/gsl>)
	# undef HAVE_GSL
	# endif

as the header is not available when the WITH_GSL is off.
We shouldn't define it though, I cleaned it from the CMake file.

[lalitb]

ok, thanks for the clarification.

[lalitb]

LGTM with a minor suggestion.