Fixes #249 (#1677)

Fixed many remaining compiler warnings.

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!

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();
   }
 }
 

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
   }

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));
     }));
   }

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
   {}
 

exporters/elasticsearch/src/es_log_exporter.cc

@@ -107,25 +107,61 @@ 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)
     {
+      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 send 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;
     }
@@ -160,15 +196,15 @@ class AsyncResponseHandler : public http_client::EventHandler
       std::shared_ptr<ext::http::client::Session> session,
       std::function<bool(opentelemetry::sdk::common::ExportResult)> &&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}
   {}
 
   /**
    * Cleans up the session in the destructor.
    */
-  ~AsyncResponseHandler() { session_->FinishSession(); }
+  ~AsyncResponseHandler() override { session_->FinishSession(); }
 
   /**
    * Automatically called when the response is received
@@ -197,7 +233,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 +400,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<opentelemetry::common::SpinLockMutex> locked(lock_);
   is_shutdown_ = true;

exporters/elasticsearch/test/es_log_exporter_test.cc

@@ -83,4 +83,4 @@ TEST(ElasticsearchLogsExporterTests, RecordableCreation)
   exporter->Export(nostd::span<std::unique_ptr<sdklogs::Recordable>>(&record, 1));
 }
 #  endif
-#endif
+#endif

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",

exporters/jaeger/CMakeLists.txt

@@ -9,6 +9,14 @@ 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

exporters/jaeger/include/opentelemetry/exporters/jaeger/recordable.h

@@ -3,7 +3,12 @@
 
 #pragma once
 
+#include <opentelemetry/exporters/jaeger/thrift_include_prefix.h>
+
 #include <jaeger_types.h>
+
+#include <opentelemetry/exporters/jaeger/thrift_include_suffix.h>
+
 #include <opentelemetry/sdk/trace/recordable.h>
 #include <opentelemetry/version.h>
 

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

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

exporters/jaeger/src/jaeger_exporter.cc

@@ -1,7 +1,12 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
+#include <opentelemetry/exporters/jaeger/thrift_include_prefix.h>
+
 #include <agent_types.h>
+
+#include <opentelemetry/exporters/jaeger/thrift_include_suffix.h>
+
 #include <opentelemetry/exporters/jaeger/jaeger_exporter.h>
 #include <opentelemetry/exporters/jaeger/recordable.h>
 #include "opentelemetry/sdk_config.h"

exporters/jaeger/src/thrift_sender.h

@@ -3,7 +3,12 @@
 
 #pragma once
 
+#include <opentelemetry/exporters/jaeger/thrift_include_prefix.h>
+
 #include <Agent.h>
+
+#include <opentelemetry/exporters/jaeger/thrift_include_suffix.h>
+
 #include <atomic>
 #include <memory>
 #include <mutex>

exporters/jaeger/src/transport.h

@@ -5,8 +5,12 @@
 
 #include <opentelemetry/version.h>
 
+#include <opentelemetry/exporters/jaeger/thrift_include_prefix.h>
+
 #include <jaeger_types.h>
 
+#include <opentelemetry/exporters/jaeger/thrift_include_suffix.h>
+
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace exporter
 {

exporters/jaeger/src/udp_transport.h

@@ -6,7 +6,12 @@
 #include "TUDPTransport.h"
 #include "transport.h"
 
+#include <opentelemetry/exporters/jaeger/thrift_include_prefix.h>
+
 #include <Agent.h>
+
+#include <opentelemetry/exporters/jaeger/thrift_include_suffix.h>
+
 #include <thrift/protocol/TBinaryProtocol.h>
 #include <thrift/protocol/TCompactProtocol.h>
 #include <thrift/protocol/TProtocol.h>

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_; }

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
+#  pragma clang diagnostic ignored "-Wshadow-field"
+#endif

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 <gtest/gtest.h>
 
 OPENTELEMETRY_BEGIN_NAMESPACE

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<ext::http::client::Response>{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<std::unique_ptr<sdk::trace::Recordable>> 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<ext::http::client::Response>{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);
 

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 <gtest/gtest.h>
 
 namespace trace    = opentelemetry::trace;

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:
   /**

sdk/include/opentelemetry/sdk/logs/logger_provider.h

@@ -61,7 +61,7 @@ class LoggerProvider final : public opentelemetry::logs::LoggerProvider
    */
   explicit LoggerProvider(std::shared_ptr<sdk::logs::LoggerContext> context) noexcept;
 
-  ~LoggerProvider();
+  ~LoggerProvider() override;
 
   /**
    * Creates a logger with the given name, and returns a shared pointer to it.

sdk/include/opentelemetry/sdk/logs/multi_log_processor.h

@@ -29,7 +29,7 @@ class MultiLogProcessor : public LogProcessor
 {
 public:
   MultiLogProcessor(std::vector<std::unique_ptr<LogProcessor>> &&processors);
-  ~MultiLogProcessor();
+  ~MultiLogProcessor() override;
 
   void AddProcessor(std::unique_ptr<LogProcessor> &&processor);