From 84ffcd716834a452301b51da08435afe08c7f2b1 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Mon, 8 Feb 2021 20:37:08 -0800 Subject: [PATCH] Simplify logging (#1568) Closes #1180. --- sdk/core/azure-core/CHANGELOG.md | 1 + .../azure-core/inc/azure/core/http/policy.hpp | 22 +- .../inc/azure/core/internal/log.hpp | 8 +- .../inc/azure/core/logging/logging.hpp | 141 ++------- sdk/core/azure-core/src/http/curl/curl.cpp | 59 ++-- .../azure-core/src/http/logging_policy.cpp | 11 +- sdk/core/azure-core/src/http/policy.cpp | 10 - sdk/core/azure-core/src/http/retry_policy.cpp | 4 +- sdk/core/azure-core/src/logging/logging.cpp | 53 +--- sdk/core/azure-core/test/ut/logging.cpp | 286 ++++++++++++------ sdk/core/azure-core/test/ut/pipeline.cpp | 10 +- .../sample/get-key/main.cpp | 5 +- 12 files changed, 286 insertions(+), 324 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 606f7b80c..ca3619436 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/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 diff --git a/sdk/core/azure-core/inc/azure/core/http/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policy.hpp index 0e8065c16..79733012b 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policy.hpp +++ b/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 @@ -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. diff --git a/sdk/core/azure-core/inc/azure/core/internal/log.hpp b/sdk/core/azure-core/inc/azure/core/internal/log.hpp index 28a99bf16..370b4a5d2 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/log.hpp +++ b/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 diff --git a/sdk/core/azure-core/inc/azure/core/logging/logging.hpp b/sdk/core/azure-core/inc/azure/core/logging/logging.hpp index 7e24aba92..35772e8b5 100644 --- a/sdk/core/azure-core/inc/azure/core/logging/logging.hpp +++ b/sdk/core/azure-core/inc/azure/core/logging/logging.hpp @@ -9,143 +9,50 @@ #pragma once -#include "azure/core/dll_import_export.hpp" - #include -#include -#include #include -#include 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 - LogListener; + typedef std::function 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 class LogClassificationProvider; - - class LogClassificationsPrivate; - } // namespace Details - - /** - * @brief Represents a set of log classifications. - */ - class LogClassifications { - friend class Details::LogClassificationsPrivate; - - std::set 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 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 set) - : m_classifications(std::move(set)), m_all(false) - { - } - }; - - /** - * @brief Represents a log classification. - */ - class LogClassification { - template friend class Details::LogClassificationProvider; - friend struct std::less; - - int32_t m_value; - - constexpr explicit LogClassification(Details::Facility facility, int16_t number) - : m_value((static_cast(number) << 16) | static_cast(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 class LogClassificationProvider { - protected: - constexpr static auto Classification(int16_t number) { return LogClassification(F, number); } - }; - } // namespace Details + void SetLogLevel(LogLevel level); }}} // namespace Azure::Core::Logging diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp index cee97eefb..7542492f4 100644 --- a/sdk/core/azure-core/src/http/curl/curl.cpp +++ b/sdk/core/azure-core/src/http/curl/curl.cpp @@ -25,17 +25,7 @@ #include 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 #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 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( request, CurlConnectionPool::GetCurlConnection(request, m_options), m_options.HttpKeepAlive); CURLcode performing; @@ -191,7 +192,9 @@ std::unique_ptr 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 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. diff --git a/sdk/core/azure-core/src/http/logging_policy.cpp b/sdk/core/azure-core/src/http/logging_policy.cpp index 29fd7b85f..07dd62429 100644 --- a/sdk/core/azure-core/src/http/logging_policy.cpp +++ b/sdk/core/azure-core/src/http/logging_policy.cpp @@ -77,12 +77,11 @@ std::unique_ptr 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 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; } diff --git a/sdk/core/azure-core/src/http/policy.cpp b/sdk/core/azure-core/src/http/policy.cpp index c9d0f6366..dba03eec1 100644 --- a/sdk/core/azure-core/src/http/policy.cpp +++ b/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 NextHttpPolicy::Send(Context const& ctx, Request& req) diff --git a/sdk/core/azure-core/src/http/retry_policy.cpp b/sdk/core/azure-core/src/http/retry_policy.cpp index 54e470b97..0ac83a05e 100644 --- a/sdk/core/azure-core/src/http/retry_policy.cpp +++ b/sdk/core/azure-core/src/http/retry_policy.cpp @@ -129,7 +129,7 @@ std::unique_ptr 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 Azure::Core::Http::RetryPolicy::Send( log << "HTTP Retry attempt #" << attempt << " will be made in " << std::chrono::duration_cast(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 diff --git a/sdk/core/azure-core/src/logging/logging.cpp b/sdk/core/azure-core/src/logging/logging.cpp index e63785464..12d5fdbef 100644 --- a/sdk/core/azure-core/src/logging/logging.cpp +++ b/sdk/core/azure-core/src/logging/logging.cpp @@ -7,72 +7,41 @@ #include 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 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 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 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); } } diff --git a/sdk/core/azure-core/test/ut/logging.cpp b/sdk/core/azure-core/test/ut/logging.cpp index 5462f82b7..79d1b79eb 100644 --- a/sdk/core/azure-core/test/ut/logging.cpp +++ b/sdk/core/azure-core/test/ut/logging.cpp @@ -1,112 +1,226 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // SPDX-License-Identifier: MIT -#include #include #include #include -#include -#include +using namespace Azure::Core::Logging; +using namespace Azure::Core::Logging::Internal; -using namespace Azure::Core; - -namespace { -typedef std::vector> 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); } diff --git a/sdk/core/azure-core/test/ut/pipeline.cpp b/sdk/core/azure-core/test/ut/pipeline.cpp index b889a0b34..9e2e16cbc 100644 --- a/sdk/core/azure-core/test/ut/pipeline.cpp +++ b/sdk/core/azure-core/test/ut/pipeline.cpp @@ -7,7 +7,7 @@ #include -TEST(Logging, createPipeline) +TEST(Pipeline, createPipeline) { // Construct pipeline without exception std::vector> 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> 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> 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>(1))); } -TEST(Logging, refrefEmptyPipeline) +TEST(Pipeline, refrefEmptyPipeline) { // Construct pipeline with invalid exception with move constructor EXPECT_THROW( diff --git a/sdk/keyvault/azure-security-keyvault-keys/sample/get-key/main.cpp b/sdk/keyvault/azure-security-keyvault-keys/sample/get-key/main.cpp index 1b1000649..35969b94f 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/sample/get-key/main.cpp +++ b/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");