Simplify logging (#1568)

Closes #1180.

sdk/core/azure-core/CHANGELOG.md

@@ -10,6 +10,7 @@
 ### Breaking Changes
 
 - Remove `Context::CancelWhen()`.
+- Removed `LogClassification` and related functionality, added `LogLevel` instead.
 
 ### Bug Fixes
 

sdk/core/azure-core/inc/azure/core/http/policy.hpp

@@ -12,7 +12,6 @@
 #include "azure/core/credentials.hpp"
 #include "azure/core/http/http.hpp"
 #include "azure/core/http/transport.hpp"
-#include "azure/core/logging/logging.hpp"
 #include "azure/core/uuid.hpp"
 
 #include <chrono>
@@ -362,7 +361,7 @@ namespace Azure { namespace Core { namespace Http {
   /**
    * @brief Logs every HTTP request.
    *
-   * @detail Logs every HTTP request, response, or retry attempt (see #LogClassification)
+   * @detail Logs every HTTP request, response, or retry attempt
    * @remark See #logging.hpp
    */
   class LoggingPolicy : public HttpPolicy {
@@ -383,25 +382,6 @@ namespace Azure { namespace Core { namespace Http {
         NextHttpPolicy nextHttpPolicy) const override;
   };
 
-  /**
-   * @brief Log classigications being used to designate log messages from HTTP #LoggingPolicy.
-   */
-  class LogClassification : private Azure::Core::Logging::Details::LogClassificationProvider<
-                                Azure::Core::Logging::Details::Facility::Core> {
-  public:
-    /// HTTP request.
-    static constexpr auto const Request = Classification(1);
-
-    /// HTTP response.
-    static constexpr auto const Response = Classification(2);
-
-    /// HTTP retry attempt.
-    static constexpr auto const Retry = Classification(3);
-
-    /// HTTP Transport adapter.
-    static constexpr auto const HttpTransportAdapter = Classification(4);
-  };
-
   namespace Internal {
     /**
      * @brief @ValuePolicy options.

sdk/core/azure-core/inc/azure/core/internal/log.hpp

@@ -5,7 +5,7 @@
 
 #include "azure/core/logging/logging.hpp"
 
-namespace Azure { namespace Core { namespace Logging { namespace Details {
-  bool ShouldWrite(LogClassification const& classification);
-  void Write(LogClassification const& classification, std::string const& message);
-}}}} // namespace Azure::Core::Logging::Details
+namespace Azure { namespace Core { namespace Logging { namespace Internal {
+  bool ShouldLog(LogLevel level);
+  void Log(LogLevel level, std::string const& message);
+}}}} // namespace Azure::Core::Logging::Internal

sdk/core/azure-core/inc/azure/core/logging/logging.hpp

@@ -9,143 +9,50 @@
 
 #pragma once
 
-#include "azure/core/dll_import_export.hpp"
-
 #include <functional>
-#include <initializer_list>
-#include <set>
 #include <string>
-#include <utility>
 
 namespace Azure { namespace Core { namespace Logging {
-  class LogClassification;
-  class LogClassifications;
+  /**
+   * @brief Log message level.
+   */
+  enum class LogLevel
+  {
+    /// Logging level for failures that the application is unlikely to recover from.
+    Error,
+
+    /// Logging level when a function fails to perform its intended task.
+    Warning,
+
+    /// Logging level when a function operates normally.
+    Informational,
+
+    /// Logging level for detailed troubleshooting scenarios.
+    Verbose,
+  };
 
   /**
    * @brief Defines the signature of the callback function that application developers must write in
    * order to receive Azure SDK log messages.
    *
-   * @param classification The log message classification.
-   * @param classification The log message.
+   * @param level The log message level.
+   * @param message The log message.
    */
-  typedef std::function<void(LogClassification const& classification, std::string const& message)>
-      LogListener;
+  typedef std::function<void(LogLevel level, std::string const& message)> LogListener;
 
   /**
    * @brief Set the function that will be invoked to report an SDK log message.
    *
    * @param logListener A #LogListener function that will be invoked when the SDK reports a log
-   * message matching one of the log classifications passed to #SetLogClassifications(). If null, no
+   * message matching one of the log levels passed to #SetLogLevel(). If `nullptr`, no
    * function will be invoked.
    */
   void SetLogListener(LogListener logListener);
 
   /**
-   * @brief Allows the application to specify which log classification types it is interested in
-   * receiving.
+   * @brief Sets the #LogLevel an application is interested in receiving.
    *
-   * @param logClassifications Log classification values.
+   * @param level Maximum log level.
    */
-  void SetLogClassifications(LogClassifications logClassifications);
-
-  namespace Details {
-    enum class Facility : uint16_t
-    {
-      Core = 1,
-      Storage = 100,
-    };
-
-    template <Facility> class LogClassificationProvider;
-
-    class LogClassificationsPrivate;
-  } // namespace Details
-
-  /**
-   * @brief Represents a set of log classifications.
-   */
-  class LogClassifications {
-    friend class Details::LogClassificationsPrivate;
-
-    std::set<LogClassification> m_classifications;
-    bool m_all;
-
-    explicit LogClassifications(bool all) : m_all(all) {}
-
-  public:
-    /**
-     * @brief Initialize the list of log classifications with `std::initializer_list`.
-     * @param list An initializer list.
-     */
-    LogClassifications(std::initializer_list<LogClassification> list)
-        : m_classifications(list), m_all(false)
-    {
-    }
-
-    /**
-     * @brief Initialize the list of log classifications with `std::set`.
-     * @param set A set of classifications.
-     */
-    explicit LogClassifications(std::set<LogClassification> set)
-        : m_classifications(std::move(set)), m_all(false)
-    {
-    }
-  };
-
-  /**
-   * @brief Represents a log classification.
-   */
-  class LogClassification {
-    template <Details::Facility> friend class Details::LogClassificationProvider;
-    friend struct std::less<LogClassification>;
-
-    int32_t m_value;
-
-    constexpr explicit LogClassification(Details::Facility facility, int16_t number)
-        : m_value((static_cast<int32_t>(number) << 16) | static_cast<int32_t>(facility))
-    {
-    }
-
-    constexpr bool operator<(LogClassification const& other) const
-    {
-      return m_value < other.m_value;
-    }
-
-  public:
-    /**
-     * @brief Compare log classification to another one.
-     * @param other Another log classification to compare to.
-     * @return `true` if this log classification equals to \p other, `false` otherwise.
-     */
-    constexpr bool operator==(LogClassification const& other) const
-    {
-      return m_value == other.m_value;
-    }
-
-    /**
-     * @brief Compare log classification to another one.
-     * @param other Another log classification to compare to.
-     * @return `true` if this log classification does not equal to \p other, `false` otherwise.
-     */
-    constexpr bool operator!=(LogClassification const& other) const
-    {
-      return m_value != other.m_value;
-    }
-
-    /**
-     * @brief Represents a list of all classifications.
-     */
-    AZ_CORE_DLLEXPORT static LogClassifications const All;
-
-    /**
-     * @brief Represents an empty list of classifications.
-     */
-    AZ_CORE_DLLEXPORT static LogClassifications const None;
-  };
-
-  namespace Details {
-    template <Facility F> class LogClassificationProvider {
-    protected:
-      constexpr static auto Classification(int16_t number) { return LogClassification(F, number); }
-    };
-  } // namespace Details
+  void SetLogLevel(LogLevel level);
 }}} // namespace Azure::Core::Logging

sdk/core/azure-core/src/http/curl/curl.cpp

@@ -25,17 +25,7 @@
 #include <thread>
 
 namespace {
-// Can be used from anywhere a little simpler
-inline void LogThis(std::string const& msg)
-{
-  if (Azure::Core::Logging::Details::ShouldWrite(
-          Azure::Core::Http::LogClassification::HttpTransportAdapter))
-  {
-    Azure::Core::Logging::Details::Write(
-        Azure::Core::Http::LogClassification::HttpTransportAdapter,
-        "[CURL Transport Adapter]: " + msg);
-  }
-}
+std::string const LogMsgPrefix = "[CURL Transport Adapter]: ";
 
 template <typename T>
 #if defined(_MSC_VER)
@@ -136,9 +126,17 @@ void WinSocketSetBuffSize(curl_socket_t socket)
     // Specifies the total per-socket buffer space reserved for sends.
     // https://docs.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-setsockopt
     auto result = setsockopt(socket, SOL_SOCKET, SO_SNDBUF, (const char*)&ideal, sizeof(ideal));
-    LogThis(
-        "Windows - calling setsockopt after uploading chunk. ideal = " + std::to_string(ideal)
-        + " result = " + std::to_string(result));
+
+    {
+      using namespace Azure::Core::Logging;
+      using namespace Azure::Core::Logging::Internal;
+      if (ShouldLog(LogLevel::Verbose))
+      {
+        Log(LogLevel::Verbose,
+            LogMsgPrefix + "Windows - calling setsockopt after uploading chunk. ideal = "
+                + std::to_string(ideal) + " result = " + std::to_string(result));
+      }
+    }
   }
 }
 #endif
@@ -152,15 +150,18 @@ using Azure::Core::Http::CurlSession;
 using Azure::Core::Http::CurlTransport;
 using Azure::Core::Http::CurlTransportOptions;
 using Azure::Core::Http::HttpStatusCode;
-using Azure::Core::Http::LogClassification;
 using Azure::Core::Http::RawResponse;
 using Azure::Core::Http::Request;
 using Azure::Core::Http::TransportException;
 
 std::unique_ptr<RawResponse> CurlTransport::Send(Context const& context, Request& request)
 {
+  using namespace Azure::Core::Logging;
+  using namespace Azure::Core::Logging::Internal;
+
   // Create CurlSession to perform request
-  LogThis("Creating a new session.");
+  Log(LogLevel::Verbose, LogMsgPrefix + "Creating a new session.");
+
   auto session = std::make_unique<CurlSession>(
       request, CurlConnectionPool::GetCurlConnection(request, m_options), m_options.HttpKeepAlive);
   CURLcode performing;
@@ -191,7 +192,9 @@ std::unique_ptr<RawResponse> CurlTransport::Send(Context const& context, Request
         "Error while sending request. " + std::string(curl_easy_strerror(performing)));
   }
 
-  LogThis("Request completed. Moving response out of session and session to response.");
+  Log(LogLevel::Verbose,
+      LogMsgPrefix + "Request completed. Moving response out of session and session to response.");
+
   // Move Response out of the session
   auto response = session->GetResponse();
   // Move the ownership of the CurlSession (bodyStream) to the response
@@ -201,6 +204,8 @@ std::unique_ptr<RawResponse> CurlTransport::Send(Context const& context, Request
 
 CURLcode CurlSession::Perform(Context const& context)
 {
+  using namespace Azure::Core::Logging;
+  using namespace Azure::Core::Logging::Internal;
 
   // Set the session state
   m_sessionState = SessionState::PERFORM;
@@ -211,13 +216,13 @@ CURLcode CurlSession::Perform(Context const& context)
     auto hostHeader = headers.find("Host");
     if (hostHeader == headers.end())
     {
-      LogThis("No Host in request headers. Adding it");
+      Log(LogLevel::Verbose, LogMsgPrefix + "No Host in request headers. Adding it");
       this->m_request.AddHeader("Host", this->m_request.GetUrl().GetHost());
     }
     auto isContentLengthHeaderInRequest = headers.find("content-length");
     if (isContentLengthHeaderInRequest == headers.end())
     {
-      LogThis("No content-length in headers. Adding it");
+      Log(LogLevel::Verbose, LogMsgPrefix + "No content-length in headers. Adding it");
       this->m_request.AddHeader(
           "content-length", std::to_string(this->m_request.GetBodyStream()->Length()));
     }
@@ -226,21 +231,22 @@ CURLcode CurlSession::Perform(Context const& context)
   // use expect:100 for PUT requests. Server will decide if it can take our request
   if (this->m_request.GetMethod() == HttpMethod::Put)
   {
-    LogThis("Using 100-continue for PUT request");
+    Log(LogLevel::Verbose, LogMsgPrefix + "Using 100-continue for PUT request");
     this->m_request.AddHeader("expect", "100-continue");
   }
 
   // Send request. If the connection assigned to this curlSession is closed or the socket is
   // somehow lost, libcurl will return CURLE_UNSUPPORTED_PROTOCOL
   // (https://curl.haxx.se/libcurl/c/curl_easy_send.html). Return the error back.
-  LogThis("Send request without payload");
+  Log(LogLevel::Verbose, LogMsgPrefix + "Send request without payload");
+
   auto result = SendRawHttp(context);
   if (result != CURLE_OK)
   {
     return result;
   }
 
-  LogThis("Parse server response");
+  Log(LogLevel::Verbose, LogMsgPrefix + "Parse server response");
   ReadStatusLineAndHeadersFromRawResponse(context);
 
   // non-PUT request are ready to be stream at this point. Only PUT request would start an uploading
@@ -251,18 +257,17 @@ CURLcode CurlSession::Perform(Context const& context)
     return result;
   }
 
-  LogThis("Check server response before upload starts");
-
+  Log(LogLevel::Verbose, LogMsgPrefix + "Check server response before upload starts");
   // Check server response from Expect:100-continue for PUT;
   // This help to prevent us from start uploading data when Server can't handle it
   if (this->m_lastStatusCode != HttpStatusCode::Continue)
   {
-    LogThis("Server rejected the upload request");
+    Log(LogLevel::Verbose, LogMsgPrefix + "Server rejected the upload request");
     m_sessionState = SessionState::STREAMING;
     return result; // Won't upload.
   }
 
-  LogThis("Upload payload");
+  Log(LogLevel::Verbose, LogMsgPrefix + "Upload payload");
   if (this->m_bodyStartInBuffer > 0)
   {
     // If internal buffer has more data after the 100-continue means Server return an error.
@@ -280,7 +285,7 @@ CURLcode CurlSession::Perform(Context const& context)
     return result; // will throw transport exception before trying to read
   }
 
-  LogThis("Upload completed. Parse server response");
+  Log(LogLevel::Verbose, LogMsgPrefix + "Upload completed. Parse server response");
   ReadStatusLineAndHeadersFromRawResponse(context);
   // If no throw at this point, the request is ready to stream.
   // If any throw happened before this point, the state will remain as PERFORM.

sdk/core/azure-core/src/http/logging_policy.cpp

@@ -77,12 +77,11 @@ std::unique_ptr<RawResponse> Azure::Core::Http::LoggingPolicy::Send(
     Request& request,
     NextHttpPolicy nextHttpPolicy) const
 {
-  if (Logging::Details::ShouldWrite(LogClassification::Request))
+  if (Logging::Internal::ShouldLog(Logging::LogLevel::Verbose))
   {
-    Logging::Details::Write(LogClassification::Request, GetRequestLogMessage(request));
+    Logging::Internal::Log(Logging::LogLevel::Verbose, GetRequestLogMessage(request));
   }
-
-  if (!Logging::Details::ShouldWrite(LogClassification::Response))
+  else
   {
     return nextHttpPolicy.Send(ctx, request);
   }
@@ -91,8 +90,8 @@ std::unique_ptr<RawResponse> Azure::Core::Http::LoggingPolicy::Send(
   auto response = nextHttpPolicy.Send(ctx, request);
   auto const end = std::chrono::system_clock::now();
 
-  Logging::Details::Write(
-      LogClassification::Response, GetResponseLogMessage(request, *response, end - start));
+  Logging::Internal::Log(
+      Logging::LogLevel::Verbose, GetResponseLogMessage(request, *response, end - start));
 
   return response;
 }

sdk/core/azure-core/src/http/policy.cpp

@@ -7,16 +7,6 @@
 using Azure::Core::Context;
 using namespace Azure::Core::Http;
 
-#ifndef _MSC_VER
-// Non-MSVC compilers do require allocation of statics, even if they are const constexpr.
-// MSVC, on the other hand, has problem if you "redefine" static constexprs.
-Azure::Core::Logging::LogClassification const Azure::Core::Http::LogClassification::Request;
-Azure::Core::Logging::LogClassification const Azure::Core::Http::LogClassification::Response;
-Azure::Core::Logging::LogClassification const Azure::Core::Http::LogClassification::Retry;
-Azure::Core::Logging::LogClassification const
-    Azure::Core::Http::LogClassification::HttpTransportAdapter;
-#endif
-
 // The NextHttpPolicy can't be created from a nullptr because it is a reference. So we don't need to
 // check if m_policies is nullptr.
 std::unique_ptr<RawResponse> NextHttpPolicy::Send(Context const& ctx, Request& req)

sdk/core/azure-core/src/http/retry_policy.cpp

@@ -129,7 +129,7 @@ std::unique_ptr<RawResponse> Azure::Core::Http::RetryPolicy::Send(
     Request& request,
     NextHttpPolicy nextHttpPolicy) const
 {
-  auto const shouldLog = Logging::Details::ShouldWrite(LogClassification::Retry);
+  auto const shouldLog = Logging::Internal::ShouldLog(Logging::LogLevel::Informational);
 
   for (RetryNumber attempt = 1;; ++attempt)
   {
@@ -170,7 +170,7 @@ std::unique_ptr<RawResponse> Azure::Core::Http::RetryPolicy::Send(
       log << "HTTP Retry attempt #" << attempt << " will be made in "
           << std::chrono::duration_cast<std::chrono::milliseconds>(retryAfter).count() << "ms.";
 
-      Logging::Details::Write(LogClassification::Retry, log.str());
+      Logging::Internal::Log(Logging::LogLevel::Informational, log.str());
     }
 
     // Sleep(0) behavior is implementation-defined: it may yield, or may do nothing. Let's make sure

sdk/core/azure-core/src/logging/logging.cpp

@@ -7,72 +7,41 @@
 #include <mutex>
 
 using namespace Azure::Core::Logging;
-using namespace Azure::Core::Logging::Details;
-
-class Azure::Core::Logging::Details::LogClassificationsPrivate {
-public:
-  static LogClassifications const LogClassificationsConstant(bool all)
-  {
-    return LogClassifications(all);
-  }
-
-  static bool IsClassificationEnabled(LogClassifications const& cls, LogClassification const& c)
-  {
-    return cls.m_all || (cls.m_classifications.find(c) != cls.m_classifications.end());
-  }
-};
+using namespace Azure::Core::Logging::Internal;
 
 namespace {
 std::mutex g_loggerMutex;
 LogListener g_logListener(nullptr);
+LogLevel g_logLevel = LogLevel::Verbose;
 
-LogClassifications g_logClassifications(
-    LogClassificationsPrivate::LogClassificationsConstant(true));
-
-LogListener GetLogListener(LogClassification const& classification)
+LogListener GetLogListener(LogLevel level)
 {
-  // lock listener and classifications
   std::lock_guard<std::mutex> loggerLock(g_loggerMutex);
-
-  return (!g_logListener // if no logger is set
-          || LogClassificationsPrivate::IsClassificationEnabled(
-              g_logClassifications, classification))
-      // or if classification is enabled
-      ? g_logListener // return actual listener (may be null)
-      : LogListener(nullptr) // return null listener
-      ;
+  return level <= g_logLevel ? g_logListener : LogListener(nullptr);
 }
 } // namespace
 
-LogClassifications const Azure::Core::Logging::LogClassification::All(
-    LogClassificationsPrivate::LogClassificationsConstant(true));
-
-LogClassifications const Azure::Core::Logging::LogClassification::None(
-    LogClassificationsPrivate::LogClassificationsConstant(false));
-
 void Azure::Core::Logging::SetLogListener(LogListener logListener)
 {
   std::lock_guard<std::mutex> loggerLock(g_loggerMutex);
   g_logListener = std::move(logListener);
 }
 
-void Azure::Core::Logging::SetLogClassifications(LogClassifications logClassifications)
+void Azure::Core::Logging::SetLogLevel(LogLevel level)
 {
   std::lock_guard<std::mutex> loggerLock(g_loggerMutex);
-  g_logClassifications = std::move(logClassifications);
+  g_logLevel = level;
 }
 
-bool Azure::Core::Logging::Details::ShouldWrite(LogClassification const& classification)
+bool Azure::Core::Logging::Internal::ShouldLog(LogLevel level)
 {
-  return GetLogListener(classification) != nullptr;
+  return GetLogListener(level) != nullptr;
 }
 
-void Azure::Core::Logging::Details::Write(
-    LogClassification const& classification,
-    std::string const& message)
+void Azure::Core::Logging::Internal::Log(LogLevel level, std::string const& message)
 {
-  if (auto logListener = GetLogListener(classification))
+  if (auto logListener = GetLogListener(level))
   {
-    logListener(classification, message);
+    logListener(level, message);
   }
 }

sdk/core/azure-core/test/ut/logging.cpp

@@ -1,112 +1,226 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-#include <azure/core/http/pipeline.hpp>
 #include <azure/core/internal/log.hpp>
 #include <azure/core/logging/logging.hpp>
 #include <gtest/gtest.h>
 
-#include <utility>
-#include <vector>
+using namespace Azure::Core::Logging;
+using namespace Azure::Core::Logging::Internal;
 
-using namespace Azure::Core;
-
-namespace {
-typedef std::vector<std::pair<Logging::LogClassification, std::string>> LogArguments;
-
-struct LogRecorder
+TEST(Logging, Defaults)
 {
-  LogArguments Actual;
+  EXPECT_FALSE(ShouldLog(LogLevel::Verbose));
+  EXPECT_FALSE(ShouldLog(LogLevel::Informational));
+  EXPECT_FALSE(ShouldLog(LogLevel::Warning));
+  EXPECT_FALSE(ShouldLog(LogLevel::Error));
 
-  Logging::LogListener LogListener
-      = [&](Logging::LogClassification const& c, std::string const& m) {
-          Actual.push_back(std::make_pair(c, m));
-        };
-};
-} // namespace
+  SetLogListener([](auto, auto) {});
 
-TEST(Logging, allClassifications)
-{
-  LogArguments const Values{
-      {Http::LogClassification::Request, "Request"},
-      {Http::LogClassification::Request, "Request"},
-      {Http::LogClassification::Response, "Response"},
-      {Http::LogClassification::Request, "Request"},
-  };
+  EXPECT_TRUE(ShouldLog(LogLevel::Verbose));
+  EXPECT_TRUE(ShouldLog(LogLevel::Informational));
+  EXPECT_TRUE(ShouldLog(LogLevel::Warning));
+  EXPECT_TRUE(ShouldLog(LogLevel::Error));
 
-  LogRecorder logRecorder;
-  Logging::SetLogListener(logRecorder.LogListener);
+  SetLogListener(nullptr);
 
-  Logging::SetLogClassifications(Logging::LogClassification::All);
-  auto const expected = Values;
-
-  for (auto value : Values)
-  {
-    Logging::Details::Write(value.first, value.second);
-  }
-
-  EXPECT_EQ(logRecorder.Actual, expected);
+  EXPECT_FALSE(ShouldLog(LogLevel::Verbose));
+  EXPECT_FALSE(ShouldLog(LogLevel::Informational));
+  EXPECT_FALSE(ShouldLog(LogLevel::Warning));
+  EXPECT_FALSE(ShouldLog(LogLevel::Error));
 }
 
-TEST(Logging, filteredClassifications)
+TEST(Logging, Levels)
 {
-  LogArguments const Values{
-      {Http::LogClassification::Request, "Request"},
-      {Http::LogClassification::Request, "Request"},
-      {Http::LogClassification::Response, "Response"},
-      {Http::LogClassification::Request, "Request"},
-      {Http::LogClassification::Retry, "Retry"},
-  };
+  SetLogListener([](auto, auto) {});
 
-  LogRecorder logRecorder;
-  Logging::SetLogListener(logRecorder.LogListener);
+  SetLogLevel(LogLevel::Verbose);
+  EXPECT_TRUE(ShouldLog(LogLevel::Verbose));
+  EXPECT_TRUE(ShouldLog(LogLevel::Informational));
+  EXPECT_TRUE(ShouldLog(LogLevel::Warning));
+  EXPECT_TRUE(ShouldLog(LogLevel::Error));
 
-  auto const AllowedClassification1 = Http::LogClassification::Request;
-  auto const AllowedClassification2 = Http::LogClassification::Retry;
-  Logging::SetLogClassifications({AllowedClassification1, AllowedClassification2});
+  SetLogLevel(LogLevel::Informational);
+  EXPECT_FALSE(ShouldLog(LogLevel::Verbose));
+  EXPECT_TRUE(ShouldLog(LogLevel::Informational));
+  EXPECT_TRUE(ShouldLog(LogLevel::Warning));
+  EXPECT_TRUE(ShouldLog(LogLevel::Error));
 
-  LogArguments expected;
-  for (auto value : Values)
+  SetLogLevel(LogLevel::Warning);
+  EXPECT_FALSE(ShouldLog(LogLevel::Verbose));
+  EXPECT_FALSE(ShouldLog(LogLevel::Informational));
+  EXPECT_TRUE(ShouldLog(LogLevel::Warning));
+  EXPECT_TRUE(ShouldLog(LogLevel::Error));
+
+  SetLogLevel(LogLevel::Error);
+  EXPECT_FALSE(ShouldLog(LogLevel::Verbose));
+  EXPECT_FALSE(ShouldLog(LogLevel::Informational));
+  EXPECT_FALSE(ShouldLog(LogLevel::Warning));
+  EXPECT_TRUE(ShouldLog(LogLevel::Error));
+
+  SetLogLevel(LogLevel::Verbose);
+  EXPECT_TRUE(ShouldLog(LogLevel::Verbose));
+  EXPECT_TRUE(ShouldLog(LogLevel::Informational));
+  EXPECT_TRUE(ShouldLog(LogLevel::Warning));
+  EXPECT_TRUE(ShouldLog(LogLevel::Error));
+
+  SetLogListener(nullptr);
+}
+
+TEST(Logging, Message)
+{
+  try
   {
-    if (value.first == AllowedClassification1 || value.first == AllowedClassification2)
+    LogLevel level = LogLevel::Error;
+    std::string message;
+
+    SetLogListener([&](auto lvl, auto msg) {
+      level = lvl;
+      message = msg;
+    });
+
+    SetLogLevel(LogLevel::Verbose);
     {
-      expected.push_back(value);
+      level = LogLevel::Error;
+      message = "";
+
+      Log(LogLevel::Verbose, "Verbose");
+      EXPECT_EQ(level, LogLevel::Verbose);
+      EXPECT_EQ(message, "Verbose");
+
+      Log(LogLevel::Informational, "Informational");
+      EXPECT_EQ(level, LogLevel::Informational);
+      EXPECT_EQ(message, "Informational");
+
+      Log(LogLevel::Warning, "Warning");
+      EXPECT_EQ(level, LogLevel::Warning);
+      EXPECT_EQ(message, "Warning");
+
+      Log(LogLevel::Error, "Error");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "Error");
+    }
+
+    SetLogLevel(LogLevel::Informational);
+    {
+      level = LogLevel::Error;
+      message = "";
+
+      Log(LogLevel::Verbose, "Verbose");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "");
+
+      Log(LogLevel::Informational, "Informational");
+      EXPECT_EQ(level, LogLevel::Informational);
+      EXPECT_EQ(message, "Informational");
+
+      Log(LogLevel::Warning, "Warning");
+      EXPECT_EQ(level, LogLevel::Warning);
+      EXPECT_EQ(message, "Warning");
+
+      Log(LogLevel::Error, "Error");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "Error");
+    }
+
+    SetLogLevel(LogLevel::Warning);
+    {
+      level = LogLevel::Error;
+      message = "";
+
+      Log(LogLevel::Verbose, "Verbose");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "");
+
+      Log(LogLevel::Informational, "Informational");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "");
+
+      Log(LogLevel::Warning, "Warning");
+      EXPECT_EQ(level, LogLevel::Warning);
+      EXPECT_EQ(message, "Warning");
+
+      Log(LogLevel::Error, "Error");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "Error");
+    }
+
+    SetLogLevel(LogLevel::Error);
+    {
+      level = LogLevel::Error;
+      message = "";
+
+      Log(LogLevel::Verbose, "Verbose");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "");
+
+      Log(LogLevel::Informational, "Informational");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "");
+
+      Log(LogLevel::Warning, "Warning");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "");
+
+      level = LogLevel::Verbose;
+
+      Log(LogLevel::Error, "Error");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "Error");
+    }
+
+    // Verify that we can switch back to Verbose
+    SetLogLevel(LogLevel::Verbose);
+    {
+      level = LogLevel::Error;
+      message = "";
+
+      Log(LogLevel::Verbose, "Verbose");
+      EXPECT_EQ(level, LogLevel::Verbose);
+      EXPECT_EQ(message, "Verbose");
+
+      Log(LogLevel::Informational, "Informational");
+      EXPECT_EQ(level, LogLevel::Informational);
+      EXPECT_EQ(message, "Informational");
+
+      Log(LogLevel::Warning, "Warning");
+      EXPECT_EQ(level, LogLevel::Warning);
+      EXPECT_EQ(message, "Warning");
+
+      Log(LogLevel::Error, "Error");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "Error");
+    }
+
+    SetLogListener(nullptr);
+
+    SetLogLevel(LogLevel::Verbose);
+    {
+      level = LogLevel::Error;
+      message = "";
+
+      Log(LogLevel::Verbose, "Verbose");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "");
+
+      Log(LogLevel::Informational, "Informational");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "");
+
+      Log(LogLevel::Warning, "Warning");
+      EXPECT_EQ(level, LogLevel::Error);
+      EXPECT_EQ(message, "");
+
+      level = LogLevel::Verbose;
+
+      Log(LogLevel::Error, "Error");
+      EXPECT_EQ(level, LogLevel::Verbose);
+      EXPECT_EQ(message, "");
     }
   }
-
-  for (auto value : Values)
+  catch (...)
   {
-    EXPECT_EQ(
-        Logging::Details::ShouldWrite(value.first),
-        (value.first == AllowedClassification1 || value.first == AllowedClassification2));
-
-    Logging::Details::Write(value.first, value.second);
+    SetLogListener(nullptr);
+    throw;
   }
-
-  EXPECT_EQ(logRecorder.Actual, expected);
-}
-
-TEST(Logging, noClassifications)
-{
-  LogArguments const Values{
-      {Http::LogClassification::Request, "Request"},
-      {Http::LogClassification::Request, "Request"},
-      {Http::LogClassification::Response, "Response"},
-      {Http::LogClassification::Request, "Request"},
-  };
-
-  LogRecorder logRecorder;
-  Logging::SetLogListener(logRecorder.LogListener);
-
-  auto const AllowedClassification = Http::LogClassification::Request;
-  Logging::SetLogClassifications(Logging::LogClassification::None);
-  LogArguments const expected; // Empty
-
-  for (auto value : Values)
-  {
-    EXPECT_EQ(Logging::Details::ShouldWrite(value.first), false);
-    Logging::Details::Write(value.first, value.second);
-  }
-
-  EXPECT_EQ(logRecorder.Actual, expected);
 }

sdk/core/azure-core/test/ut/pipeline.cpp

@@ -7,7 +7,7 @@
 
 #include <vector>
 
-TEST(Logging, createPipeline)
+TEST(Pipeline, createPipeline)
 {
   // Construct pipeline without exception
   std::vector<std::unique_ptr<Azure::Core::Http::HttpPolicy>> policies;
@@ -16,14 +16,14 @@ TEST(Logging, createPipeline)
   EXPECT_NO_THROW(Azure::Core::Http::HttpPipeline pipeline(policies));
 }
 
-TEST(Logging, createEmptyPipeline)
+TEST(Pipeline, createEmptyPipeline)
 {
   // throw invalid arg for empty policies
   std::vector<std::unique_ptr<Azure::Core::Http::HttpPolicy>> policies;
   EXPECT_THROW(Azure::Core::Http::HttpPipeline pipeline(policies), std::invalid_argument);
 }
 
-TEST(Logging, clonePipeline)
+TEST(Pipeline, clonePipeline)
 {
   // Construct pipeline without exception and clone
   std::vector<std::unique_ptr<Azure::Core::Http::HttpPolicy>> policies;
@@ -33,14 +33,14 @@ TEST(Logging, clonePipeline)
   EXPECT_NO_THROW(Azure::Core::Http::HttpPipeline pipeline2(pipeline));
 }
 
-TEST(Logging, refrefPipeline)
+TEST(Pipeline, refrefPipeline)
 {
   // Construct pipeline without exception
   EXPECT_NO_THROW(Azure::Core::Http::HttpPipeline pipeline(
       std::vector<std::unique_ptr<Azure::Core::Http::HttpPolicy>>(1)));
 }
 
-TEST(Logging, refrefEmptyPipeline)
+TEST(Pipeline, refrefEmptyPipeline)
 {
   // Construct pipeline with invalid exception with move constructor
   EXPECT_THROW(

sdk/keyvault/azure-security-keyvault-keys/sample/get-key/main.cpp

@@ -39,10 +39,7 @@ using namespace Azure::Security::KeyVault::Keys;
 int main()
 {
   Azure::Core::Logging::SetLogListener(
-      [](Azure::Core::Logging::LogClassification const&, std::string const& message) {
-        std::cout << message << std::endl;
-      });
-  Azure::Core::Logging::SetLogClassifications({Azure::Core::Http::LogClassification::Response});
+      [](auto, std::string const& message) { std::cout << message << std::endl; });
 
   auto tenantId = std::getenv("AZURE_KEYVAULT_TENANT_ID");
   auto clientId = std::getenv("AZURE_KEYVAULT_CLIENT_ID");