Working towards not requiring any pre-#defines before including otel_…

…sdk. Added fail tests in the otel_sdk.cmd, removed -k (skip-errors). Added #include opentelemetry/version.h where it was missing

BUILD

@@ -177,12 +177,15 @@ alias(
 # Present the import library above as cc_library so we can add headers to it, force defines, and make it public.
 [cc_library(
     name = otel_sdk_binary + "_dll",
-    # These have to be propagate to users of the dll library
-    defines = [
-        "OPENTELEMETRY_DLL=1",
-        "OPENTELEMETRY_ABI_VERSION_NO=2",
-        "OPENTELEMETRY_STL_VERSION=2017",
-    ], #1=dllimport, -1=dllexport
+    # There should be no need to define OPENTELEMETRY_DLL=1 in this clone (https://github.com/malkia/opentelemetry-cpp)
+    # By default OPENTELEMETRY_DLL=1 would be defined in macros.h (1=dllimport -1=dllexport 0=static)
+    # Also there OPENTELEMETRY_ABI_VERSION_NO=2 and OPENTELEMETRY_STL_VERSION=2017
+    # Keeping these just in case if something breaks to uncomment them easy
+    # defines = [
+    #     "OPENTELEMETRY_DLL=1",
+    #     "OPENTELEMETRY_ABI_VERSION_NO=2",
+    #     "OPENTELEMETRY_STL_VERSION=2017",
+    # ],
     implementation_deps = [
         otel_sdk_binary + "_import",  # The otel_sdk.dll, .lib and .pdb files
     ],

CMakeLists.txt

@@ -617,6 +617,9 @@ if(DEFINED OPENTELEMETRY_BUILD_DLL)
   add_definitions(-DOPENTELEMETRY_BUILD_EXPORT_DLL)
 endif()
 
+# This definition must be kept to 0, it's only used in the bazel build
+add_definitions(-DOPENTELEMETRY_DLL=0)
+
 include_directories(api/include)
 
 add_subdirectory(api)

api/BUILD

@@ -14,9 +14,9 @@ cc_library(
     name = "api",
     hdrs = glob(["include/**/*.h"]),
     defines = select({
-        ":with_external_abseil": ["HAVE_ABSEIL"],
-        "//:with_dll_enabled": ["OPENTELEMETRY_DLL=-1", "OPENTELEMETRY_ABI_VERSION_NO=2", "OPENTELEMETRY_STL_VERSION=2017"], #1=dllimport, -1=dllexport
-        "//conditions:default": [],
+        ":with_external_abseil": ["HAVE_ABSEIL", "OPENTELEMETRY_DLL=0"],
+        "//:with_dll_enabled": ["OPENTELEMETRY_DLL=-1"], # -1=dllexport, 1=dllimport, 0=static
+        "//conditions:default": ["OPENTELEMETRY_DLL=0"],
     }),
     strip_include_prefix = "include",
     tags = ["api"],

api/include/opentelemetry/common/macros.h

@@ -227,12 +227,26 @@ point.
 
 #endif
 
-// Below is specific to the single-dll clone of OpenTelemetry C++ here:
-// https://github.com/malkia/opentelemetry-cpp 
-#if defined(OPENTELEMETRY_DLL)
+// TODO: Simplfiy below even further. Make it even more succint, but don't let errors escape!
 
-#   define OPENTELEMETRY_DLL_STRX(x) #x
-#   define OPENTELEMETRY_DLL_STR(x) OPENTELEMETRY_DLL_STRX(x)
+// What follows below is strictly for the otel_sdk.dll Windows version compiled with bazel using MSVC from the https://github.com/malkia/opentelemetry-cpp clone
+// In the CMake scripts, the OPENTELEMETRY_BUILD_IMPORT_DLL and OPENTELEMETRY_BUILD_EXPORT_DLL are used (above). We ignore them, and redefine OPENTELEMETRY_EXPORT below.
+
+// Users of the otel_sdk.dll library must be able to #include <opentelemetry/...> with ease, avoiding any extra #defines.
+// Certain defaults would have to be set for them, like OPENTELEMETRY_DLL=1 (dllimport), the STL/ABI version and others in the future.
+// The only requirement for MSVC is to enable C++17 or later.
+
+#if !defined(OPENTELEMETRY_DLL)
+#define OPENTELEMETRY_DLL 1 // dllimport default
+#define OPENTELEMETRY_STL_VERSION 2017
+#define OPENTELEMETRY_ABI_VERSION_NO 2
+#endif
+
+#define OPENTELEMETRY_DLL_STRX(x) #x
+#define OPENTELEMETRY_DLL_STR(x) OPENTELEMETRY_DLL_STRX(x)
+
+#if defined(OPENTELEMETRY_DLL) 
+#if OPENTELEMETRY_DLL != 0 // OPENTELEMETRY_DLL=0 is defined during the static bazel build. This probably breaks CMake one.
 
 #   if !defined(_MSC_VER)
 #      error OPENTELEMETRY_DLL: Only MSVC compiler is supported.
@@ -263,14 +277,24 @@ point.
 #   if OPENTELEMETRY_DLL==1
 #      define OPENTELEMETRY_EXPORT __declspec(dllimport)
 #   elif OPENTELEMETRY_DLL==-1 // Only used during build
+#      undef OPENTELEMETRY_DLL
+#      define OPENTELEMETRY_DLL 1 // this is for the detect_mismatch down below
 #      define OPENTELEMETRY_EXPORT __declspec(dllexport)
 #   else
 #      error OPENTELEMETRY_DLL: OPENTELEMETRY_DLL must be 1 before including opentelemetry header files
 #   endif
 
+// The rule is that if there is struct/class with one or more OPENTELEMETRY_API_SINGLETON function members,
+// then itself can't be defined OPENTELEMETRY_EXPORT 
 #   undef OPENTELEMETRY_API_SINGLETON
 #   define OPENTELEMETRY_API_SINGLETON OPENTELEMETRY_EXPORT
 
-#   pragma detect_mismatch("detect_opentelemetry_dll_mismatch", "stl" OPENTELEMETRY_DLL_STR(OPENTELEMETRY_STL_VERSION) "_abi" OPENTELEMETRY_DLL_STR(OPENTELEMETRY_ABI_VERSION_NO))
-
+#endif // if OPENTELEMETRY_DLL != 0
 #endif // if defined(OPENTELEMETRY_DLL)
+
+#ifdef _MSC_VER
+#  pragma detect_mismatch("otel_sdk_detect_mismatch", "dll" OPENTELEMETRY_DLL_STR(OPENTELEMETRY_DLL) "_stl" OPENTELEMETRY_DLL_STR(OPENTELEMETRY_STL_VERSION) "_abi" OPENTELEMETRY_DLL_STR(OPENTELEMETRY_ABI_VERSION_NO))
+#endif
+
+#undef OPENTELEMETRY_DLL_STRX
+#undef OPENTELEMETRY_DLL_STR

exporters/otlp/test/otlp_grpc_exporter_test.cc

@@ -1,6 +1,8 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#include "opentelemetry/version.h"
+
 #ifndef OPENTELEMETRY_STL_VERSION
 // Unfortunately as of 04/27/2021 the fix is NOT in the vcpkg snapshot of Google Test.
 // Remove above `#ifdef` once the GMock fix for C++20 is in the mainline.

exporters/otlp/test/otlp_grpc_metric_exporter_test.cc

@@ -1,6 +1,8 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#include "opentelemetry/version.h"
+
 #ifndef OPENTELEMETRY_STL_VERSION
 // Unfortunately as of 04/27/2021 the fix is NOT in the vcpkg snapshot of Google Test.
 // Remove above `#ifdef` once the GMock fix for C++20 is in the mainline.

exporters/otlp/test/otlp_http_exporter_test.cc

@@ -1,6 +1,8 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#include "opentelemetry/version.h"
+
 #ifndef OPENTELEMETRY_STL_VERSION
 
 #  include <chrono>

exporters/otlp/test/otlp_http_log_record_exporter_test.cc

@@ -1,6 +1,8 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#include "opentelemetry/version.h"
+
 #ifndef OPENTELEMETRY_STL_VERSION
 
 #  include <chrono>

exporters/zipkin/test/zipkin_exporter_test.cc

@@ -1,6 +1,8 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#include "opentelemetry/version.h"
+
 #ifndef OPENTELEMETRY_STL_VERSION
 
 #  include "opentelemetry/exporters/zipkin/zipkin_exporter.h"

otel_sdk_build.cmd

@@ -1,6 +1,5 @@
 @echo off
 setlocal
-rem I've had these setup on my machine, these should not be present, but wanted to have bit cleaner build
 set BAZEL_SH=
 set BAZEL_VC=
 set BAZEL_VC_FULL_VERSION=
@@ -18,19 +17,21 @@ if "%__BAZEL__%"=="" goto:no-bazel
 set PATH=%__COMSPEC_DIR__%;%__PYTHON_DIR__%
 pushd "%~dp0"
 
-"%__BAZEL__%" build -k --//:with_dll=true ...
+REM singleton_test does not work when linked as static under Windows
+"%__BAZEL__%" test --//:with_dll=false -- ... -//api/test/singleton:singleton_test || goto:error
 
-rem Note that this builds (through the magic of force_debug/release/reldeb) all configurations (unlike tests).
-rem Note that `otel_sdk.zip` is built here for the default fastdbg (e.g. when no -c is specified)
-"%__BAZEL__%" build --//:with_dll=true ... || goto:error
+REM Exclude building otel_sdk_zip right now, do it later. This warms up the tests bellow
+"%__BAZEL__%" build --//:with_dll=true -- ... -otel_sdk_zip || goto:error
 
 rem We can't test dbg, fastbuild and opt at the same time, as done above ^^^ (no config "transition" possible when doing testing (AFAIK))
-"%__BAZEL__%" test -k --//:with_dll=true --test_size_filters=small,medium,large,enormous --test_timeout_filters=short,moderate,long,eternal --test_verbose_timeout_warnings -c dbg ... || goto:error
-"%__BAZEL__%" test -k --//:with_dll=true --test_size_filters=small,medium,large,enormous --test_timeout_filters=short,moderate,long,eternal --test_verbose_timeout_warnings -c fastbuild ... || goto:error
-"%__BAZEL__%" test -k --//:with_dll=true --test_size_filters=small,medium,large,enormous --test_timeout_filters=short,moderate,long,eternal --test_verbose_timeout_warnings -c opt ... || goto:error
+"%__BAZEL__%" test --//:with_dll=true --test_size_filters=small,medium,large,enormous --test_timeout_filters=short,moderate,long,eternal --test_verbose_timeout_warnings -c dbg -- ... -otel_sdk_zip || goto:error
+"%__BAZEL__%" test --//:with_dll=true --test_size_filters=small,medium,large,enormous --test_timeout_filters=short,moderate,long,eternal --test_verbose_timeout_warnings -c fastbuild -- ... -otel_sdk_zip || goto:error
+"%__BAZEL__%" test --//:with_dll=true --test_size_filters=small,medium,large,enormous --test_timeout_filters=short,moderate,long,eternal --test_verbose_timeout_warnings -c opt -- ... -otel_sdk_zip || goto:error
 
-for /F "usebackq" %%i in (`"%__BAZEL__%" info execution_root 2^>nul`) do set __ROOT__=%%i
-for /F "usebackq" %%i in (`"%__BAZEL__%" cquery --//:with_dll^=true otel_sdk_zip --output^=files 2^>nul`) do set __ZIP__=%%i
+"%__BAZEL__%" build --//:with_dll=true otel_sdk_zip || goto:error
+
+for /F "usebackq delims=" %%i in (`"%__BAZEL__%" info execution_root 2^>nul`) do set __ROOT__=%%i
+for /F "usebackq delims=" %%i in (`"%__BAZEL__%" cquery --//:with_dll^=true otel_sdk_zip --output^=files 2^>nul`) do set __ZIP__=%%i
 
 if "%__ZIP__%"=="" goto:broken-build-zip-file
 
@@ -43,20 +44,25 @@ goto:eof
 
 :no-bazel
 echo FAILED: No bazelisk or bazel found!
+exit /b 1
 goto:eof
 
 :no-python
 echo FAILED: No python found!
+exit /b 1
 goto:eof
 
 :no-comspec
 echo FAILED: No ComSpec env var set!
+exit /b 1
 goto:eof
 
 :error
 echo FAILED!
+exit /b 1
 goto:eof
 
 :broken-build-zip-file
 echo The BUILD has broken otel_sdk.zip that can't be build for all compilation_modes
+exit /b 1
 goto:eof