From 5871e9297fc8286f0003b599116c670ca93aeda4 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 3 Oct 2022 16:45:02 +0200 Subject: [PATCH 01/25] Fixes #1648 Add a MAINTAINER_MODE in CMake. Build with gcc and clang in maintainer mode in github CI. --- .github/workflows/ci.yml | 32 +++++++++++++++++++++++++++ CMakeLists.txt | 48 ++++++++++++++++++++++++++++++++++++++++ ci/README.md | 1 + ci/do_ci.sh | 16 ++++++++++++++ 4 files changed, 97 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3a1c419334..6fa272b195 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,6 +23,38 @@ jobs: sudo ./ci/setup_thrift.sh ./ci/do_ci.sh cmake.test + cmake_gcc_maintainer_test: + name: CMake gcc (maintainer mode) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + submodules: 'recursive' + - name: setup + run: | + sudo ./ci/setup_cmake.sh + sudo ./ci/setup_ci_environment.sh + - name: run cmake gcc (maintainer mode) + run: | + sudo ./ci/setup_thrift.sh + ./ci/do_ci.sh cmake.maintainer.test + + cmake_clang_maintainer_test: + name: CMake clang (maintainer mode) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + submodules: 'recursive' + - name: setup + run: | + sudo ./ci/setup_cmake.sh + sudo ./ci/setup_ci_environment.sh + - name: run cmake clang (maintainer mode) + run: | + sudo ./ci/setup_thrift.sh + CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/do_ci.sh cmake.maintainer.test + cmake_deprecated_metrics_test: name: CMake test (without otlp-exporter and with deprecated metrics) runs-on: ubuntu-latest diff --git a/CMakeLists.txt b/CMakeLists.txt index a1b6934003..cf5337db0a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -163,6 +163,54 @@ option(BUILD_TESTING "Whether to enable tests" ON) option(BUILD_W3CTRACECONTEXT_TEST "Whether to build w3c trace context" OFF) +option(MAINTAINER_MODE "Build in maintainer mode (-Wall -Werror)" OFF) + +if(MAINTAINER_MODE) + + add_compile_options(-Wall) + add_compile_options(-Werror) + add_compile_options(-Wextra) + + if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") + + # Tested with GCC 9.4, + # compiling options most likely are available + # on older compilers as well. + if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.4) + # Relaxed warnings + + # Enforced warnings + endif() + + endif() + + if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + + # Tested with CLang 12.0.1, + # compiling options most likely are available + # on older compilers as well. + # FIXME: figure out which version is used in github ci. + if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 1.0) + # Relaxed warnings + add_compile_options(-Wno-error=unused-private-field) + + # Relaxed warning, for GRPC generated code. + add_compile_options(-Wno-error=shadow-field) + + # Enforced warnings + add_compile_options(-Wconditional-uninitialized) + add_compile_options(-Wextra-semi) + add_compile_options(-Wheader-hygiene) + add_compile_options(-Wnon-virtual-dtor) + add_compile_options(-Wundefined-reinterpret-cast) + add_compile_options(-Wrange-loop-analysis) + add_compile_options(-Winconsistent-missing-destructor-override) + add_compile_options(-Winconsistent-missing-override) + add_compile_options(-Wnewline-eof) + endif() + endif() +endif(MAINTAINER_MODE) + if(WIN32) if(BUILD_TESTING) if(MSVC) diff --git a/ci/README.md b/ci/README.md index 14a6107eb8..65bea032d7 100644 --- a/ci/README.md +++ b/ci/README.md @@ -4,6 +4,7 @@ CI tests can be run on docker by invoking the script `./ci/run_docker.sh ./ci/do_ci.sh {TARGET}` where the targets are: * `cmake.test`: build cmake targets and run tests. +* `cmake.maintainer.test`: build with cmake and test, in maintainer mode. * `cmake.legacy.test`: build cmake targets with gcc 4.8 and run tests. * `cmake.c++20.test`: build cmake targets with the C++20 standard and run tests. * `cmake.test_example_plugin`: build and test an example OpenTelemetry plugin. diff --git a/ci/do_ci.sh b/ci/do_ci.sh index bf775fa564..8c4e483237 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -89,6 +89,22 @@ if [[ "$1" == "cmake.test" ]]; then make make test exit 0 +elif [[ "$1" == "cmake.maintainer.test" ]]; then + cd "${BUILD_DIR}" + rm -rf * + cmake -DCMAKE_BUILD_TYPE=Debug \ + -DWITH_PROMETHEUS=ON \ + -DWITH_ZIPKIN=ON \ + -DWITH_JAEGER=ON \ + -DWITH_ELASTICSEARCH=ON \ + -DWITH_LOGS_PREVIEW=ON \ + -DWITH_METRICS_PREVIEW=OFF \ + -DWITH_ASYNC_EXPORT_PREVIEW=ON \ + -DMAINTAINER_MODE=ON \ + "${SRC_DIR}" + make + make test + exit 0 elif [[ "$1" == "cmake.with_async_export.test" ]]; then cd "${BUILD_DIR}" rm -rf * From 7ec5536887cc1ef3ad9675eeadc3c250125f2ce8 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 3 Oct 2022 17:09:48 +0200 Subject: [PATCH 02/25] Format, versions used in github. --- .github/workflows/ci.yml | 14 +++++++------- CMakeLists.txt | 12 +++--------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6fa272b195..9247a310cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,12 +32,12 @@ jobs: submodules: 'recursive' - name: setup run: | - sudo ./ci/setup_cmake.sh - sudo ./ci/setup_ci_environment.sh + sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_cmake.sh + sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_ci_environment.sh - name: run cmake gcc (maintainer mode) run: | - sudo ./ci/setup_thrift.sh - ./ci/do_ci.sh cmake.maintainer.test + sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_thrift.sh + CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/do_ci.sh cmake.maintainer.test cmake_clang_maintainer_test: name: CMake clang (maintainer mode) @@ -48,11 +48,11 @@ jobs: submodules: 'recursive' - name: setup run: | - sudo ./ci/setup_cmake.sh - sudo ./ci/setup_ci_environment.sh + sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_cmake.sh + sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_ci_environment.sh - name: run cmake clang (maintainer mode) run: | - sudo ./ci/setup_thrift.sh + sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_thrift.sh CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/do_ci.sh cmake.maintainer.test cmake_deprecated_metrics_test: diff --git a/CMakeLists.txt b/CMakeLists.txt index cf5337db0a..b3e5612a58 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -166,16 +166,13 @@ option(BUILD_W3CTRACECONTEXT_TEST "Whether to build w3c trace context" OFF) option(MAINTAINER_MODE "Build in maintainer mode (-Wall -Werror)" OFF) if(MAINTAINER_MODE) - add_compile_options(-Wall) add_compile_options(-Werror) add_compile_options(-Wextra) if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") - # Tested with GCC 9.4, - # compiling options most likely are available - # on older compilers as well. + # Tested with GCC 9.4 on github. if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.4) # Relaxed warnings @@ -186,11 +183,8 @@ if(MAINTAINER_MODE) if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") - # Tested with CLang 12.0.1, - # compiling options most likely are available - # on older compilers as well. - # FIXME: figure out which version is used in github ci. - if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 1.0) + # Tested with CLang 11.0 on github. + if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 11.0) # Relaxed warnings add_compile_options(-Wno-error=unused-private-field) From 37f0d2c1320cc51bcc0386384ab22a4d453d93e9 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 3 Oct 2022 17:32:43 +0200 Subject: [PATCH 03/25] Fixed format. --- CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b3e5612a58..8a5b0ba07a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -173,7 +173,7 @@ if(MAINTAINER_MODE) if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") # Tested with GCC 9.4 on github. - if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.4) + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.4) # Relaxed warnings # Enforced warnings @@ -183,8 +183,8 @@ if(MAINTAINER_MODE) if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") - # Tested with CLang 11.0 on github. - if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 11.0) + # Tested with Clang 11.0 on github. + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 11.0) # Relaxed warnings add_compile_options(-Wno-error=unused-private-field) From 9ad340cab869d5d4ebcdcc0bd99479d7c10623e0 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 3 Oct 2022 18:41:01 +0200 Subject: [PATCH 04/25] Added MSVC MAINTAINER_MODE --- CMakeLists.txt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8a5b0ba07a..2cdd0a0f64 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -166,11 +166,10 @@ option(BUILD_W3CTRACECONTEXT_TEST "Whether to build w3c trace context" OFF) option(MAINTAINER_MODE "Build in maintainer mode (-Wall -Werror)" OFF) if(MAINTAINER_MODE) - add_compile_options(-Wall) - add_compile_options(-Werror) - add_compile_options(-Wextra) - if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") + add_compile_options(-Wall) + add_compile_options(-Werror) + add_compile_options(-Wextra) # Tested with GCC 9.4 on github. if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.4) @@ -182,6 +181,9 @@ if(MAINTAINER_MODE) endif() if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options(-Wall) + add_compile_options(-Werror) + add_compile_options(-Wextra) # Tested with Clang 11.0 on github. if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 11.0) @@ -203,6 +205,11 @@ if(MAINTAINER_MODE) add_compile_options(-Wnewline-eof) endif() endif() + + if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") + add_compile_options(/WX) + add_compile_options(/W4) + endif() endif(MAINTAINER_MODE) if(WIN32) From 52e47dfa9e872097bbf2a3429323a9c457d75684 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 3 Oct 2022 18:53:29 +0200 Subject: [PATCH 05/25] Add a MSVC build in maintainer mode, in CI. --- .github/workflows/ci.yml | 14 ++++++++++++++ ci/do_ci.ps1 | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9247a310cf..8123323ec5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,6 +55,20 @@ jobs: sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_thrift.sh CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/do_ci.sh cmake.maintainer.test + cmake_msvc_maintainer_test: + name: CMake msvc (maintainer mode) + runs-on: windows-2019 + steps: + - uses: actions/checkout@v3 + with: + submodules: 'recursive' + - name: setup + run: | + ./ci/setup_windows_cmake.ps1 + ./ci/setup_windows_ci_environment.ps1 + - name: run tests + run: ./ci/do_ci.ps1 cmake.maintainer.test + cmake_deprecated_metrics_test: name: CMake test (without otlp-exporter and with deprecated metrics) runs-on: ubuntu-latest diff --git a/ci/do_ci.ps1 b/ci/do_ci.ps1 index 88e7becf90..b371c505e9 100644 --- a/ci/do_ci.ps1 +++ b/ci/do_ci.ps1 @@ -48,6 +48,27 @@ switch ($action) { exit $exit } } + "cmake.maintainer.test" { + cd "$BUILD_DIR" + cmake $SRC_DIR ` + -DMAINTAINER_MODE=ON ` + -DVCPKG_TARGET_TRIPLET=x64-windows ` + "-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake" + $exit = $LASTEXITCODE + if ($exit -ne 0) { + exit $exit + } + cmake --build . + $exit = $LASTEXITCODE + if ($exit -ne 0) { + exit $exit + } + ctest -C Debug + $exit = $LASTEXITCODE + if ($exit -ne 0) { + exit $exit + } + } "cmake.with_async_export.test" { cd "$BUILD_DIR" cmake $SRC_DIR ` From d900a427cbec032a62e80a621be2df9712000715 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 3 Oct 2022 22:41:34 +0200 Subject: [PATCH 06/25] Fix review comments. --- CMakeLists.txt | 42 ++++++++++++++++++++++++++++++++++-------- ci/do_ci.sh | 2 +- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2cdd0a0f64..83d3170302 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -167,26 +167,41 @@ option(MAINTAINER_MODE "Build in maintainer mode (-Wall -Werror)" OFF) if(MAINTAINER_MODE) if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") + message("Building with gcc in maintainer mode.") + add_compile_options(-Wall) add_compile_options(-Werror) add_compile_options(-Wextra) # Tested with GCC 9.4 on github. if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.4) + message("Building with additional warnings for gcc.") + # Relaxed warnings # Enforced warnings + add_compile_options(-Wcast-qual) + add_compile_options(-Wextra-semi) + add_compile_options(-Wformat-security) + add_compile_options(-Wlogical-op) + add_compile_options(-Wmissing-include-dirs) + add_compile_options(-Woverloaded-virtual) + add_compile_options(-Wstringop-truncation) + add_compile_options(-Wsuggest-override) + add_compile_options(-Wundef) + add_compile_options(-Wvla) endif() + elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + message("Building with clang in maintainer mode.") - endif() - - if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") add_compile_options(-Wall) add_compile_options(-Werror) add_compile_options(-Wextra) # Tested with Clang 11.0 on github. if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 11.0) + message("Building with additional warnings for clang.") + # Relaxed warnings add_compile_options(-Wno-error=unused-private-field) @@ -194,21 +209,32 @@ if(MAINTAINER_MODE) add_compile_options(-Wno-error=shadow-field) # Enforced warnings + add_compile_options(-Wcast-qual) add_compile_options(-Wconditional-uninitialized) add_compile_options(-Wextra-semi) + add_compile_options(-Wformat-security) add_compile_options(-Wheader-hygiene) - add_compile_options(-Wnon-virtual-dtor) - add_compile_options(-Wundefined-reinterpret-cast) - add_compile_options(-Wrange-loop-analysis) add_compile_options(-Winconsistent-missing-destructor-override) add_compile_options(-Winconsistent-missing-override) add_compile_options(-Wnewline-eof) + add_compile_options(-Wnon-virtual-dtor) + add_compile_options(-Woverloaded-virtual) + add_compile_options(-Wrange-loop-analysis) + add_compile_options(-Wundef) + add_compile_options(-Wundefined-reinterpret-cast) + add_compile_options(-Wvla) endif() - endif() + elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") + message("Building with msvc in maintainer mode.") - if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") add_compile_options(/WX) add_compile_options(/W4) + + # Relaxed warnings + + # Enforced warnings + elseif() + message(FATAL_ERROR "Building with unknown compiler in maintainer mode.") endif() endif(MAINTAINER_MODE) diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 8c4e483237..c0250bef5b 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -102,7 +102,7 @@ elif [[ "$1" == "cmake.maintainer.test" ]]; then -DWITH_ASYNC_EXPORT_PREVIEW=ON \ -DMAINTAINER_MODE=ON \ "${SRC_DIR}" - make + make -k make test exit 0 elif [[ "$1" == "cmake.with_async_export.test" ]]; then From 3c562b9e75846a54502b332d147c8bc90806c4a3 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 4 Oct 2022 11:56:30 +0200 Subject: [PATCH 07/25] Fix review comments. Moved grpc work around to protobuf_include_prefix.h Disabled some warnings for windows. Also, used windows-latest (2022) --- .github/workflows/ci.yml | 2 +- CMakeLists.txt | 8 +++++--- .../exporters/otlp/protobuf_include_prefix.h | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8123323ec5..e1d22fda31 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,7 +57,7 @@ jobs: cmake_msvc_maintainer_test: name: CMake msvc (maintainer mode) - runs-on: windows-2019 + runs-on: windows-latest steps: - uses: actions/checkout@v3 with: diff --git a/CMakeLists.txt b/CMakeLists.txt index 83d3170302..acc10dd3cf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -205,9 +205,6 @@ if(MAINTAINER_MODE) # Relaxed warnings add_compile_options(-Wno-error=unused-private-field) - # Relaxed warning, for GRPC generated code. - add_compile_options(-Wno-error=shadow-field) - # Enforced warnings add_compile_options(-Wcast-qual) add_compile_options(-Wconditional-uninitialized) @@ -231,6 +228,11 @@ if(MAINTAINER_MODE) add_compile_options(/W4) # Relaxed warnings + add_compile_options(/wd4100) + add_compile_options(/wd4125) + add_compile_options(/wd4566) + add_compile_options(/wd4127) + add_compile_options(/wd4512) # Enforced warnings elseif() diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/protobuf_include_prefix.h b/exporters/otlp/include/opentelemetry/exporters/otlp/protobuf_include_prefix.h index ae103b0173..036cb0ae7f 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/protobuf_include_prefix.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/protobuf_include_prefix.h @@ -34,4 +34,5 @@ # pragma clang diagnostic push # pragma clang diagnostic ignored "-Wunused-parameter" # pragma clang diagnostic ignored "-Wtype-limits" -#endif \ No newline at end of file +# pragma clang diagnostic ignored "-Wshadow-field" +#endif From 4c412a4cfcb402341289e62b786c0d469900e880 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 5 Oct 2022 15:00:14 +0200 Subject: [PATCH 08/25] Renamed to OTELCPP_MAINTAINER_MODE, to avoid collisions with other code. --- CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index acc10dd3cf..ace06c5f14 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -163,9 +163,9 @@ option(BUILD_TESTING "Whether to enable tests" ON) option(BUILD_W3CTRACECONTEXT_TEST "Whether to build w3c trace context" OFF) -option(MAINTAINER_MODE "Build in maintainer mode (-Wall -Werror)" OFF) +option(OTELCPP_MAINTAINER_MODE "Build in maintainer mode (-Wall -Werror)" OFF) -if(MAINTAINER_MODE) +if(OTELCPP_MAINTAINER_MODE) if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") message("Building with gcc in maintainer mode.") @@ -238,7 +238,7 @@ if(MAINTAINER_MODE) elseif() message(FATAL_ERROR "Building with unknown compiler in maintainer mode.") endif() -endif(MAINTAINER_MODE) +endif(OTELCPP_MAINTAINER_MODE) if(WIN32) if(BUILD_TESTING) From ea6e74f1737f5111c44d47f23d4dc455dc48e506 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 5 Oct 2022 21:20:34 +0200 Subject: [PATCH 09/25] Upgrade CI to gcc 11, clang 14. --- .github/workflows/ci.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e1d22fda31..d0f0761b70 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,12 +32,12 @@ jobs: submodules: 'recursive' - name: setup run: | - sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_cmake.sh - sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_ci_environment.sh + sudo CC=/usr/bin/gcc-11 CXX=/usr/bin/g++-11 ./ci/setup_cmake.sh + sudo CC=/usr/bin/gcc-11 CXX=/usr/bin/g++-11 ./ci/setup_ci_environment.sh - name: run cmake gcc (maintainer mode) run: | - sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_thrift.sh - CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/do_ci.sh cmake.maintainer.test + sudo CC=/usr/bin/gcc-11 CXX=/usr/bin/g++-11 ./ci/setup_thrift.sh + CC=/usr/bin/gcc-11 CXX=/usr/bin/g++-11 ./ci/do_ci.sh cmake.maintainer.test cmake_clang_maintainer_test: name: CMake clang (maintainer mode) @@ -48,12 +48,12 @@ jobs: submodules: 'recursive' - name: setup run: | - sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_cmake.sh - sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_ci_environment.sh + sudo CC=/usr/bin/clang-14 CXX=/usr/bin/clang++-14 ./ci/setup_cmake.sh + sudo CC=/usr/bin/clang-14 CXX=/usr/bin/clang++-14 ./ci/setup_ci_environment.sh - name: run cmake clang (maintainer mode) run: | - sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_thrift.sh - CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/do_ci.sh cmake.maintainer.test + sudo CC=/usr/bin/clang-14 CXX=/usr/bin/clang++-14 ./ci/setup_thrift.sh + CC=/usr/bin/clang-14 CXX=/usr/bin/clang++-14 ./ci/do_ci.sh cmake.maintainer.test cmake_msvc_maintainer_test: name: CMake msvc (maintainer mode) From 57e8919788a2904a4b4098ed0b126381db02ce1b Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 5 Oct 2022 21:28:53 +0200 Subject: [PATCH 10/25] Revert: Upgrade CI to gcc 11, clang 14. Does not exist on the runner image, contrary to doc. --- .github/workflows/ci.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d0f0761b70..e1d22fda31 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,12 +32,12 @@ jobs: submodules: 'recursive' - name: setup run: | - sudo CC=/usr/bin/gcc-11 CXX=/usr/bin/g++-11 ./ci/setup_cmake.sh - sudo CC=/usr/bin/gcc-11 CXX=/usr/bin/g++-11 ./ci/setup_ci_environment.sh + sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_cmake.sh + sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_ci_environment.sh - name: run cmake gcc (maintainer mode) run: | - sudo CC=/usr/bin/gcc-11 CXX=/usr/bin/g++-11 ./ci/setup_thrift.sh - CC=/usr/bin/gcc-11 CXX=/usr/bin/g++-11 ./ci/do_ci.sh cmake.maintainer.test + sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_thrift.sh + CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/do_ci.sh cmake.maintainer.test cmake_clang_maintainer_test: name: CMake clang (maintainer mode) @@ -48,12 +48,12 @@ jobs: submodules: 'recursive' - name: setup run: | - sudo CC=/usr/bin/clang-14 CXX=/usr/bin/clang++-14 ./ci/setup_cmake.sh - sudo CC=/usr/bin/clang-14 CXX=/usr/bin/clang++-14 ./ci/setup_ci_environment.sh + sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_cmake.sh + sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_ci_environment.sh - name: run cmake clang (maintainer mode) run: | - sudo CC=/usr/bin/clang-14 CXX=/usr/bin/clang++-14 ./ci/setup_thrift.sh - CC=/usr/bin/clang-14 CXX=/usr/bin/clang++-14 ./ci/do_ci.sh cmake.maintainer.test + sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_thrift.sh + CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/do_ci.sh cmake.maintainer.test cmake_msvc_maintainer_test: name: CMake msvc (maintainer mode) From ee7f15ef0be6e578de05a4b8348b3120d51fb65a Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 5 Oct 2022 21:54:14 +0200 Subject: [PATCH 11/25] Upgrade CI to gcc 10, clang 12. --- .github/workflows/ci.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e1d22fda31..fb5eb88fca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,12 +32,12 @@ jobs: submodules: 'recursive' - name: setup run: | - sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_cmake.sh - sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_ci_environment.sh + sudo CC=/usr/bin/gcc-10 CXX=/usr/bin/g++-10 ./ci/setup_cmake.sh + sudo CC=/usr/bin/gcc-10 CXX=/usr/bin/g++-10 ./ci/setup_ci_environment.sh - name: run cmake gcc (maintainer mode) run: | - sudo CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/setup_thrift.sh - CC=/usr/bin/gcc CXX=/usr/bin/g++ ./ci/do_ci.sh cmake.maintainer.test + sudo CC=/usr/bin/gcc-10 CXX=/usr/bin/g++-10 ./ci/setup_thrift.sh + CC=/usr/bin/gcc-10 CXX=/usr/bin/g++-10 ./ci/do_ci.sh cmake.maintainer.test cmake_clang_maintainer_test: name: CMake clang (maintainer mode) @@ -48,12 +48,12 @@ jobs: submodules: 'recursive' - name: setup run: | - sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_cmake.sh - sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_ci_environment.sh + sudo CC=/usr/bin/clang-12 CXX=/usr/bin/clang++-12 ./ci/setup_cmake.sh + sudo CC=/usr/bin/clang-12 CXX=/usr/bin/clang++-12 ./ci/setup_ci_environment.sh - name: run cmake clang (maintainer mode) run: | - sudo CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/setup_thrift.sh - CC=/usr/bin/clang CXX=/usr/bin/clang++ ./ci/do_ci.sh cmake.maintainer.test + sudo CC=/usr/bin/clang-12 CXX=/usr/bin/clang++-12 ./ci/setup_thrift.sh + CC=/usr/bin/clang-12 CXX=/usr/bin/clang++-14 ./ci/do_ci.sh cmake.maintainer.test cmake_msvc_maintainer_test: name: CMake msvc (maintainer mode) From d1a158a0ac68b0557690a3975860768df536b57e Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 5 Oct 2022 21:59:47 +0200 Subject: [PATCH 12/25] clang fixup --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fb5eb88fca..3fd4be9c11 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -53,7 +53,7 @@ jobs: - name: run cmake clang (maintainer mode) run: | sudo CC=/usr/bin/clang-12 CXX=/usr/bin/clang++-12 ./ci/setup_thrift.sh - CC=/usr/bin/clang-12 CXX=/usr/bin/clang++-14 ./ci/do_ci.sh cmake.maintainer.test + CC=/usr/bin/clang-12 CXX=/usr/bin/clang++-12 ./ci/do_ci.sh cmake.maintainer.test cmake_msvc_maintainer_test: name: CMake msvc (maintainer mode) From c5b95957ee9dbbe4157da54d7ec16b280a826277 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 5 Oct 2022 22:20:55 +0200 Subject: [PATCH 13/25] Enforce OTELCPP_MAINTAINER_MODE --- ci/do_ci.ps1 | 2 +- ci/do_ci.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/do_ci.ps1 b/ci/do_ci.ps1 index b371c505e9..f0d08a6d4d 100644 --- a/ci/do_ci.ps1 +++ b/ci/do_ci.ps1 @@ -51,7 +51,7 @@ switch ($action) { "cmake.maintainer.test" { cd "$BUILD_DIR" cmake $SRC_DIR ` - -DMAINTAINER_MODE=ON ` + -DOTELCPP_MAINTAINER_MODE=ON ` -DVCPKG_TARGET_TRIPLET=x64-windows ` "-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake" $exit = $LASTEXITCODE diff --git a/ci/do_ci.sh b/ci/do_ci.sh index c0250bef5b..c6d638ffe3 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -100,7 +100,7 @@ elif [[ "$1" == "cmake.maintainer.test" ]]; then -DWITH_LOGS_PREVIEW=ON \ -DWITH_METRICS_PREVIEW=OFF \ -DWITH_ASYNC_EXPORT_PREVIEW=ON \ - -DMAINTAINER_MODE=ON \ + -DOTELCPP_MAINTAINER_MODE=ON \ "${SRC_DIR}" make -k make test From 50793c2ad5b1dd08e703498910debcaf94e2cd13 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 5 Oct 2022 23:52:35 +0200 Subject: [PATCH 14/25] Move warning cleanup (work in progress) --- CMakeLists.txt | 16 ++++++++++------ .../internal/absl/types/bad_variant_access.h | 2 +- exporters/elasticsearch/src/es_log_exporter.cc | 12 ++++++------ exporters/jaeger/CMakeLists.txt | 6 ++++++ exporters/otlp/test/otlp_recordable_test.cc | 7 +++++++ exporters/zipkin/test/zipkin_exporter_test.cc | 8 ++++---- .../zipkin/test/zipkin_recordable_test.cc | 6 ++++++ .../exemplar/fixed_size_exemplar_reservoir.h | 18 ++++++++++-------- .../exemplar/histogram_exemplar_reservoir.h | 6 +++--- .../exemplar/with_trace_sample_filter.h | 8 ++++---- 10 files changed, 57 insertions(+), 32 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ace06c5f14..36b6a62630 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -166,7 +166,7 @@ option(BUILD_W3CTRACECONTEXT_TEST "Whether to build w3c trace context" OFF) option(OTELCPP_MAINTAINER_MODE "Build in maintainer mode (-Wall -Werror)" OFF) if(OTELCPP_MAINTAINER_MODE) - if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") message("Building with gcc in maintainer mode.") add_compile_options(-Wall) @@ -180,18 +180,22 @@ if(OTELCPP_MAINTAINER_MODE) # Relaxed warnings # Enforced warnings + + # C++ options only + add_compile_options($<$,CXX>:-Wextra-semi>) + add_compile_options($<$,CXX>:-Woverloaded-virtual>) + add_compile_options($<$,CXX>:-Wsuggest-override>) + + # C and C++ add_compile_options(-Wcast-qual) - add_compile_options(-Wextra-semi) add_compile_options(-Wformat-security) add_compile_options(-Wlogical-op) add_compile_options(-Wmissing-include-dirs) - add_compile_options(-Woverloaded-virtual) add_compile_options(-Wstringop-truncation) - add_compile_options(-Wsuggest-override) add_compile_options(-Wundef) add_compile_options(-Wvla) endif() - elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") message("Building with clang in maintainer mode.") add_compile_options(-Wall) @@ -221,7 +225,7 @@ if(OTELCPP_MAINTAINER_MODE) add_compile_options(-Wundefined-reinterpret-cast) add_compile_options(-Wvla) endif() - elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") message("Building with msvc in maintainer mode.") add_compile_options(/WX) diff --git a/api/include/opentelemetry/nostd/internal/absl/types/bad_variant_access.h b/api/include/opentelemetry/nostd/internal/absl/types/bad_variant_access.h index 9783504d2d..81caa0d75c 100644 --- a/api/include/opentelemetry/nostd/internal/absl/types/bad_variant_access.h +++ b/api/include/opentelemetry/nostd/internal/absl/types/bad_variant_access.h @@ -75,7 +75,7 @@ namespace variant_internal { [[noreturn]] static inline void ThrowBadVariantAccess() { THROW_BAD_VARIANT_ACCESS; -}; +} //[[noreturn]] static inline void Rethrow() //{ // THROW_BAD_VARIANT_ACCESS; // Unused! diff --git a/exporters/elasticsearch/src/es_log_exporter.cc b/exporters/elasticsearch/src/es_log_exporter.cc index 618f6d6b30..705ae3ba01 100644 --- a/exporters/elasticsearch/src/es_log_exporter.cc +++ b/exporters/elasticsearch/src/es_log_exporter.cc @@ -107,7 +107,7 @@ class ResponseHandler : public http_client::EventHandler } // Callback method when an http event occurs - void OnEvent(http_client::SessionState state, nostd::string_view reason) noexcept override + void OnEvent(http_client::SessionState state, nostd::string_view /* reason */) noexcept override { // If any failure event occurs, release the condition variable to unblock main thread switch (state) @@ -160,9 +160,9 @@ class AsyncResponseHandler : public http_client::EventHandler std::shared_ptr session, std::function &&result_callback, bool console_debug = false) - : console_debug_{console_debug}, - session_{std::move(session)}, - result_callback_{std::move(result_callback)} + : session_{std::move(session)}, + result_callback_{std::move(result_callback)}, + console_debug_{console_debug} {} /** @@ -197,7 +197,7 @@ class AsyncResponseHandler : public http_client::EventHandler } // Callback method when an http event occurs - void OnEvent(http_client::SessionState state, nostd::string_view reason) noexcept override + void OnEvent(http_client::SessionState state, nostd::string_view /* reason */) noexcept override { bool need_stop = false; switch (state) @@ -364,7 +364,7 @@ sdk::common::ExportResult ElasticsearchLogExporter::Export( # endif } -bool ElasticsearchLogExporter::Shutdown(std::chrono::microseconds timeout) noexcept +bool ElasticsearchLogExporter::Shutdown(std::chrono::microseconds /* timeout */) noexcept { const std::lock_guard locked(lock_); is_shutdown_ = true; diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index 017b43d494..b0ab36f0cf 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -9,6 +9,12 @@ set(JAEGER_THRIFT_GENCPP_SOURCES thrift-gen/Agent.cpp thrift-gen/jaeger_types.cpp thrift-gen/Collector.cpp thrift-gen/zipkincore_types.cpp) +if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # THRIFT generated code is not warning clean for gcc. + set_source_files_properties( + JAEGER_THRIFT_GENCPP_SOURCES PROPERTIES COMPILE_OPTIONS "-Wno-suggest-override") +endif() + set(JAEGER_EXPORTER_SOURCES src/jaeger_exporter.cc src/jaeger_exporter_factory.cc diff --git a/exporters/otlp/test/otlp_recordable_test.cc b/exporters/otlp/test/otlp_recordable_test.cc index 03e6a37b99..abad2991b7 100644 --- a/exporters/otlp/test/otlp_recordable_test.cc +++ b/exporters/otlp/test/otlp_recordable_test.cc @@ -2,6 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 #include "opentelemetry/exporters/otlp/otlp_recordable.h" + +#if defined(__GNUC__) +// GCC raises -Wsuggest-override warnings on GTest, +// in code related to TYPED_TEST() . +# pragma GCC diagnostic ignored "-Wsuggest-override" +#endif + #include OPENTELEMETRY_BEGIN_NAMESPACE diff --git a/exporters/zipkin/test/zipkin_exporter_test.cc b/exporters/zipkin/test/zipkin_exporter_test.cc index eec71f43d6..af518d1655 100644 --- a/exporters/zipkin/test/zipkin_exporter_test.cc +++ b/exporters/zipkin/test/zipkin_exporter_test.cc @@ -148,9 +148,9 @@ TEST_F(ZipkinExporterTestPeer, ExportJsonIntegrationTest) auto expected_url = nostd::string_view{"http://localhost:9411/api/v2/spans"}; EXPECT_CALL(*mock_http_client, Post(expected_url, IsValidMessage(report_trace_id), _)) .Times(Exactly(1)) - .WillOnce(Return(ByMove(std::move(ext::http::client::Result{ + .WillOnce(Return(ByMove(ext::http::client::Result{ std::unique_ptr{new ext::http::client::curl::Response()}, - ext::http::client::SessionState::Response})))); + ext::http::client::SessionState::Response}))); child_span->End(); parent_span->End(); @@ -172,9 +172,9 @@ TEST_F(ZipkinExporterTestPeer, ShutdownTest) nostd::span> batch_1(&recordable_1, 1); EXPECT_CALL(*mock_http_client, Post(_, _, _)) .Times(Exactly(1)) - .WillOnce(Return(ByMove(std::move(ext::http::client::Result{ + .WillOnce(Return(ByMove(ext::http::client::Result{ std::unique_ptr{new ext::http::client::curl::Response()}, - ext::http::client::SessionState::Response})))); + ext::http::client::SessionState::Response}))); auto result = exporter->Export(batch_1); EXPECT_EQ(sdk_common::ExportResult::kSuccess, result); diff --git a/exporters/zipkin/test/zipkin_recordable_test.cc b/exporters/zipkin/test/zipkin_recordable_test.cc index 5da618d5b2..630ca332f0 100644 --- a/exporters/zipkin/test/zipkin_recordable_test.cc +++ b/exporters/zipkin/test/zipkin_recordable_test.cc @@ -12,6 +12,12 @@ #include "opentelemetry/common/timestamp.h" #include "opentelemetry/exporters/zipkin/recordable.h" +#if defined(__GNUC__) +// GCC raises -Wsuggest-override warnings on GTest, +// in code related to TYPED_TEST() . +# pragma GCC diagnostic ignored "-Wsuggest-override" +#endif + #include namespace trace = opentelemetry::trace; diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h index 1a0abeb6b6..a809f026b0 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h @@ -32,10 +32,11 @@ class FixedSizeExemplarReservoir : public ExemplarReservoir map_and_reset_cell_(map_and_reset_cell) {} - void OfferMeasurement(long value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context, - const opentelemetry::common::SystemTimestamp ×tamp) noexcept override + void OfferMeasurement( + long value, + const MetricAttributes &attributes, + const opentelemetry::context::Context &context, + const opentelemetry::common::SystemTimestamp & /* timestamp */) noexcept override { if (!reservoir_cell_selector_) { @@ -49,10 +50,11 @@ class FixedSizeExemplarReservoir : public ExemplarReservoir } } - void OfferMeasurement(double value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context, - const opentelemetry::common::SystemTimestamp ×tamp) noexcept override + void OfferMeasurement( + double value, + const MetricAttributes &attributes, + const opentelemetry::context::Context &context, + const opentelemetry::common::SystemTimestamp & /* timestamp */) noexcept override { if (!reservoir_cell_selector_) { diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h index 83f410f815..5481327a5a 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h @@ -50,10 +50,10 @@ class HistogramExemplarReservoir : public FixedSizeExemplarReservoir return ReservoirCellIndexFor(cells, (double)value, attributes, context); } - int ReservoirCellIndexFor(const std::vector &cells, + int ReservoirCellIndexFor(const std::vector & /* cells */, double value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context) override + const MetricAttributes & /* attributes */, + const opentelemetry::context::Context & /* context */) override { for (size_t i = 0; i < boundaries_.size(); ++i) { diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/with_trace_sample_filter.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/with_trace_sample_filter.h index b81ccbccb2..46fceb306d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/with_trace_sample_filter.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/with_trace_sample_filter.h @@ -15,15 +15,15 @@ namespace metrics class WithTraceSampleFilter final : public ExemplarFilter { public: - bool ShouldSampleMeasurement(long value, - const MetricAttributes &attributes, + bool ShouldSampleMeasurement(long /* value */, + const MetricAttributes & /* attributes */, const opentelemetry::context::Context &context) noexcept override { return hasSampledTrace(context); } - bool ShouldSampleMeasurement(double value, - const MetricAttributes &attributes, + bool ShouldSampleMeasurement(double /* value */, + const MetricAttributes & /* attributes */, const opentelemetry::context::Context &context) noexcept override { return hasSampledTrace(context); From 4e81f46e44708efa52871e8e18bcafe0c9fea3b1 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 6 Oct 2022 00:38:14 +0200 Subject: [PATCH 15/25] More warning cleanup (thrift, logs) --- exporters/elasticsearch/src/es_log_exporter.cc | 2 +- exporters/jaeger/CMakeLists.txt | 11 +++++------ .../exporters/otlp/otlp_log_recordable.h | 2 +- .../opentelemetry/sdk/logs/batch_log_processor.h | 2 +- sdk/include/opentelemetry/sdk/logs/logger_provider.h | 2 +- .../opentelemetry/sdk/logs/multi_log_processor.h | 2 +- .../opentelemetry/sdk/logs/simple_log_processor.h | 2 +- 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/exporters/elasticsearch/src/es_log_exporter.cc b/exporters/elasticsearch/src/es_log_exporter.cc index 705ae3ba01..8593f4c8d8 100644 --- a/exporters/elasticsearch/src/es_log_exporter.cc +++ b/exporters/elasticsearch/src/es_log_exporter.cc @@ -168,7 +168,7 @@ class AsyncResponseHandler : public http_client::EventHandler /** * Cleans up the session in the destructor. */ - ~AsyncResponseHandler() { session_->FinishSession(); } + ~AsyncResponseHandler() override { session_->FinishSession(); } /** * Automatically called when the response is received diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index b0ab36f0cf..031ae76e80 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -9,12 +9,6 @@ set(JAEGER_THRIFT_GENCPP_SOURCES thrift-gen/Agent.cpp thrift-gen/jaeger_types.cpp thrift-gen/Collector.cpp thrift-gen/zipkincore_types.cpp) -if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - # THRIFT generated code is not warning clean for gcc. - set_source_files_properties( - JAEGER_THRIFT_GENCPP_SOURCES PROPERTIES COMPILE_OPTIONS "-Wno-suggest-override") -endif() - set(JAEGER_EXPORTER_SOURCES src/jaeger_exporter.cc src/jaeger_exporter_factory.cc @@ -28,6 +22,11 @@ set(JAEGER_EXPORTER_SOURCES add_library(opentelemetry_exporter_jaeger_trace ${JAEGER_EXPORTER_SOURCES} ${JAEGER_THRIFT_GENCPP_SOURCES}) +if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + # THRIFT generated code is not warning clean for gcc. + target_compile_options(opentelemetry_exporter_jaeger_trace PRIVATE "-Wno-suggest-override") +endif() + set_target_properties(opentelemetry_exporter_jaeger_trace PROPERTIES EXPORT_NAME jaeger_trace_exporter) diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_log_recordable.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_log_recordable.h index f8f0cabfdd..b10898ac3c 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_log_recordable.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_log_recordable.h @@ -30,7 +30,7 @@ namespace otlp class OtlpLogRecordable final : public opentelemetry::sdk::logs::Recordable { public: - virtual ~OtlpLogRecordable() = default; + ~OtlpLogRecordable() override = default; proto::logs::v1::LogRecord &log_record() noexcept { return log_record_; } const proto::logs::v1::LogRecord &log_record() const noexcept { return log_record_; } diff --git a/sdk/include/opentelemetry/sdk/logs/batch_log_processor.h b/sdk/include/opentelemetry/sdk/logs/batch_log_processor.h index e942564e28..efeb6dc675 100644 --- a/sdk/include/opentelemetry/sdk/logs/batch_log_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/batch_log_processor.h @@ -87,7 +87,7 @@ class BatchLogProcessor : public LogProcessor /** * Class destructor which invokes the Shutdown() method. */ - virtual ~BatchLogProcessor(); + ~BatchLogProcessor() override; protected: /** diff --git a/sdk/include/opentelemetry/sdk/logs/logger_provider.h b/sdk/include/opentelemetry/sdk/logs/logger_provider.h index 9afb67ba0c..e91c69040e 100644 --- a/sdk/include/opentelemetry/sdk/logs/logger_provider.h +++ b/sdk/include/opentelemetry/sdk/logs/logger_provider.h @@ -61,7 +61,7 @@ class LoggerProvider final : public opentelemetry::logs::LoggerProvider */ explicit LoggerProvider(std::shared_ptr context) noexcept; - ~LoggerProvider(); + ~LoggerProvider() override; /** * Creates a logger with the given name, and returns a shared pointer to it. diff --git a/sdk/include/opentelemetry/sdk/logs/multi_log_processor.h b/sdk/include/opentelemetry/sdk/logs/multi_log_processor.h index 227e2e020f..6b23a2652f 100644 --- a/sdk/include/opentelemetry/sdk/logs/multi_log_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/multi_log_processor.h @@ -29,7 +29,7 @@ class MultiLogProcessor : public LogProcessor { public: MultiLogProcessor(std::vector> &&processors); - ~MultiLogProcessor(); + ~MultiLogProcessor() override; void AddProcessor(std::unique_ptr &&processor); diff --git a/sdk/include/opentelemetry/sdk/logs/simple_log_processor.h b/sdk/include/opentelemetry/sdk/logs/simple_log_processor.h index 8364cedd4c..f8f29542a5 100644 --- a/sdk/include/opentelemetry/sdk/logs/simple_log_processor.h +++ b/sdk/include/opentelemetry/sdk/logs/simple_log_processor.h @@ -29,7 +29,7 @@ class SimpleLogProcessor : public LogProcessor public: explicit SimpleLogProcessor(std::unique_ptr &&exporter); - virtual ~SimpleLogProcessor() = default; + ~SimpleLogProcessor() override = default; std::unique_ptr MakeRecordable() noexcept override; From 01834ac79dd45ccca296c664963bb83e5666fdf7 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 10 Oct 2022 21:35:44 +0200 Subject: [PATCH 16/25] Warning cleanup for Thrift-gen --- examples/plugin/plugin/tracer.cc | 4 ++++ exporters/jaeger/CMakeLists.txt | 12 +++++++----- .../opentelemetry/exporters/jaeger/recordable.h | 5 +++++ .../exporters/jaeger/thrift_include_prefix.h | 11 +++++++++++ .../exporters/jaeger/thrift_include_suffix.h | 10 ++++++++++ exporters/jaeger/src/jaeger_exporter.cc | 5 +++++ exporters/jaeger/src/thrift_sender.h | 5 +++++ exporters/jaeger/src/transport.h | 4 ++++ exporters/jaeger/src/udp_transport.h | 5 +++++ sdk/test/metrics/async_metric_storage_test.cc | 1 - 10 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 exporters/jaeger/include/opentelemetry/exporters/jaeger/thrift_include_prefix.h create mode 100644 exporters/jaeger/include/opentelemetry/exporters/jaeger/thrift_include_suffix.h diff --git a/examples/plugin/plugin/tracer.cc b/examples/plugin/plugin/tracer.cc index 1c1b8678ca..55de64438d 100644 --- a/examples/plugin/plugin/tracer.cc +++ b/examples/plugin/plugin/tracer.cc @@ -45,6 +45,10 @@ class Span final : public trace::Span const common::KeyValueIterable & /*attributes*/) noexcept override {} + void AddEvent(nostd::string_view /*name*/, + const common::KeyValueIterable & /*attributes*/) noexcept override + {} + void SetStatus(trace::StatusCode /*code*/, nostd::string_view /*description*/) noexcept override {} diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index 031ae76e80..4bee61e325 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -9,6 +9,13 @@ set(JAEGER_THRIFT_GENCPP_SOURCES thrift-gen/Agent.cpp thrift-gen/jaeger_types.cpp thrift-gen/Collector.cpp thrift-gen/zipkincore_types.cpp) +if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + message("Relaxing GCC warnings on thrift-gen") + # THRIFT generated code is not warning clean for gcc. + set_source_files_properties( + ${JAEGER_THRIFT_GENCPP_SOURCES} PROPERTIES COMPILE_OPTIONS "-Wno-suggest-override") +endif() + set(JAEGER_EXPORTER_SOURCES src/jaeger_exporter.cc src/jaeger_exporter_factory.cc @@ -22,11 +29,6 @@ set(JAEGER_EXPORTER_SOURCES add_library(opentelemetry_exporter_jaeger_trace ${JAEGER_EXPORTER_SOURCES} ${JAEGER_THRIFT_GENCPP_SOURCES}) -if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - # THRIFT generated code is not warning clean for gcc. - target_compile_options(opentelemetry_exporter_jaeger_trace PRIVATE "-Wno-suggest-override") -endif() - set_target_properties(opentelemetry_exporter_jaeger_trace PROPERTIES EXPORT_NAME jaeger_trace_exporter) diff --git a/exporters/jaeger/include/opentelemetry/exporters/jaeger/recordable.h b/exporters/jaeger/include/opentelemetry/exporters/jaeger/recordable.h index e604d56d79..b1b944fd07 100644 --- a/exporters/jaeger/include/opentelemetry/exporters/jaeger/recordable.h +++ b/exporters/jaeger/include/opentelemetry/exporters/jaeger/recordable.h @@ -3,7 +3,12 @@ #pragma once +#include + #include + +#include + #include #include diff --git a/exporters/jaeger/include/opentelemetry/exporters/jaeger/thrift_include_prefix.h b/exporters/jaeger/include/opentelemetry/exporters/jaeger/thrift_include_prefix.h new file mode 100644 index 0000000000..92754d4800 --- /dev/null +++ b/exporters/jaeger/include/opentelemetry/exporters/jaeger/thrift_include_prefix.h @@ -0,0 +1,11 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +// This file may be include multiple times, do not add #pragma once here + +#if defined(__GNUC__) && !defined(__clang__) && !defined(__apple_build_version__) +# if (__GNUC__ * 100 + __GNUC_MINOR__ * 10) >= 460 +# pragma GCC diagnostic push +# endif +# pragma GCC diagnostic ignored "-Wsuggest-override" +#endif diff --git a/exporters/jaeger/include/opentelemetry/exporters/jaeger/thrift_include_suffix.h b/exporters/jaeger/include/opentelemetry/exporters/jaeger/thrift_include_suffix.h new file mode 100644 index 0000000000..fc276705c3 --- /dev/null +++ b/exporters/jaeger/include/opentelemetry/exporters/jaeger/thrift_include_suffix.h @@ -0,0 +1,10 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +// This file may be include multiple times, do not add #pragma once here + +#if defined(__GNUC__) && !defined(__clang__) && !defined(__apple_build_version__) +# if (__GNUC__ * 100 + __GNUC_MINOR__ * 10) >= 460 +# pragma GCC diagnostic pop +# endif +#endif diff --git a/exporters/jaeger/src/jaeger_exporter.cc b/exporters/jaeger/src/jaeger_exporter.cc index 9e1dac1162..8e9e75bd73 100644 --- a/exporters/jaeger/src/jaeger_exporter.cc +++ b/exporters/jaeger/src/jaeger_exporter.cc @@ -1,7 +1,12 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include + #include + +#include + #include #include #include "opentelemetry/sdk_config.h" diff --git a/exporters/jaeger/src/thrift_sender.h b/exporters/jaeger/src/thrift_sender.h index 0ec8d47f11..687f6fac45 100644 --- a/exporters/jaeger/src/thrift_sender.h +++ b/exporters/jaeger/src/thrift_sender.h @@ -3,7 +3,12 @@ #pragma once +#include + #include + +#include + #include #include #include diff --git a/exporters/jaeger/src/transport.h b/exporters/jaeger/src/transport.h index 8121e30076..d7ba8fa604 100644 --- a/exporters/jaeger/src/transport.h +++ b/exporters/jaeger/src/transport.h @@ -5,8 +5,12 @@ #include +#include + #include +#include + OPENTELEMETRY_BEGIN_NAMESPACE namespace exporter { diff --git a/exporters/jaeger/src/udp_transport.h b/exporters/jaeger/src/udp_transport.h index 3e07992f47..ce1f51e647 100644 --- a/exporters/jaeger/src/udp_transport.h +++ b/exporters/jaeger/src/udp_transport.h @@ -6,7 +6,12 @@ #include "TUDPTransport.h" #include "transport.h" +#include + #include + +#include + #include #include #include diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 74fff4661d..7e19912353 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -165,7 +165,6 @@ TEST_P(WritableMetricStorageTestObservableGaugeFixture, TestAggregation) std::shared_ptr{}); long freq_cpu0 = 3l; long freq_cpu1 = 5l; - size_t attribute_count = 2; std::unordered_map measurements1 = { {{{"CPU", "0"}}, freq_cpu0}, {{{"CPU", "1"}}, freq_cpu1}}; storage.RecordLong(measurements1, From 8f5c762c751cae400d06e5b03870b35f05df079f Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 10 Oct 2022 23:07:15 +0200 Subject: [PATCH 17/25] Move maintainer mode later in CMakeList.txt, so it is not enforced on third party code. --- CMakeLists.txt | 158 ++++++++++++++++++++++++------------------------- 1 file changed, 79 insertions(+), 79 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a8a01133cf..59ff23f3ba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -168,85 +168,6 @@ option(BUILD_W3CTRACECONTEXT_TEST "Whether to build w3c trace context" OFF) option(OTELCPP_MAINTAINER_MODE "Build in maintainer mode (-Wall -Werror)" OFF) -if(OTELCPP_MAINTAINER_MODE) - if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - message("Building with gcc in maintainer mode.") - - add_compile_options(-Wall) - add_compile_options(-Werror) - add_compile_options(-Wextra) - - # Tested with GCC 9.4 on github. - if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.4) - message("Building with additional warnings for gcc.") - - # Relaxed warnings - - # Enforced warnings - - # C++ options only - add_compile_options($<$,CXX>:-Wextra-semi>) - add_compile_options($<$,CXX>:-Woverloaded-virtual>) - add_compile_options($<$,CXX>:-Wsuggest-override>) - - # C and C++ - add_compile_options(-Wcast-qual) - add_compile_options(-Wformat-security) - add_compile_options(-Wlogical-op) - add_compile_options(-Wmissing-include-dirs) - add_compile_options(-Wstringop-truncation) - add_compile_options(-Wundef) - add_compile_options(-Wvla) - endif() - elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - message("Building with clang in maintainer mode.") - - add_compile_options(-Wall) - add_compile_options(-Werror) - add_compile_options(-Wextra) - - # Tested with Clang 11.0 on github. - if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 11.0) - message("Building with additional warnings for clang.") - - # Relaxed warnings - add_compile_options(-Wno-error=unused-private-field) - - # Enforced warnings - add_compile_options(-Wcast-qual) - add_compile_options(-Wconditional-uninitialized) - add_compile_options(-Wextra-semi) - add_compile_options(-Wformat-security) - add_compile_options(-Wheader-hygiene) - add_compile_options(-Winconsistent-missing-destructor-override) - add_compile_options(-Winconsistent-missing-override) - add_compile_options(-Wnewline-eof) - add_compile_options(-Wnon-virtual-dtor) - add_compile_options(-Woverloaded-virtual) - add_compile_options(-Wrange-loop-analysis) - add_compile_options(-Wundef) - add_compile_options(-Wundefined-reinterpret-cast) - add_compile_options(-Wvla) - endif() - elseif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") - message("Building with msvc in maintainer mode.") - - add_compile_options(/WX) - add_compile_options(/W4) - - # Relaxed warnings - add_compile_options(/wd4100) - add_compile_options(/wd4125) - add_compile_options(/wd4566) - add_compile_options(/wd4127) - add_compile_options(/wd4512) - - # Enforced warnings - elseif() - message(FATAL_ERROR "Building with unknown compiler in maintainer mode.") - endif() -endif(OTELCPP_MAINTAINER_MODE) - if(WIN32) if(BUILD_TESTING) if(MSVC) @@ -439,6 +360,85 @@ if((NOT WITH_API_ONLY) AND USE_NLOHMANN_JSON) endif() endif() +if(OTELCPP_MAINTAINER_MODE) + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + message("Building with gcc in maintainer mode.") + + add_compile_options(-Wall) + add_compile_options(-Werror) + add_compile_options(-Wextra) + + # Tested with GCC 9.4 on github. + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.4) + message("Building with additional warnings for gcc.") + + # Relaxed warnings + + # Enforced warnings + + # C++ options only + add_compile_options($<$,CXX>:-Wextra-semi>) + add_compile_options($<$,CXX>:-Woverloaded-virtual>) + add_compile_options($<$,CXX>:-Wsuggest-override>) + + # C and C++ + add_compile_options(-Wcast-qual) + add_compile_options(-Wformat-security) + add_compile_options(-Wlogical-op) + add_compile_options(-Wmissing-include-dirs) + add_compile_options(-Wstringop-truncation) + add_compile_options(-Wundef) + add_compile_options(-Wvla) + endif() + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + message("Building with clang in maintainer mode.") + + add_compile_options(-Wall) + add_compile_options(-Werror) + add_compile_options(-Wextra) + + # Tested with Clang 11.0 on github. + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 11.0) + message("Building with additional warnings for clang.") + + # Relaxed warnings + add_compile_options(-Wno-error=unused-private-field) + + # Enforced warnings + add_compile_options(-Wcast-qual) + add_compile_options(-Wconditional-uninitialized) + add_compile_options(-Wextra-semi) + add_compile_options(-Wformat-security) + add_compile_options(-Wheader-hygiene) + add_compile_options(-Winconsistent-missing-destructor-override) + add_compile_options(-Winconsistent-missing-override) + add_compile_options(-Wnewline-eof) + add_compile_options(-Wnon-virtual-dtor) + add_compile_options(-Woverloaded-virtual) + add_compile_options(-Wrange-loop-analysis) + add_compile_options(-Wundef) + add_compile_options(-Wundefined-reinterpret-cast) + add_compile_options(-Wvla) + endif() + elseif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") + message("Building with msvc in maintainer mode.") + + add_compile_options(/WX) + add_compile_options(/W4) + + # Relaxed warnings + add_compile_options(/wd4100) + add_compile_options(/wd4125) + add_compile_options(/wd4566) + add_compile_options(/wd4127) + add_compile_options(/wd4512) + + # Enforced warnings + elseif() + message(FATAL_ERROR "Building with unknown compiler in maintainer mode.") + endif() +endif(OTELCPP_MAINTAINER_MODE) + list(APPEND CMAKE_PREFIX_PATH "${CMAKE_BINARY_DIR}") include(CTest) From a79f13d0c5988606ba8893c25c4c5b8a29a54af9 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 11 Oct 2022 21:52:37 +0200 Subject: [PATCH 18/25] More warning cleanup --- CMakeLists.txt | 6 ++- examples/grpc/tracer_common.h | 6 +-- .../elasticsearch/src/es_log_exporter.cc | 44 +++++++++++++++++-- .../test/es_log_exporter_test.cc | 2 +- exporters/jaeger/BUILD | 2 + exporters/jaeger/CMakeLists.txt | 3 +- .../opentelemetry/sdk/metrics/instruments.h | 3 ++ 7 files changed, 55 insertions(+), 11 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 59ff23f3ba..498f6501e5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -378,8 +378,10 @@ if(OTELCPP_MAINTAINER_MODE) # C++ options only add_compile_options($<$,CXX>:-Wextra-semi>) - add_compile_options($<$,CXX>:-Woverloaded-virtual>) - add_compile_options($<$,CXX>:-Wsuggest-override>) + add_compile_options( + $<$,CXX>:-Woverloaded-virtual>) + add_compile_options( + $<$,CXX>:-Wsuggest-override>) # C and C++ add_compile_options(-Wcast-qual) diff --git a/examples/grpc/tracer_common.h b/examples/grpc/tracer_common.h index 51ffeb101f..71f0afeaa7 100644 --- a/examples/grpc/tracer_common.h +++ b/examples/grpc/tracer_common.h @@ -29,7 +29,7 @@ class GrpcClientCarrier : public opentelemetry::context::propagation::TextMapCar GrpcClientCarrier(ClientContext *context) : context_(context) {} GrpcClientCarrier() = default; virtual opentelemetry::nostd::string_view Get( - opentelemetry::nostd::string_view key) const noexcept override + opentelemetry::nostd::string_view /* key */) const noexcept override { return ""; } @@ -60,8 +60,8 @@ class GrpcServerCarrier : public opentelemetry::context::propagation::TextMapCar return ""; } - virtual void Set(opentelemetry::nostd::string_view key, - opentelemetry::nostd::string_view value) noexcept override + virtual void Set(opentelemetry::nostd::string_view /* key */, + opentelemetry::nostd::string_view /* value */) noexcept override { // Not required for server } diff --git a/exporters/elasticsearch/src/es_log_exporter.cc b/exporters/elasticsearch/src/es_log_exporter.cc index 8593f4c8d8..52ec1ef30a 100644 --- a/exporters/elasticsearch/src/es_log_exporter.cc +++ b/exporters/elasticsearch/src/es_log_exporter.cc @@ -112,20 +112,56 @@ class ResponseHandler : public http_client::EventHandler // If any failure event occurs, release the condition variable to unblock main thread switch (state) { + case http_client::SessionState::CreateFailed: + OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Failed to create session"); + cv_.notify_all(); + break; + case http_client::SessionState::Created: + OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Session created"); + break; + case http_client::SessionState::Destroyed: + OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Session destroyed"); + break; + case http_client::SessionState::Connecting: + OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Connecting to peer"); + break; case http_client::SessionState::ConnectFailed: - OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Connection to elasticsearch failed"); + OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Failed to connect to peer"); cv_.notify_all(); break; + case http_client::SessionState::Connected: + OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Connected to peer"); + break; + case http_client::SessionState::Sending: + OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Sending request"); + break; case http_client::SessionState::SendFailed: - OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Request failed to be sent to elasticsearch"); + OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Failed to sent request"); + cv_.notify_all(); + break; + case http_client::SessionState::Response: + OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Received response"); + break; + case http_client::SessionState::SSLHandshakeFailed: + OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Failed SSL Handshake"); cv_.notify_all(); break; case http_client::SessionState::TimedOut: - OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Request to elasticsearch timed out"); + OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Request timed out"); cv_.notify_all(); break; case http_client::SessionState::NetworkError: - OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Network error to elasticsearch"); + OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] Network error"); + cv_.notify_all(); + break; + case http_client::SessionState::ReadError: + OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Read error"); + break; + case http_client::SessionState::WriteError: + OTEL_INTERNAL_LOG_DEBUG("[ES Log Exporter] Write error"); + break; + case http_client::SessionState::Cancelled: + OTEL_INTERNAL_LOG_ERROR("[ES Log Exporter] (manually) cancelled"); cv_.notify_all(); break; } diff --git a/exporters/elasticsearch/test/es_log_exporter_test.cc b/exporters/elasticsearch/test/es_log_exporter_test.cc index 943f9fa7ba..e256c969e7 100644 --- a/exporters/elasticsearch/test/es_log_exporter_test.cc +++ b/exporters/elasticsearch/test/es_log_exporter_test.cc @@ -83,4 +83,4 @@ TEST(ElasticsearchLogsExporterTests, RecordableCreation) exporter->Export(nostd::span>(&record, 1)); } # endif -#endif \ No newline at end of file +#endif diff --git a/exporters/jaeger/BUILD b/exporters/jaeger/BUILD index 156d7cde62..1937d4457e 100644 --- a/exporters/jaeger/BUILD +++ b/exporters/jaeger/BUILD @@ -177,6 +177,8 @@ cc_library( "include/opentelemetry/exporters/jaeger/jaeger_exporter_factory.h", "include/opentelemetry/exporters/jaeger/jaeger_exporter_options.h", "include/opentelemetry/exporters/jaeger/recordable.h", + "include/opentelemetry/exporters/jaeger/thrift_include_prefix.h", + "include/opentelemetry/exporters/jaeger/thrift_include_suffix.h", ], copts = ["-fexceptions"], strip_include_prefix = "include", diff --git a/exporters/jaeger/CMakeLists.txt b/exporters/jaeger/CMakeLists.txt index 4bee61e325..9641300124 100644 --- a/exporters/jaeger/CMakeLists.txt +++ b/exporters/jaeger/CMakeLists.txt @@ -13,7 +13,8 @@ if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") message("Relaxing GCC warnings on thrift-gen") # THRIFT generated code is not warning clean for gcc. set_source_files_properties( - ${JAEGER_THRIFT_GENCPP_SOURCES} PROPERTIES COMPILE_OPTIONS "-Wno-suggest-override") + ${JAEGER_THRIFT_GENCPP_SOURCES} PROPERTIES COMPILE_OPTIONS + "-Wno-suggest-override") endif() set(JAEGER_EXPORTER_SOURCES diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index a529095600..4442b06765 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -62,6 +62,8 @@ struct InstrumentDescriptor using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap; using AggregationTemporalitySelector = std::function; + +#if 0 OPENTELEMETRY_MAYBE_UNUSED static InstrumentClass GetInstrumentClass(InstrumentType type) { if (type == InstrumentType::kCounter || type == InstrumentType::kHistogram || @@ -74,6 +76,7 @@ OPENTELEMETRY_MAYBE_UNUSED static InstrumentClass GetInstrumentClass(InstrumentT return InstrumentClass::kAsync; } } +#endif /*class InstrumentSelector { public: From e422bd18cab09c41471fc10aa8bfeb0fe7af5ace Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Tue, 11 Oct 2022 22:26:53 +0200 Subject: [PATCH 19/25] More cleanup --- api/test/trace/span_benchmark.cc | 18 ++++++++-------- examples/multithreaded/main.cc | 2 +- .../opentelemetry/sdk/metrics/instruments.h | 4 ++-- sdk/test/common/random_test.cc | 21 ++++++++++--------- sdk/test/metrics/meter_test.cc | 2 +- sdk/test/trace/tracer_test.cc | 4 ++-- 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/api/test/trace/span_benchmark.cc b/api/test/trace/span_benchmark.cc index ef14cd4ca3..fc54081ca6 100644 --- a/api/test/trace/span_benchmark.cc +++ b/api/test/trace/span_benchmark.cc @@ -55,19 +55,19 @@ void BM_NestedSpanCreationWithScope(benchmark::State &state) auto tracer = initTracer(); while (state.KeepRunning()) { - auto span = tracer->StartSpan("outer"); - auto scope = tracer->WithActiveSpan(span); + auto o_span = tracer->StartSpan("outer"); + auto o_scope = tracer->WithActiveSpan(o_span); { - auto span = tracer->StartSpan("inner"); - auto scope = tracer->WithActiveSpan(span); + auto i_span = tracer->StartSpan("inner"); + auto i_scope = tracer->WithActiveSpan(i_span); { - auto span = tracer->StartSpan("innermost"); - auto scope = tracer->WithActiveSpan(span); - span->End(); + auto im_span = tracer->StartSpan("innermost"); + auto im_scope = tracer->WithActiveSpan(im_span); + im_span->End(); } - span->End(); + i_span->End(); } - span->End(); + o_span->End(); } } diff --git a/examples/multithreaded/main.cc b/examples/multithreaded/main.cc index bb6020bf58..33679f8309 100644 --- a/examples/multithreaded/main.cc +++ b/examples/multithreaded/main.cc @@ -46,7 +46,7 @@ void run_threads() // parent spans across threads. threads.push_back(std::thread([=] { trace_api::Scope scope(thread_span); - auto thread_span = + auto thread_span_2 = get_tracer()->StartSpan(std::string("thread ") + std::to_string(thread_num)); })); } diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index 4442b06765..d69d39f648 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -63,7 +63,7 @@ struct InstrumentDescriptor using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap; using AggregationTemporalitySelector = std::function; -#if 0 +# if 0 OPENTELEMETRY_MAYBE_UNUSED static InstrumentClass GetInstrumentClass(InstrumentType type) { if (type == InstrumentType::kCounter || type == InstrumentType::kHistogram || @@ -76,7 +76,7 @@ OPENTELEMETRY_MAYBE_UNUSED static InstrumentClass GetInstrumentClass(InstrumentT return InstrumentClass::kAsync; } } -#endif +# endif /*class InstrumentSelector { public: diff --git a/sdk/test/common/random_test.cc b/sdk/test/common/random_test.cc index 35cfd4a1ee..8132b2d5a4 100644 --- a/sdk/test/common/random_test.cc +++ b/sdk/test/common/random_test.cc @@ -16,20 +16,21 @@ TEST(RandomTest, GenerateRandom64) TEST(RandomTest, GenerateRandomBuffer) { - uint8_t buf1[8] = {0}; - uint8_t buf2[8] = {0}; - Random::GenerateRandomBuffer(buf1); - Random::GenerateRandomBuffer(buf2); - EXPECT_FALSE(std::equal(std::begin(buf1), std::end(buf1), std::begin(buf2))); + uint8_t buf1_array[8] = {0}; + uint8_t buf2_array[8] = {0}; + Random::GenerateRandomBuffer(buf1_array); + Random::GenerateRandomBuffer(buf2_array); + EXPECT_FALSE(std::equal(std::begin(buf1_array), std::end(buf1_array), std::begin(buf2_array))); // Edge cases. for (auto size : {7, 8, 9, 16, 17}) { - std::vector buf1(size); - std::vector buf2(size); + std::vector buf1_vector(size); + std::vector buf2_vector(size); - Random::GenerateRandomBuffer(buf1); - Random::GenerateRandomBuffer(buf2); - EXPECT_FALSE(std::equal(std::begin(buf1), std::end(buf1), std::begin(buf2))); + Random::GenerateRandomBuffer(buf1_vector); + Random::GenerateRandomBuffer(buf2_vector); + EXPECT_FALSE( + std::equal(std::begin(buf1_vector), std::end(buf1_vector), std::begin(buf2_vector))); } } diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc index 77fde74d03..b4cd12416c 100644 --- a/sdk/test/metrics/meter_test.cc +++ b/sdk/test/metrics/meter_test.cc @@ -115,7 +115,7 @@ TEST(MeterTest, StressMultiThread) } if (do_collect.exchange(false)) { - metric_reader_ptr->Collect([](ResourceMetrics &metric_data) { return true; }); + metric_reader_ptr->Collect([](ResourceMetrics & /* metric_data */) { return true; }); do_sync_create.store(true); } }); diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 16be43fa6f..3287b39e5d 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -723,10 +723,10 @@ TEST(Tracer, SpanCleanupWithScope) auto span0 = tracer->StartSpan("Span0"); auto span1 = tracer->StartSpan("span1"); { - trace_api::Scope scope(span1); + trace_api::Scope scope1(span1); auto span2 = tracer->StartSpan("span2"); { - trace_api::Scope scope(span2); + trace_api::Scope scope2(span2); auto span3 = tracer->StartSpan("span3"); } } From 18ed3490b6ceaf00ed48c82baf3c477620d4ff92 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 13 Oct 2022 22:21:02 +0200 Subject: [PATCH 20/25] Fix last 4 warnings known. --- api/test/nostd/unique_ptr_test.cc | 7 ++++++- api/test/trace/span_benchmark.cc | 4 ++-- .../exemplar/histogram_exemplar_reservoir.h | 16 ++++++++-------- .../metrics/exemplar/reservoir_cell_selector.h | 16 ++++++++-------- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/api/test/nostd/unique_ptr_test.cc b/api/test/nostd/unique_ptr_test.cc index 1d2e6c062f..82553605f5 100644 --- a/api/test/nostd/unique_ptr_test.cc +++ b/api/test/nostd/unique_ptr_test.cc @@ -122,7 +122,12 @@ TEST(UniquePtrTest, Reset) { bool was_destructed1; unique_ptr ptr{new A{was_destructed1}}; - bool was_destructed2; + /* + MSVC reports: + warning C4701: potentially uninitialized local variable 'was_destructed2' used + so initializing explicitly. + */ + bool was_destructed2 = true; ptr.reset(new A{was_destructed2}); EXPECT_TRUE(was_destructed1); EXPECT_FALSE(was_destructed2); diff --git a/api/test/trace/span_benchmark.cc b/api/test/trace/span_benchmark.cc index fc54081ca6..3120eb0e5a 100644 --- a/api/test/trace/span_benchmark.cc +++ b/api/test/trace/span_benchmark.cc @@ -115,10 +115,10 @@ void BM_SpanCreationWitContextPropagation(benchmark::State &state) nostd::shared_ptr(new trace_api::DefaultSpan(outer_span_context)); trace_api::SetSpan(current_ctx, outer_span); auto inner_child = tracer->StartSpan("inner"); - auto scope = tracer->WithActiveSpan(inner_child); + auto inner_scope = tracer->WithActiveSpan(inner_child); { auto innermost_child = tracer->StartSpan("innermost"); - auto scope = tracer->WithActiveSpan(innermost_child); + auto innermost_scope = tracer->WithActiveSpan(innermost_child); innermost_child->End(); } inner_child->End(); diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h index 5481327a5a..8eb9b7be52 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h @@ -42,18 +42,18 @@ class HistogramExemplarReservoir : public FixedSizeExemplarReservoir public: HistogramCellSelector(const std::vector &boundaries) : boundaries_(boundaries) {} - int ReservoirCellIndexFor(const std::vector &cells, - long value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context) override + size_t ReservoirCellIndexFor(const std::vector &cells, + long value, + const MetricAttributes &attributes, + const opentelemetry::context::Context &context) override { return ReservoirCellIndexFor(cells, (double)value, attributes, context); } - int ReservoirCellIndexFor(const std::vector & /* cells */, - double value, - const MetricAttributes & /* attributes */, - const opentelemetry::context::Context & /* context */) override + size_t ReservoirCellIndexFor(const std::vector & /* cells */, + double value, + const MetricAttributes & /* attributes */, + const opentelemetry::context::Context & /* context */) override { for (size_t i = 0; i < boundaries_.size(); ++i) { diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h index 13e149f0aa..7bd9187742 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h @@ -23,16 +23,16 @@ class ReservoirCellSelector virtual ~ReservoirCellSelector() = default; /** Determine the index of the {@code cells} to record the measurement to. */ - virtual int ReservoirCellIndexFor(const std::vector &cells, - long value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context) = 0; + virtual size_t ReservoirCellIndexFor(const std::vector &cells, + long value, + const MetricAttributes &attributes, + const opentelemetry::context::Context &context) = 0; /** Determine the index of the {@code cells} to record the measurement to. */ - virtual int ReservoirCellIndexFor(const std::vector &cells, - double value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context) = 0; + virtual size_t ReservoirCellIndexFor(const std::vector &cells, + double value, + const MetricAttributes &attributes, + const opentelemetry::context::Context &context) = 0; /** Called when {@link FixedSizeExemplarReservoir#CollectAndReset(Attributes)}. */ virtual void reset() = 0; From e7310b23c6e22c494cbd71cbca6ca7aee1c78960 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 13 Oct 2022 22:59:34 +0200 Subject: [PATCH 21/25] Cleanup --- CHANGELOG.md | 2 ++ .../exemplar/histogram_exemplar_reservoir.h | 19 ++++++++++--------- .../exemplar/reservoir_cell_selector.h | 16 ++++++++-------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a5e5af31e..1bc1c8e054 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Increment the: ## [Unreleased] +* [BUILD] Add CMake OTELCPP_MAINTAINER_MODE [#1650](https://github.com/open-telemetry/opentelemetry-cpp/pull/1650) + ## [1.6.1] 2022-09-22 * [BUILD] Upgrade opentelemetry-proto to v0.19.0 [#1579](https://github.com/open-telemetry/opentelemetry-cpp/pull/1579) diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h index 8eb9b7be52..f15665de33 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/histogram_exemplar_reservoir.h @@ -42,20 +42,21 @@ class HistogramExemplarReservoir : public FixedSizeExemplarReservoir public: HistogramCellSelector(const std::vector &boundaries) : boundaries_(boundaries) {} - size_t ReservoirCellIndexFor(const std::vector &cells, - long value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context) override + int ReservoirCellIndexFor(const std::vector &cells, + long value, + const MetricAttributes &attributes, + const opentelemetry::context::Context &context) override { return ReservoirCellIndexFor(cells, (double)value, attributes, context); } - size_t ReservoirCellIndexFor(const std::vector & /* cells */, - double value, - const MetricAttributes & /* attributes */, - const opentelemetry::context::Context & /* context */) override + int ReservoirCellIndexFor(const std::vector & /* cells */, + double value, + const MetricAttributes & /* attributes */, + const opentelemetry::context::Context & /* context */) override { - for (size_t i = 0; i < boundaries_.size(); ++i) + int max_size = boundaries_.size(); + for (int i = 0; i < max_size; ++i) { if (value <= boundaries_[i]) { diff --git a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h index 7bd9187742..13e149f0aa 100644 --- a/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h +++ b/sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h @@ -23,16 +23,16 @@ class ReservoirCellSelector virtual ~ReservoirCellSelector() = default; /** Determine the index of the {@code cells} to record the measurement to. */ - virtual size_t ReservoirCellIndexFor(const std::vector &cells, - long value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context) = 0; + virtual int ReservoirCellIndexFor(const std::vector &cells, + long value, + const MetricAttributes &attributes, + const opentelemetry::context::Context &context) = 0; /** Determine the index of the {@code cells} to record the measurement to. */ - virtual size_t ReservoirCellIndexFor(const std::vector &cells, - double value, - const MetricAttributes &attributes, - const opentelemetry::context::Context &context) = 0; + virtual int ReservoirCellIndexFor(const std::vector &cells, + double value, + const MetricAttributes &attributes, + const opentelemetry::context::Context &context) = 0; /** Called when {@link FixedSizeExemplarReservoir#CollectAndReset(Attributes)}. */ virtual void reset() = 0; From cd3b8f839efb0276142ed175af049437e827d0be Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 13 Oct 2022 23:25:35 +0200 Subject: [PATCH 22/25] Relax MSVC warning --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 498f6501e5..14db63d130 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -434,6 +434,7 @@ if(OTELCPP_MAINTAINER_MODE) add_compile_options(/wd4566) add_compile_options(/wd4127) add_compile_options(/wd4512) + add_compile_options(/wd4267) # Enforced warnings elseif() From 895b87185d1a0289ab2b8c2788bc5d3d03f15503 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Fri, 14 Oct 2022 00:00:54 +0200 Subject: [PATCH 23/25] Cleanup, variable hides parameter. --- sdk/test/metrics/async_metric_storage_test.cc | 4 ++-- sdk/test/metrics/sync_metric_storage_counter_test.cc | 4 ++-- sdk/test/metrics/sync_metric_storage_histogram_test.cc | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 7e19912353..772099abfe 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -78,8 +78,8 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now())); storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( diff --git a/sdk/test/metrics/sync_metric_storage_counter_test.cc b/sdk/test/metrics/sync_metric_storage_counter_test.cc index 555ae8ce09..8d02b2bdf2 100644 --- a/sdk/test/metrics/sync_metric_storage_counter_test.cc +++ b/sdk/test/metrics/sync_metric_storage_counter_test.cc @@ -80,8 +80,8 @@ TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) auto collection_ts = std::chrono::system_clock::now(); size_t count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index 865fcf7125..6242d78c96 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -81,8 +81,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, LongHistogram) auto collection_ts = std::chrono::system_clock::now(); size_t count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( From 7555a07ccebcf76f8c2dba44764ddfc71810ccb1 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Fri, 14 Oct 2022 00:41:30 +0200 Subject: [PATCH 24/25] Cleanup, data parameter hides function param. --- sdk/test/metrics/async_metric_storage_test.cc | 12 +++++------ .../sync_metric_storage_counter_test.cc | 20 +++++++++---------- .../sync_metric_storage_histogram_test.cc | 20 +++++++++---------- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 772099abfe..6b92ff1ec4 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -105,8 +105,8 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) storage.RecordLong(measurements2, opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now())); storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( @@ -171,8 +171,8 @@ TEST_P(WritableMetricStorageTestObservableGaugeFixture, TestAggregation) opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now())); storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get(data_attr.attributes.find("CPU")->second) == @@ -197,8 +197,8 @@ TEST_P(WritableMetricStorageTestObservableGaugeFixture, TestAggregation) storage.RecordLong(measurements2, opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now())); storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get(data_attr.attributes.find("CPU")->second) == diff --git a/sdk/test/metrics/sync_metric_storage_counter_test.cc b/sdk/test/metrics/sync_metric_storage_counter_test.cc index 8d02b2bdf2..268823f6c0 100644 --- a/sdk/test/metrics/sync_metric_storage_counter_test.cc +++ b/sdk/test/metrics/sync_metric_storage_counter_test.cc @@ -112,8 +112,8 @@ TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( @@ -146,8 +146,8 @@ TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( @@ -219,8 +219,8 @@ TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) auto collection_ts = std::chrono::system_clock::now(); size_t count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( @@ -252,8 +252,8 @@ TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( @@ -288,8 +288,8 @@ TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index 6242d78c96..0656381188 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -113,8 +113,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, LongHistogram) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( @@ -147,8 +147,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, LongHistogram) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( @@ -220,8 +220,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, DoubleHistogram) auto collection_ts = std::chrono::system_clock::now(); size_t count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( @@ -253,8 +253,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, DoubleHistogram) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( @@ -289,8 +289,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, DoubleHistogram) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData data) { - for (auto data_attr : data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { + for (auto data_attr : metric_data.point_data_attr_) { auto data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( From 97ad7aeddaef57535f1c9f3a9e6c44d06d554225 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Sat, 15 Oct 2022 02:06:56 +0200 Subject: [PATCH 25/25] Fixed code review comments. --- api/test/nostd/unique_ptr_test.cc | 5 --- sdk/test/metrics/async_metric_storage_test.cc | 42 +++++++++---------- .../sync_metric_storage_counter_test.cc | 36 ++++++++-------- .../sync_metric_storage_histogram_test.cc | 36 ++++++++-------- 4 files changed, 57 insertions(+), 62 deletions(-) diff --git a/api/test/nostd/unique_ptr_test.cc b/api/test/nostd/unique_ptr_test.cc index 82553605f5..aa49d387b7 100644 --- a/api/test/nostd/unique_ptr_test.cc +++ b/api/test/nostd/unique_ptr_test.cc @@ -122,11 +122,6 @@ TEST(UniquePtrTest, Reset) { bool was_destructed1; unique_ptr ptr{new A{was_destructed1}}; - /* - MSVC reports: - warning C4701: potentially uninitialized local variable 'was_destructed2' used - so initializing explicitly. - */ bool was_destructed2 = true; ptr.reset(new A{was_destructed2}); EXPECT_TRUE(was_destructed1); diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 6b92ff1ec4..88be4bb7f2 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/sdk/test/metrics/async_metric_storage_test.cc @@ -77,24 +77,24 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) storage.RecordLong(measurements1, opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now())); - storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts, - [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) - { - auto data = opentelemetry::nostd::get(data_attr.point_data); - if (opentelemetry::nostd::get( - data_attr.attributes.find("RequestType")->second) == "GET") - { - EXPECT_EQ(opentelemetry::nostd::get(data.value_), get_count1); - } - else if (opentelemetry::nostd::get( - data_attr.attributes.find("RequestType")->second) == "PUT") - { - EXPECT_EQ(opentelemetry::nostd::get(data.value_), put_count1); - } - } - return true; - }); + storage.Collect( + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) + { + const auto &data = opentelemetry::nostd::get(data_attr.point_data); + if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "GET") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), get_count1); + } + else if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "PUT") + { + EXPECT_EQ(opentelemetry::nostd::get(data.value_), put_count1); + } + } + return true; + }); // subsequent recording after collection shouldn't fail // monotonic increasing values; long get_count2 = 50l; @@ -105,10 +105,10 @@ TEST_P(WritableMetricStorageTestFixture, TestAggregation) storage.RecordLong(measurements2, opentelemetry::common::SystemTimestamp(std::chrono::system_clock::now())); storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { diff --git a/sdk/test/metrics/sync_metric_storage_counter_test.cc b/sdk/test/metrics/sync_metric_storage_counter_test.cc index 268823f6c0..de0a7a5ce2 100644 --- a/sdk/test/metrics/sync_metric_storage_counter_test.cc +++ b/sdk/test/metrics/sync_metric_storage_counter_test.cc @@ -80,10 +80,10 @@ TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) auto collection_ts = std::chrono::system_clock::now(); size_t count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -112,10 +112,10 @@ TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -146,10 +146,10 @@ TEST_P(WritableMetricStorageTestFixture, LongSumAggregation) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -219,10 +219,10 @@ TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) auto collection_ts = std::chrono::system_clock::now(); size_t count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -252,10 +252,10 @@ TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -288,10 +288,10 @@ TEST_P(WritableMetricStorageTestFixture, DoubleSumAggregation) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index 0656381188..8a2d368e64 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -81,10 +81,10 @@ TEST_P(WritableMetricStorageHistogramTestFixture, LongHistogram) auto collection_ts = std::chrono::system_clock::now(); size_t count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -113,10 +113,10 @@ TEST_P(WritableMetricStorageHistogramTestFixture, LongHistogram) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -147,10 +147,10 @@ TEST_P(WritableMetricStorageHistogramTestFixture, LongHistogram) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -220,10 +220,10 @@ TEST_P(WritableMetricStorageHistogramTestFixture, DoubleHistogram) auto collection_ts = std::chrono::system_clock::now(); size_t count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -253,10 +253,10 @@ TEST_P(WritableMetricStorageHistogramTestFixture, DoubleHistogram) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -289,10 +289,10 @@ TEST_P(WritableMetricStorageHistogramTestFixture, DoubleHistogram) collection_ts = std::chrono::system_clock::now(); count_attributes = 0; storage.Collect( - collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData metric_data) { - for (auto data_attr : metric_data.point_data_attr_) + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) { - auto data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") {