From 622e8da4fefe8c01f8974f5ad1ab38b655829260 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Fri, 5 Mar 2021 12:16:14 -0800 Subject: [PATCH] API Review Feedback: Logging and DateTime updates (#1752) --- CMakeLists.txt | 1 + sdk/core/azure-core/CHANGELOG.md | 5 + sdk/core/azure-core/CMakeLists.txt | 14 +- sdk/core/azure-core/inc/azure/core.hpp | 4 +- ...ap.hpp => case_insensitive_containers.hpp} | 6 + .../azure-core/inc/azure/core/datetime.hpp | 39 +--- .../azure-core/inc/azure/core/http/http.hpp | 29 ++- .../azure-core/inc/azure/core/http/policy.hpp | 60 +++-- .../azure/core/internal/client_options.hpp | 9 +- .../inc/azure/core/internal/http/pipeline.hpp | 4 +- .../inc/azure/core/internal/log.hpp | 38 ++- sdk/core/azure-core/inc/azure/core/logger.hpp | 68 ++++++ .../inc/azure/core/logging/logging.hpp | 58 ----- sdk/core/azure-core/src/datetime.cpp | 19 +- .../src/environment_log_level_listener.cpp | 148 ++++++++++++ ...environment_log_level_listener_private.hpp | 40 ++++ sdk/core/azure-core/src/http/curl/curl.cpp | 45 ++-- sdk/core/azure-core/src/http/log_policy.cpp | 146 ++++++++++++ .../azure-core/src/http/logging_policy.cpp | 97 -------- sdk/core/azure-core/src/http/retry_policy.cpp | 43 +++- sdk/core/azure-core/src/http/url.cpp | 57 ++--- sdk/core/azure-core/src/logger.cpp | 52 +++++ sdk/core/azure-core/src/logging/logging.cpp | 47 ---- sdk/core/azure-core/test/ut/CMakeLists.txt | 2 +- ...ap.cpp => case_insensitive_containers.cpp} | 2 +- sdk/core/azure-core/test/ut/datetime.cpp | 20 +- sdk/core/azure-core/test/ut/logging.cpp | 221 ++++++++---------- .../azure-core/test/ut/simplified_header.cpp | 3 + .../blobs/protocol/blob_rest_client.hpp | 17 +- .../src/blob_sas_builder.cpp | 32 +-- .../azure/storage/common/storage_common.hpp | 2 +- .../src/account_sas_builder.cpp | 12 +- .../src/datalake_sas_builder.cpp | 32 +-- .../shares/protocol/share_rest_client.hpp | 10 +- .../src/share_directory_client.cpp | 18 +- .../src/share_file_client.cpp | 42 ++-- .../src/share_sas_builder.cpp | 14 +- 37 files changed, 924 insertions(+), 532 deletions(-) rename sdk/core/azure-core/inc/azure/core/{case_insensitive_map.hpp => case_insensitive_containers.hpp} (72%) create mode 100644 sdk/core/azure-core/inc/azure/core/logger.hpp delete mode 100644 sdk/core/azure-core/inc/azure/core/logging/logging.hpp create mode 100644 sdk/core/azure-core/src/environment_log_level_listener.cpp create mode 100644 sdk/core/azure-core/src/environment_log_level_listener_private.hpp create mode 100644 sdk/core/azure-core/src/http/log_policy.cpp delete mode 100644 sdk/core/azure-core/src/http/logging_policy.cpp create mode 100644 sdk/core/azure-core/src/logger.cpp delete mode 100644 sdk/core/azure-core/src/logging/logging.cpp rename sdk/core/azure-core/test/ut/{case_insensitive_map.cpp => case_insensitive_containers.cpp} (92%) diff --git a/CMakeLists.txt b/CMakeLists.txt index 11e2c2bd0..577158ef4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,6 +19,7 @@ option(RUN_LONG_UNIT_TESTS "Tests that takes more than 5 minutes to complete. No option(BUILD_STORAGE_SAMPLES "Build sample application for Azure Storage clients" OFF) option(BUILD_PERFORMANCE_TESTS "Build the performance test library" OFF) option(MSVC_USE_STATIC_CRT "(MSVC only) Set to ON to link SDK with static CRT (/MT or /MTd switch)." OFF) +option(BUILD_ENV_LOGGER "Build support for enabling logging to console when AZURE_LOG_LEVEL environment variable is set." ON) include(AzureTransportAdapters) include(AzureVcpkg) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 0b1c80f09..a0eb92a78 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -22,6 +22,11 @@ - Renamed `TelemetryPolicyOptions` to `TelemetryOptions`. - Renamed `ValuePolicyOptions` to `ValueOptions`. - Removed `StartTry()` from `Azure::Core::Http::Request`. +- Changed type of `Azure::Core::Http::RetryOptions::StatusCodes` from `std::vector` to `std::set`. +- Renamed `Azure::Core::Http::LoggingPolicy` to `LogPolicy`. +- Introduced `Azure::Core::Http::LogOptions`, a mandatory parameter for `LogPolicy` construction. Entities that are not specified in the allow lists are hidden in the log. +- Moved `Azure::Core::Logging` namespace entities to `Azure::Core::Logger` class. +- Removed `Azure::Core::DateTime::GetRfc3339String()`: `Azure::Core::DateTime::ToString()` was extended to provide the same functionality. ### Bug Fixes diff --git a/sdk/core/azure-core/CMakeLists.txt b/sdk/core/azure-core/CMakeLists.txt index 54dbc7fda..453dd527c 100644 --- a/sdk/core/azure-core/CMakeLists.txt +++ b/sdk/core/azure-core/CMakeLists.txt @@ -21,6 +21,10 @@ az_vcpkg_integrate() find_package(Threads REQUIRED) +if (NOT BUILD_ENV_LOGGER) + add_compile_definitions(AZ_NO_ENV_LOGGER) +endif() + if(BUILD_TRANSPORT_CURL) # min version for `CURLSSLOPT_NO_REVOKE` # https://curl.haxx.se/libcurl/c/CURLOPT_SSL_OPTIONS.html @@ -59,15 +63,15 @@ set( inc/azure/core/internal/null_body_stream.hpp inc/azure/core/internal/strings.hpp inc/azure/core/io/body_stream.hpp - inc/azure/core/logging/logging.hpp inc/azure/core/base64.hpp - inc/azure/core/case_insensitive_map.hpp + inc/azure/core/case_insensitive_containers.hpp inc/azure/core/context.hpp inc/azure/core/credentials.hpp inc/azure/core/datetime.hpp inc/azure/core/dll_import_export.hpp inc/azure/core/etag.hpp inc/azure/core/exception.hpp + inc/azure/core/logger.hpp inc/azure/core/match_conditions.hpp inc/azure/core/modified_conditions.hpp inc/azure/core/nullable.hpp @@ -87,7 +91,7 @@ set( src/cryptography/md5.cpp src/http/bearer_token_authentication_policy.cpp src/http/http.cpp - src/http/logging_policy.cpp + src/http/log_policy.cpp src/http/policy.cpp src/http/raw_response.cpp src/http/request.cpp @@ -96,10 +100,12 @@ set( src/http/transport_policy.cpp src/http/url.cpp src/io/body_stream.cpp - src/logging/logging.cpp src/base64.cpp src/context.cpp src/datetime.cpp + src/environment_log_level_listener.cpp + src/environment_log_level_listener_private.hpp + src/logger.cpp src/operation_status.cpp src/strings.cpp src/version.cpp diff --git a/sdk/core/azure-core/inc/azure/core.hpp b/sdk/core/azure-core/inc/azure/core.hpp index 2de550c62..dc626297c 100644 --- a/sdk/core/azure-core/inc/azure/core.hpp +++ b/sdk/core/azure-core/inc/azure/core.hpp @@ -18,6 +18,7 @@ #include "azure/core/dll_import_export.hpp" #include "azure/core/etag.hpp" #include "azure/core/exception.hpp" +#include "azure/core/logger.hpp" #include "azure/core/match_conditions.hpp" #include "azure/core/modified_conditions.hpp" #include "azure/core/nullable.hpp" @@ -38,6 +39,3 @@ // azure/core/io #include "azure/core/io/body_stream.hpp" - -// azure/core/logging -#include "azure/core/logging/logging.hpp" diff --git a/sdk/core/azure-core/inc/azure/core/case_insensitive_map.hpp b/sdk/core/azure-core/inc/azure/core/case_insensitive_containers.hpp similarity index 72% rename from sdk/core/azure-core/inc/azure/core/case_insensitive_map.hpp rename to sdk/core/azure-core/inc/azure/core/case_insensitive_containers.hpp index dc1119ba7..cba2e72db 100644 --- a/sdk/core/azure-core/inc/azure/core/case_insensitive_map.hpp +++ b/sdk/core/azure-core/inc/azure/core/case_insensitive_containers.hpp @@ -11,6 +11,7 @@ #include "azure/core/internal/strings.hpp" #include +#include #include namespace Azure { namespace Core { @@ -22,4 +23,9 @@ namespace Azure { namespace Core { using CaseInsensitiveMap = std::map; + /** + * @brief A type alias of `std::set` with case-insensitive element comparison. + */ + using CaseInsensitiveSet = std::set; + }} // namespace Azure::Core diff --git a/sdk/core/azure-core/inc/azure/core/datetime.hpp b/sdk/core/azure-core/inc/azure/core/datetime.hpp index 9324f4d8c..5498bb5f2 100644 --- a/sdk/core/azure-core/inc/azure/core/datetime.hpp +++ b/sdk/core/azure-core/inc/azure/core/datetime.hpp @@ -11,6 +11,7 @@ #include "azure/core/dll_import_export.hpp" #include +#include #include namespace Azure { namespace Core { @@ -33,8 +34,8 @@ namespace Azure { namespace Core { // It would not be possible to base this clock on steady_clock and provide an implementation // that universally works in any context in predictable manner. However, it does not mean that // implementation can't use steady_clock in conjunction with this clock: an author can get a - // duration beteen two time_points of this clock (or between system_clock::time point ant this - // clock's time_point), and add that duration to steady clock's time_point to get a new + // duration between two time_points of this clock (or between system_clock::time point at + // this clock's time_point), and add that duration to steady clock's time_point to get a new // time_point in the steady clock's "coordinate system". static constexpr bool is_steady = std::chrono::system_clock::is_steady; static time_point now() noexcept; @@ -162,42 +163,18 @@ namespace Azure { namespace Core { */ static DateTime Parse(std::string const& dateTime, DateFormat format); - private: /** * @brief Get a string representation of the #Azure::Core::DateTime. * * @param format The representation format to use. - * @param fractionFormat The format for the fraction part of the Datetime. Only supported by - * RFC 3339. + * @param fractionFormat The format for the fraction part of the DateTime. Only + * supported by RFC3339. * * @throw std::invalid_argument If year exceeds 9999, or if \p format is not recognized. */ - std::string ToString(DateFormat format, TimeFractionFormat fractionFormat) const; - - public: - /** - * @brief Get a string representation of the #Azure::Core::DateTime. - * - * @param format The representation format to use. - * - * @throw std::invalid_argument If year exceeds 9999, or if \p format is not recognized. - */ - std::string ToString(DateFormat format) const - { - return ToString(format, TimeFractionFormat::DropTrailingZeros); - }; - - /** - * @brief Get a string representation of the #Azure::Core::DateTime formatted with RFC 3339. - * - * @param fractionFormat The format that is applied to the fraction part from the RFC 3339 date. - * - * @throw std::invalid_argument If year exceeds 9999, or if \p format is not recognized. - */ - std::string GetRfc3339String(TimeFractionFormat fractionFormat) const - { - return ToString(DateFormat::Rfc3339, fractionFormat); - }; + std::string ToString( + DateFormat format, + TimeFractionFormat fractionFormat = TimeFractionFormat::DropTrailingZeros) const; }; inline Details::Clock::time_point Details::Clock::now() noexcept diff --git a/sdk/core/azure-core/inc/azure/core/http/http.hpp b/sdk/core/azure-core/inc/azure/core/http/http.hpp index 5528ed7b4..75f6110a5 100644 --- a/sdk/core/azure-core/inc/azure/core/http/http.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/http.hpp @@ -8,7 +8,7 @@ #pragma once -#include "azure/core/case_insensitive_map.hpp" +#include "azure/core/case_insensitive_containers.hpp" #include "azure/core/exception.hpp" #include "azure/core/internal/contract.hpp" #include "azure/core/io/body_stream.hpp" @@ -52,6 +52,25 @@ namespace Azure { namespace Core { namespace Http { CaseInsensitiveMap& headers, std::string const& headerName, std::string const& headerValue); + + inline std::string FormatEncodedUrlQueryParameters( + std::map const& encodedQueryParameters) + { + { + std::string queryStr; + if (!encodedQueryParameters.empty()) + { + auto separ = '?'; + for (const auto& q : encodedQueryParameters) + { + queryStr += separ + q.first + '=' + q.second; + separ = '&'; + } + } + + return queryStr; + } + } } // namespace Details /********************* Exceptions **********************/ @@ -260,6 +279,8 @@ namespace Azure { namespace Core { namespace Http { // this set. const static std::unordered_set defaultNonUrlEncodeChars; + std::string GetUrlWithoutQuery(bool relative) const; + public: /** * @brief Decodes \p value by transforming all escaped characters to it's non-encoded value. @@ -424,6 +445,12 @@ namespace Azure { namespace Core { namespace Http { return m_encodedQueryParameters; } + /** + * @brief Get Scheme, host, and path, without query parameters. + * @return Absolute URL without query parameters. + */ + std::string GetUrlWithoutQuery() const { return GetUrlWithoutQuery(false); } + /** * @brief Get the path and query parameters. * 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 22e2e40ed..c2aec0dac 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policy.hpp @@ -8,9 +8,10 @@ #pragma once -#include "azure/core/case_insensitive_map.hpp" +#include "azure/core/case_insensitive_containers.hpp" #include "azure/core/context.hpp" #include "azure/core/credentials.hpp" +#include "azure/core/dll_import_export.hpp" #include "azure/core/http/http.hpp" #include "azure/core/http/transport.hpp" #include "azure/core/uuid.hpp" @@ -20,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -102,12 +104,12 @@ namespace Azure { namespace Core { namespace Http { public: /** - * @brief Construct an abstraction representing a next line in the stack sequence of policies, - * from the caller's perspective. + * @brief Construct an abstraction representing a next line in the stack sequence of + * policies, from the caller's perspective. * * @param index An sequential index of this policy in the stack sequence of policies. - * @param policies A vector of unique pointers next in the line to be invoked after the current - * policy. + * @param policies A vector of unique pointers next in the line to be invoked after the + * current policy. */ explicit NextHttpPolicy( std::size_t index, @@ -138,8 +140,8 @@ namespace Azure { namespace Core { namespace Http { * @brief Set the #Azure::Core::Http::HttpTransport that the transport policy will use to send * and receive requests and responses over the wire. * - * @remark When no option is set, the default transport adapter on non-Windows platforms is the - * curl transport adapter and winhttp transport adapter on Windows. + * @remark When no option is set, the default transport adapter on non-Windows platforms is + * the curl transport adapter and winhttp transport adapter on Windows. * * @remark When using a custom transport adapter, the implementation for * `AzureSdkGetCustomHttpTransport` must be linked in the end-user application. @@ -201,7 +203,7 @@ namespace Azure { namespace Core { namespace Http { /** * @brief HTTP status codes to retry on. */ - std::vector StatusCodes{ + std::set StatusCodes{ HttpStatusCode::RequestTimeout, HttpStatusCode::InternalServerError, HttpStatusCode::BadGateway, @@ -239,9 +241,9 @@ namespace Azure { namespace Core { namespace Http { * @brief Get the Retry Count from the context. * * @remark The sentinel `-1` is returned if there is no information in the \p Context about - * #RetryPolicy is trying to send a request. Then `0` is returned for the first try of sending a - * request by the #RetryPolicy. Any subsequent retry will be referenced with a number greater - * than 0. + * #RetryPolicy is trying to send a request. Then `0` is returned for the first try of sending + * a request by the #RetryPolicy. Any subsequent retry will be referenced with a number + * greater than 0. * * @param context The context used to call send request. * @return A positive number indicating the current intent to send the request. @@ -252,8 +254,8 @@ namespace Azure { namespace Core { namespace Http { /** * @brief HTTP Request ID policy. * - * @details Applies an HTTP header with a unique ID to each HTTP request, so that each individual - * request can be traced for troubleshooting. + * @details Applies an HTTP header with a unique ID to each HTTP request, so that each + * individual request can be traced for troubleshooting. */ class RequestIdPolicy : public HttpPolicy { private: @@ -390,22 +392,44 @@ namespace Azure { namespace Core { namespace Http { NextHttpPolicy policy) const override; }; + namespace Details { + AZ_CORE_DLLEXPORT extern Azure::Core::CaseInsensitiveSet g_defaultAllowedHttpHeaders; + } + + /** + * @brief Options for Azure::Core::Http::LogPolicy. + */ + struct LogOptions + { + /** + * @brief HTTP query parameters that are allowed to be logged. + */ + std::set AllowedHttpQueryParameters; + + /** + * @brief HTTP headers that are allowed to be logged. + */ + Azure::Core::CaseInsensitiveSet AllowedHttpHeaders = Details::g_defaultAllowedHttpHeaders; + }; + /** * @brief Logs every HTTP request. * - * @details Logs every HTTP request, response, or retry attempt. - * @remark See #logging.hpp + * @details Logs every HTTP request and response. + * @remark See Azure::Core::Logger. */ - class LoggingPolicy : public HttpPolicy { + class LogPolicy : public HttpPolicy { + LogOptions m_options; + public: /** * @brief Constructs HTTP logging policy. */ - explicit LoggingPolicy() {} + explicit LogPolicy(LogOptions options) : m_options(std::move(options)) {} std::unique_ptr Clone() const override { - return std::make_unique(*this); + return std::make_unique(*this); } std::unique_ptr Send( diff --git a/sdk/core/azure-core/inc/azure/core/internal/client_options.hpp b/sdk/core/azure-core/inc/azure/core/internal/client_options.hpp index 8d4f43045..d52832a7c 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/client_options.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/client_options.hpp @@ -48,7 +48,8 @@ namespace Azure { namespace Core { namespace Internal { * */ ClientOptions(ClientOptions const& options) - : Retry(options.Retry), Transport(options.Transport), Telemetry(options.Telemetry) + : Retry(options.Retry), Transport(options.Transport), Telemetry(options.Telemetry), + Log(options.Log) { PerOperationPolicies.reserve(options.PerOperationPolicies.size()); for (auto& policy : options.PerOperationPolicies) @@ -78,6 +79,12 @@ namespace Azure { namespace Core { namespace Internal { * @brief Telemetry options. */ Azure::Core::Http::TelemetryOptions Telemetry; + + /** + * @brief Define the information to be used for logging. + * + */ + Azure::Core::Http::LogOptions Log; }; }}} // namespace Azure::Core::Internal diff --git a/sdk/core/azure-core/inc/azure/core/internal/http/pipeline.hpp b/sdk/core/azure-core/inc/azure/core/internal/http/pipeline.hpp index 91651c971..d5b59c21e 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/http/pipeline.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/http/pipeline.hpp @@ -82,7 +82,7 @@ namespace Azure { namespace Core { namespace Internal { namespace Http { // - TelemetryPolicy // - RequestIdPolicy // - RetryPolicy - // - LoggingPolicy + // - LogPolicy // - TransportPolicy auto pipelineSize = perCallClientPolicies.size() + perRetryClientPolicies.size() + perRetryPolicies.size() + perCallPolicies.size() + 5; @@ -122,7 +122,7 @@ namespace Azure { namespace Core { namespace Internal { namespace Http { } // logging - won't update request - m_policies.emplace_back(std::make_unique()); + m_policies.emplace_back(std::make_unique(clientOptions.Log)); // transport m_policies.emplace_back( 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 370b4a5d2..b15ab96ef 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/log.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/log.hpp @@ -3,9 +3,37 @@ #pragma once -#include "azure/core/logging/logging.hpp" +#include "azure/core/dll_import_export.hpp" +#include "azure/core/logger.hpp" -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 +#include +#include + +namespace Azure { namespace Core { namespace Internal { + class Log { + using LogLevelInt = std::underlying_type::type; + + static_assert( + std::is_same::value == true && ATOMIC_INT_LOCK_FREE == 2, + "Logger::Level values must be representable as lock-free"); + + static_assert(ATOMIC_BOOL_LOCK_FREE == 2, "atomic must be lock-free"); + + static AZ_CORE_DLLEXPORT std::atomic g_isLoggingEnabled; + static AZ_CORE_DLLEXPORT std::atomic g_logLevel; + + Log() = delete; + ~Log() = delete; + + public: + static bool ShouldWrite(Logger::Level level) + { + return g_isLoggingEnabled && static_cast(level) >= g_logLevel; + } + + static void Write(Logger::Level level, std::string const& message); + + static void EnableLogging(bool isEnabled); + static void SetLogLevel(Logger::Level logLevel); + }; +}}} // namespace Azure::Core::Internal diff --git a/sdk/core/azure-core/inc/azure/core/logger.hpp b/sdk/core/azure-core/inc/azure/core/logger.hpp new file mode 100644 index 000000000..91f80db7b --- /dev/null +++ b/sdk/core/azure-core/inc/azure/core/logger.hpp @@ -0,0 +1,68 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +/** + * @file + * @brief This header defines the types and functions your application uses to be notified of Azure + * SDK client library log messages. + */ + +#pragma once + +#include +#include + +namespace Azure { namespace Core { + /** + * @brief Log message handler. + */ + class Logger { + public: + /** + * @brief Log message level. + */ + // https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/LogLevel.java + enum class Level : int + { + /// Logging level for detailed troubleshooting scenarios. + Verbose = 1, + + /// Logging level when a function operates normally. + Informational = 2, + + /// Logging level when a function fails to perform its intended task. + Warning = 3, + + /// Logging level for failures that the application is unlikely to recover from. + Error = 4, + }; + + /** + * @brief Defines the signature of the callback function that application developers must write + * in order to receive Azure SDK log messages. + * + * @param level The log message level. + * @param message The log message. + */ + typedef std::function Listener; + + /** + * @brief Set the function that will be invoked to report an SDK log message. + * + * @param listener An #Azure::Core::Logger::Listener function that will be invoked when + * the SDK reports a log message. If `nullptr`, no function will be invoked. + */ + static void SetListener(Listener listener); + + /** + * @brief Sets the #Azure::Core::Logger::Level an application is interested in receiving. + * + * @param level Maximum log level. + */ + static void SetLevel(Level level); + + private: + Logger() = delete; + ~Logger() = delete; + }; +}} // namespace Azure::Core diff --git a/sdk/core/azure-core/inc/azure/core/logging/logging.hpp b/sdk/core/azure-core/inc/azure/core/logging/logging.hpp deleted file mode 100644 index 0ff92603e..000000000 --- a/sdk/core/azure-core/inc/azure/core/logging/logging.hpp +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// SPDX-License-Identifier: MIT - -/** - * @file - * @brief This header defines the types and functions your application uses to be notified of Azure - * SDK client library log messages. - */ - -#pragma once - -#include -#include - -namespace Azure { namespace Core { namespace Logging { - /** - * @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 level The log message level. - * @param message The log message. - */ - typedef std::function LogListener; - - /** - * @brief Set the function that will be invoked to report an SDK log message. - * - * @param logListener A #Azure::Core::Logging::LogListener function that will be invoked when the - * SDK reports a log message matching one of the log levels passed to - * #Azure::Core::Logging::SetLogLevel(). If `nullptr`, no function will be invoked. - */ - void SetLogListener(LogListener logListener); - - /** - * @brief Sets the #Azure::Core::Logging::LogLevel an application is interested in receiving. - * - * @param level Maximum log level. - */ - void SetLogLevel(LogLevel level); -}}} // namespace Azure::Core::Logging diff --git a/sdk/core/azure-core/src/datetime.cpp b/sdk/core/azure-core/src/datetime.cpp index 5e137a0e6..468a252df 100644 --- a/sdk/core/azure-core/src/datetime.cpp +++ b/sdk/core/azure-core/src/datetime.cpp @@ -73,11 +73,16 @@ constexpr bool IsLeapYear(int16_t year) return (year % 4 == 0 && (year % 100 != 0 || year % 400 == 0)); } -constexpr int16_t LeapYarsSinceEpoch(int16_t year) +constexpr int16_t LeapYearsSinceEpoch(int16_t year) { int16_t const fourHundredYearPeriods = year / 400; int16_t const hundredYearPeriods = (year % 400) / 100; int16_t const remainder = year - (400 * fourHundredYearPeriods) - 100 * (hundredYearPeriods); + + // Every 4 years have 1 leap year. + // Every 100 years have 24, not 25 leap years (years divisible by 100 are not leap years). + // Every 400 years have 97, not 96 (would be 24 * 4 = 96) years. + // That's because year divisible by 100 is not a leap year, unless it is also divisible by 400. return (fourHundredYearPeriods * 97) + (hundredYearPeriods * 24) + (remainder / 4); } @@ -102,7 +107,7 @@ constexpr int16_t DayOfYear(int16_t year, int8_t month, int8_t day) constexpr int32_t DaySinceEpoch(int16_t year, int8_t month, int8_t day) { int16_t const priorYear = year - 1; - auto const leapYears = LeapYarsSinceEpoch(priorYear); + auto const leapYears = LeapYearsSinceEpoch(priorYear); auto const nonLeapYears = priorYear - leapYears; return (nonLeapYears * 365) + (leapYears * 366) + DayOfYear(year, month, day); } @@ -112,11 +117,17 @@ constexpr int8_t GetDayOfWeek(int16_t year, int8_t month, int8_t day) return DaySinceEpoch(year, month, day) % 7; } -constexpr int8_t WeekDayMonthDayOfYear(int8_t* month, int8_t* day, int16_t year, int16_t dayOfYear) +constexpr int8_t WeekDayAndMonthDayOfYear( + int8_t* month, + int8_t* day, + int16_t year, + int16_t dayOfYear) { auto remainder = dayOfYear; for (int8_t i = 1; i <= 12; ++i) { + // MonthDays = number of days in this month. + // If this month is February, then check for leap year and adjust accordingly. int8_t const MonthDays = MaxDaysPerMonth[i - 1] - ((i == 2 && !IsLeapYear(year)) ? 1 : 0); if (remainder <= MonthDays) { @@ -780,7 +791,7 @@ std::string DateTime::ToString(DateFormat format, TimeFractionFormat fractionFor year += static_cast((years400 * 400) + (years100 * 100) + (years4 * 4) + years1); - dayOfWeek = WeekDayMonthDayOfYear( + dayOfWeek = WeekDayAndMonthDayOfYear( &month, &day, year, diff --git a/sdk/core/azure-core/src/environment_log_level_listener.cpp b/sdk/core/azure-core/src/environment_log_level_listener.cpp new file mode 100644 index 000000000..1053dc0ab --- /dev/null +++ b/sdk/core/azure-core/src/environment_log_level_listener.cpp @@ -0,0 +1,148 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#if defined(AZ_NO_ENV_LOGGER) +#include "azure/core/platform.hpp" + +#if defined(AZ_PLATFORM_WINDOWS) +#if !defined(WIN32_LEAN_AND_MEAN) +#define WIN32_LEAN_AND_MEAN +#endif +#if !defined(NOMINMAX) +#define NOMINMAX +#endif + +#include +#endif +#endif + +#if !defined(AZ_NO_ENV_LOGGER) \ + && (!defined(WINAPI_PARTITION_DESKTOP) \ + || WINAPI_PARTITION_DESKTOP) // See azure/core/platform.hpp for explanation. + +#include "environment_log_level_listener_private.hpp" + +#include "azure/core/datetime.hpp" +#include "azure/core/internal/strings.hpp" + +#include +#include + +using namespace Azure::Core; +using namespace Azure::Core::Details; + +namespace { +Logger::Level const* GetEnvironmentLogLevel() +{ + static Logger::Level* envLogLevelPtr = nullptr; + + static bool initialized = false; + if (!initialized) + { + initialized = true; + +#if defined(_MSC_VER) +#pragma warning(push) +// warning C4996: 'getenv': This function or variable may be unsafe. Consider using _dupenv_s +// instead. +#pragma warning(disable : 4996) +#endif + auto envVar = std::getenv("AZURE_LOG_LEVEL"); +#if defined(_MSC_VER) +#pragma warning(pop) +#endif + + if (envVar) + { + auto const logLevelStr = Internal::Strings::ToLower(envVar); + + // See https://github.com/Azure/azure-sdk-for-java/wiki/Logging-with-Azure-SDK + // And + // https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/core/azure-core/src/main/java/com/azure/core/util/logging/LogLevel.java + + static Logger::Level envLogLevel = {}; + envLogLevelPtr = &envLogLevel; + + if (logLevelStr == "error" || logLevelStr == "err" || logLevelStr == "4") + { + envLogLevel = Logger::Level::Error; + } + else if (logLevelStr == "warning" || logLevelStr == "warn" || logLevelStr == "3") + { + envLogLevel = Logger::Level::Warning; + } + else if ( + logLevelStr == "informational" || logLevelStr == "info" || logLevelStr == "information" + || logLevelStr == "2") + { + envLogLevel = Logger::Level::Informational; + } + else if (logLevelStr == "verbose" || logLevelStr == "debug" || logLevelStr == "1") + { + envLogLevel = Logger::Level::Verbose; + } + else + { + envLogLevelPtr = nullptr; + } + } + } + + return envLogLevelPtr; +} + +// Log level textual representation, including space padding, matches slf4j and log4net. +std::string const ErrorText = "ERROR"; +std::string const WarningText = "WARN "; +std::string const InformationalText = "INFO "; +std::string const VerboseText = "DEBUG"; +std::string const UnknownText = "?????"; + +inline std::string const& LogLevelToConsoleString(Logger::Level logLevel) +{ + switch (logLevel) + { + case Logger::Level::Error: + return ErrorText; + + case Logger::Level::Warning: + return WarningText; + + case Logger::Level::Informational: + return InformationalText; + + case Logger::Level::Verbose: + return VerboseText; + + default: + return UnknownText; + }; +} +} // namespace + +Logger::Level EnvironmentLogLevelListener::GetLogLevel(Logger::Level defaultValue) +{ + auto const envLogLevelPtr = GetEnvironmentLogLevel(); + return envLogLevelPtr ? *envLogLevelPtr : defaultValue; +} + +Logger::Listener EnvironmentLogLevelListener::GetLogListener() +{ + bool const isEnvLogLevelSet = GetEnvironmentLogLevel() != nullptr; + if (!isEnvLogLevelSet) + { + return nullptr; + } + + static Logger::Listener const consoleLogger = [](auto level, auto message) { + std::cerr << '[' + << Azure::Core::DateTime(std::chrono::system_clock::now()) + .ToString( + DateTime::DateFormat::Rfc3339, DateTime::TimeFractionFormat::AllDigits) + << "] " << LogLevelToConsoleString(level) << " : " << message << std::endl; + }; + + return consoleLogger; +} + +#endif diff --git a/sdk/core/azure-core/src/environment_log_level_listener_private.hpp b/sdk/core/azure-core/src/environment_log_level_listener_private.hpp new file mode 100644 index 000000000..ae284e441 --- /dev/null +++ b/sdk/core/azure-core/src/environment_log_level_listener_private.hpp @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#pragma once + +#include "azure/core/logger.hpp" + +#if defined(AZ_PLATFORM_WINDOWS) +#if !defined(WIN32_LEAN_AND_MEAN) +#define WIN32_LEAN_AND_MEAN +#endif +#if !defined(NOMINMAX) +#define NOMINMAX +#endif + +#include +#endif + +namespace Azure { namespace Core { namespace Details { + + class EnvironmentLogLevelListener { + EnvironmentLogLevelListener() = delete; + ~EnvironmentLogLevelListener() = delete; + + public: + static Logger::Level GetLogLevel(Logger::Level defaultValue); + static Logger::Listener GetLogListener(); + }; + +#if defined(AZ_NO_ENV_LOGGER) \ + || (defined(WINAPI_PARTITION_DESKTOP) \ + && !WINAPI_PARTITION_DESKTOP) // See azure/core/platform.hpp for explanation. + inline Logger::Level EnvironmentLogLevelListener::GetLogLevel(Logger::Level defaultValue) + { + return defaultValue; + } + + inline Logger::Listener EnvironmentLogLevelListener::GetLogListener() { return nullptr; } +#endif +}}} // namespace Azure::Core::Details diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp index fc22292e6..76172401e 100644 --- a/sdk/core/azure-core/src/http/curl/curl.cpp +++ b/sdk/core/azure-core/src/http/curl/curl.cpp @@ -110,6 +110,9 @@ int pollSocketUntilEventOrTimeout( return result; } +using Azure::Core::Logger; +using Azure::Core::Internal::Log; + #if defined(AZ_PLATFORM_WINDOWS) // Windows needs this after every write to socket or performance would be reduced to 1/4 for // uploading operation. @@ -127,15 +130,12 @@ void WinSocketSetBuffSize(curl_socket_t socket) // 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)); + if (Log::ShouldWrite(Logger::Level::Verbose)) { - 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)); - } + Log::Write( + Logger::Level::Verbose, + LogMsgPrefix + "Windows - calling setsockopt after uploading chunk. ideal = " + + std::to_string(ideal) + " result = " + std::to_string(result)); } } } @@ -156,11 +156,8 @@ 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 - Log(LogLevel::Verbose, LogMsgPrefix + "Creating a new session."); + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Creating a new session."); auto session = std::make_unique( request, CurlConnectionPool::GetCurlConnection(request, m_options), m_options.HttpKeepAlive); @@ -192,7 +189,8 @@ std::unique_ptr CurlTransport::Send(Context const& context, Request "Error while sending request. " + std::string(curl_easy_strerror(performing))); } - Log(LogLevel::Verbose, + Log::Write( + Logger::Level::Verbose, LogMsgPrefix + "Request completed. Moving response out of session and session to response."); // Move Response out of the session @@ -204,9 +202,6 @@ 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; @@ -216,13 +211,13 @@ CURLcode CurlSession::Perform(Context const& context) auto hostHeader = headers.find("Host"); if (hostHeader == headers.end()) { - Log(LogLevel::Verbose, LogMsgPrefix + "No Host in request headers. Adding it"); + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "No Host in request headers. Adding it"); this->m_request.SetHeader("Host", this->m_request.GetUrl().GetHost()); } auto isContentLengthHeaderInRequest = headers.find("content-length"); if (isContentLengthHeaderInRequest == headers.end()) { - Log(LogLevel::Verbose, LogMsgPrefix + "No content-length in headers. Adding it"); + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "No content-length in headers. Adding it"); this->m_request.SetHeader( "content-length", std::to_string(this->m_request.GetBodyStream()->Length())); } @@ -231,14 +226,14 @@ 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) { - Log(LogLevel::Verbose, LogMsgPrefix + "Using 100-continue for PUT request"); + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Using 100-continue for PUT request"); this->m_request.SetHeader("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. - Log(LogLevel::Verbose, LogMsgPrefix + "Send request without payload"); + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Send request without payload"); auto result = SendRawHttp(context); if (result != CURLE_OK) @@ -246,7 +241,7 @@ CURLcode CurlSession::Perform(Context const& context) return result; } - Log(LogLevel::Verbose, LogMsgPrefix + "Parse server response"); + Log::Write(Logger::Level::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 @@ -257,17 +252,17 @@ CURLcode CurlSession::Perform(Context const& context) return result; } - Log(LogLevel::Verbose, LogMsgPrefix + "Check server response before upload starts"); + Log::Write(Logger::Level::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) { - Log(LogLevel::Verbose, LogMsgPrefix + "Server rejected the upload request"); + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Server rejected the upload request"); m_sessionState = SessionState::STREAMING; return result; // Won't upload. } - Log(LogLevel::Verbose, LogMsgPrefix + "Upload payload"); + Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Upload payload"); if (this->m_bodyStartInBuffer > 0) { // If internal buffer has more data after the 100-continue means Server return an error. @@ -285,7 +280,7 @@ CURLcode CurlSession::Perform(Context const& context) return result; // will throw transport exception before trying to read } - Log(LogLevel::Verbose, LogMsgPrefix + "Upload completed. Parse server response"); + Log::Write(Logger::Level::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/log_policy.cpp b/sdk/core/azure-core/src/http/log_policy.cpp new file mode 100644 index 000000000..287a1b237 --- /dev/null +++ b/sdk/core/azure-core/src/http/log_policy.cpp @@ -0,0 +1,146 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#include "azure/core/http/policy.hpp" +#include "azure/core/internal/log.hpp" + +#include +#include +#include + +using Azure::Core::Context; +using namespace Azure::Core::Http; + +namespace { +std::string RedactedPlaceholder = "REDACTED"; + +inline void AppendHeaders( + std::ostringstream& log, + Azure::Core::CaseInsensitiveMap const& headers, + Azure::Core::CaseInsensitiveSet const& allowedHaders) +{ + for (auto const& header : headers) + { + log << std::endl << header.first << " : "; + + if (!header.second.empty()) + { + log + << ((allowedHaders.find(header.first) != allowedHaders.end()) ? header.second + : RedactedPlaceholder); + } + } +} + +inline std::string GetRequestLogMessage( + Azure::Core::Http::LogOptions const& options, + Request const& request) +{ + auto const& requestUrl = request.GetUrl(); + + std::ostringstream log; + log << "HTTP Request : " << HttpMethodToString(request.GetMethod()) << " " + << requestUrl.GetUrlWithoutQuery(); + { + auto encodedRequestQueryParams = requestUrl.GetQueryParameters(); + auto const& unencodedAllowedQueryParams = options.AllowedHttpQueryParameters; + if (!encodedRequestQueryParams.empty() && !unencodedAllowedQueryParams.empty()) + { + std::remove_const::type>::type + encodedAllowedQueryParams; + std::transform( + unencodedAllowedQueryParams.begin(), + unencodedAllowedQueryParams.end(), + std::inserter(encodedAllowedQueryParams, encodedAllowedQueryParams.begin()), + [](std::string const& s) { return Url::Encode(s); }); + + std::remove_const::type>::type + encodedAllowedRequestQueryParams; + for (auto const& encodedRequestQueryParam : encodedRequestQueryParams) + { + if (encodedRequestQueryParam.second.empty() + || (encodedAllowedQueryParams.find(encodedRequestQueryParam.first) + != encodedAllowedQueryParams.end())) + { + encodedAllowedRequestQueryParams.insert(encodedRequestQueryParam); + } + else + { + encodedAllowedRequestQueryParams.insert( + std::make_pair(encodedRequestQueryParam.first, RedactedPlaceholder)); + } + } + + log << Details::FormatEncodedUrlQueryParameters(encodedAllowedRequestQueryParams); + } + } + AppendHeaders(log, request.GetHeaders(), options.AllowedHttpHeaders); + return log.str(); +} + +inline std::string GetResponseLogMessage( + Azure::Core::Http::LogOptions const& options, + RawResponse const& response, + std::chrono::system_clock::duration const& duration) +{ + std::ostringstream log; + + log << "HTTP Response (" + << std::chrono::duration_cast(duration).count() + << "ms) : " << static_cast(response.GetStatusCode()) << " " + << response.GetReasonPhrase(); + + AppendHeaders(log, response.GetHeaders(), options.AllowedHttpHeaders); + return log.str(); +} +} // namespace + +Azure::Core::CaseInsensitiveSet Azure::Core::Http::Details::g_defaultAllowedHttpHeaders + = {"x-ms-client-request-id", + "x-ms-return-client-request-id", + "traceparent", + "Accept", + "Cache-Control", + "Connection", + "Content-Length", + "Content-Type", + "Date", + "ETag", + "Expires", + "If-Match", + "If-Modified-Since", + "If-None-Match", + "If-Unmodified-Since", + "Last-Modified", + "Pragma", + "Request-Id", + "Retry-After", + "Server", + "Transfer-Encoding", + "User-Agent"}; + +std::unique_ptr Azure::Core::Http::LogPolicy::Send( + Context const& ctx, + Request& request, + NextHttpPolicy nextHttpPolicy) const +{ + using Azure::Core::Internal::Log; + + if (Log::ShouldWrite(Logger::Level::Verbose)) + { + Log::Write(Logger::Level::Informational, GetRequestLogMessage(m_options, request)); + } + else + { + return nextHttpPolicy.Send(ctx, request); + } + + auto const start = std::chrono::system_clock::now(); + auto response = nextHttpPolicy.Send(ctx, request); + auto const end = std::chrono::system_clock::now(); + + Log::Write( + Logger::Level::Informational, GetResponseLogMessage(m_options, *response, end - start)); + + return response; +} diff --git a/sdk/core/azure-core/src/http/logging_policy.cpp b/sdk/core/azure-core/src/http/logging_policy.cpp deleted file mode 100644 index 07dd62429..000000000 --- a/sdk/core/azure-core/src/http/logging_policy.cpp +++ /dev/null @@ -1,97 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// SPDX-License-Identifier: MIT - -#include "azure/core/http/policy.hpp" -#include "azure/core/internal/log.hpp" - -#include -#include - -using Azure::Core::Context; -using namespace Azure::Core::Http; - -namespace { -std::string TruncateIfLengthy(std::string const& s) -{ - static constexpr auto const MaxLength = 50; - - auto const length = s.length(); - if (length <= MaxLength) - { - return s; - } - - static constexpr char const Ellipsis[] = " ... "; - static constexpr auto const EllipsisLength = sizeof(Ellipsis) - 1; - - auto const BeginLength = (MaxLength / 2) - ((EllipsisLength / 2) + (EllipsisLength % 2)); - auto const EndLength = ((MaxLength / 2) + (MaxLength % 2)) - (EllipsisLength / 2); - - return s.substr(0, BeginLength) + Ellipsis + s.substr(length - EndLength, EndLength); -} - -std::string GetRequestLogMessage(Request const& request) -{ - std::ostringstream log; - - log << "HTTP Request : " << HttpMethodToString(request.GetMethod()) << " " - << request.GetUrl().GetAbsoluteUrl(); - - for (auto header : request.GetHeaders()) - { - log << "\n\t" << header.first << " : " << TruncateIfLengthy(header.second); - } - - return log.str(); -} - -std::string GetResponseLogMessage( - Request const& request, - RawResponse const& response, - std::chrono::system_clock::duration const& duration) -{ - std::ostringstream log; - - log << "HTTP Response (" - << std::chrono::duration_cast(duration).count() - << "ms) : " << static_cast(response.GetStatusCode()) << " " - << response.GetReasonPhrase(); - - for (auto header : response.GetHeaders()) - { - log << "\n\t" << header.first; - if (!header.second.empty() && header.first != "authorization") - { - log << " : " << TruncateIfLengthy(header.second); - } - } - - log << "\n\n -> " << GetRequestLogMessage(request); - - return log.str(); -} -} // namespace - -std::unique_ptr Azure::Core::Http::LoggingPolicy::Send( - Context const& ctx, - Request& request, - NextHttpPolicy nextHttpPolicy) const -{ - if (Logging::Internal::ShouldLog(Logging::LogLevel::Verbose)) - { - Logging::Internal::Log(Logging::LogLevel::Verbose, GetRequestLogMessage(request)); - } - else - { - return nextHttpPolicy.Send(ctx, request); - } - - auto const start = std::chrono::system_clock::now(); - auto response = nextHttpPolicy.Send(ctx, request); - auto const end = std::chrono::system_clock::now(); - - Logging::Internal::Log( - Logging::LogLevel::Verbose, GetResponseLogMessage(request, *response, end - start)); - - return response; -} diff --git a/sdk/core/azure-core/src/http/retry_policy.cpp b/sdk/core/azure-core/src/http/retry_policy.cpp index c1cf66ece..b65e35b96 100644 --- a/sdk/core/azure-core/src/http/retry_policy.cpp +++ b/sdk/core/azure-core/src/http/retry_policy.cpp @@ -101,6 +101,8 @@ bool ShouldRetryOnResponse( RetryNumber attempt, Delay& retryAfter) { + using Azure::Core::Logger; + using Azure::Core::Internal::Log; // Are we out of retry attempts? if (WasLastAttempt(retryOptions, attempt)) { @@ -108,11 +110,28 @@ bool ShouldRetryOnResponse( } // Should we retry on the given response retry code? - auto const& statusCodes = retryOptions.StatusCodes; - auto const statusCodesEnd = statusCodes.end(); - if (std::find(statusCodes.begin(), statusCodesEnd, response.GetStatusCode()) == statusCodesEnd) { - return false; + auto const& statusCodes = retryOptions.StatusCodes; + auto const sc = response.GetStatusCode(); + if (statusCodes.find(sc) == statusCodes.end()) + { + if (Log::ShouldWrite(Logger::Level::Warning)) + { + Log::Write( + Logger::Level::Warning, + std::string("HTTP status code ") + std::to_string(static_cast(sc)) + + " won't be retried."); + } + + return false; + } + else if (Log::ShouldWrite(Logger::Level::Informational)) + { + Log::Write( + Logger::Level::Informational, + std::string("HTTP status code ") + std::to_string(static_cast(sc)) + + " will be retried."); + } } if (!GetResponseHeaderBasedDelay(response, retryAfter)) @@ -163,8 +182,11 @@ std::unique_ptr Azure::Core::Http::RetryPolicy::Send( Request& request, NextHttpPolicy nextHttpPolicy) const { - auto const shouldLog = Logging::Internal::ShouldLog(Logging::LogLevel::Informational); + using Azure::Core::Logger; + using Azure::Core::Internal::Log; + auto retryContext = CreateRetryContext(ctx); + for (RetryNumber attempt = 1;; ++attempt) { Delay retryAfter{}; @@ -185,22 +207,27 @@ std::unique_ptr Azure::Core::Http::RetryPolicy::Send( return response; } } - catch (const TransportException&) + catch (const TransportException& e) { + if (Log::ShouldWrite(Logger::Level::Warning)) + { + Log::Write(Logger::Level::Warning, std::string("HTTP Transport error: ") + e.what()); + } + if (!ShouldRetryOnTransportFailure(m_retryOptions, attempt, retryAfter)) { throw; } } - if (shouldLog) + if (Log::ShouldWrite(Logger::Level::Informational)) { std::ostringstream log; log << "HTTP Retry attempt #" << attempt << " will be made in " << std::chrono::duration_cast(retryAfter).count() << "ms."; - Logging::Internal::Log(Logging::LogLevel::Informational, log.str()); + Log::Write(Logger::Level::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/http/url.cpp b/sdk/core/azure-core/src/http/url.cpp index 9a3df5602..72afa56a6 100644 --- a/sdk/core/azure-core/src/http/url.cpp +++ b/sdk/core/azure-core/src/http/url.cpp @@ -177,43 +177,46 @@ void Url::AppendQueryParameters(const std::string& query) } } -std::string Url::GetRelativeUrl() const +std::string Url::GetUrlWithoutQuery(bool relative) const { - std::string relative_url; - if (!m_encodedPath.empty()) + std::string url; + + if (!relative) { - relative_url += m_encodedPath; - } - { - relative_url += '?'; - for (const auto& q : m_encodedQueryParameters) + if (!m_scheme.empty()) { - relative_url += q.first + '=' + q.second + '&'; + url += m_scheme + "://"; + } + url += m_host; + if (m_port != 0) + { + url += ":" + std::to_string(m_port); } - relative_url.pop_back(); } - return relative_url; + if (!m_encodedPath.empty()) + { + if (!relative) + { + url += "/"; + } + + url += m_encodedPath; + } + + return url; +} + +std::string Url::GetRelativeUrl() const +{ + return GetUrlWithoutQuery(true) + + Details::FormatEncodedUrlQueryParameters(m_encodedQueryParameters); } std::string Url::GetAbsoluteUrl() const { - std::string full_url; - if (!m_scheme.empty()) - { - full_url += m_scheme + "://"; - } - full_url += m_host; - if (m_port != 0) - { - full_url += ":" + std::to_string(m_port); - } - if (!m_encodedPath.empty()) - { - full_url += "/"; - } - full_url += GetRelativeUrl(); - return full_url; + return GetUrlWithoutQuery(false) + + Details::FormatEncodedUrlQueryParameters(m_encodedQueryParameters); } const std::unordered_set Url::defaultNonUrlEncodeChars diff --git a/sdk/core/azure-core/src/logger.cpp b/sdk/core/azure-core/src/logger.cpp new file mode 100644 index 000000000..9604703d5 --- /dev/null +++ b/sdk/core/azure-core/src/logger.cpp @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#include "azure/core/logger.hpp" +#include "azure/core/internal/log.hpp" + +#include +#include + +#include "environment_log_level_listener_private.hpp" + +using namespace Azure::Core; +using namespace Azure::Core::Internal; + +namespace { +static std::shared_timed_mutex g_logListenerMutex; +static Logger::Listener g_logListener(Details::EnvironmentLogLevelListener::GetLogListener()); +} // namespace + +std::atomic Log::g_isLoggingEnabled( + Details::EnvironmentLogLevelListener::GetLogListener() != nullptr); + +std::atomic Log::g_logLevel(static_cast( + Details::EnvironmentLogLevelListener::GetLogLevel(Logger::Level::Warning))); + +inline void Log::EnableLogging(bool isEnabled) { g_isLoggingEnabled = isEnabled; } + +inline void Log::SetLogLevel(Logger::Level logLevel) +{ + g_logLevel = static_cast(logLevel); +} + +void Log::Write(Logger::Level level, std::string const& message) +{ + if (ShouldWrite(level)) + { + std::shared_lock loggerLock(g_logListenerMutex); + if (g_logListener) + { + g_logListener(level, message); + } + } +} + +void Logger::SetListener(Logger::Listener listener) +{ + std::unique_lock loggerLock(g_logListenerMutex); + g_logListener = std::move(listener); + Log::EnableLogging(g_logListener != nullptr); +} + +void Logger::SetLevel(Logger::Level level) { Log::SetLogLevel(level); } diff --git a/sdk/core/azure-core/src/logging/logging.cpp b/sdk/core/azure-core/src/logging/logging.cpp deleted file mode 100644 index 12d5fdbef..000000000 --- a/sdk/core/azure-core/src/logging/logging.cpp +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// SPDX-License-Identifier: MIT - -#include "azure/core/logging/logging.hpp" -#include "azure/core/internal/log.hpp" - -#include - -using namespace Azure::Core::Logging; -using namespace Azure::Core::Logging::Internal; - -namespace { -std::mutex g_loggerMutex; -LogListener g_logListener(nullptr); -LogLevel g_logLevel = LogLevel::Verbose; - -LogListener GetLogListener(LogLevel level) -{ - std::lock_guard loggerLock(g_loggerMutex); - return level <= g_logLevel ? g_logListener : LogListener(nullptr); -} -} // namespace - -void Azure::Core::Logging::SetLogListener(LogListener logListener) -{ - std::lock_guard loggerLock(g_loggerMutex); - g_logListener = std::move(logListener); -} - -void Azure::Core::Logging::SetLogLevel(LogLevel level) -{ - std::lock_guard loggerLock(g_loggerMutex); - g_logLevel = level; -} - -bool Azure::Core::Logging::Internal::ShouldLog(LogLevel level) -{ - return GetLogListener(level) != nullptr; -} - -void Azure::Core::Logging::Internal::Log(LogLevel level, std::string const& message) -{ - if (auto logListener = GetLogListener(level)) - { - logListener(level, message); - } -} diff --git a/sdk/core/azure-core/test/ut/CMakeLists.txt b/sdk/core/azure-core/test/ut/CMakeLists.txt index 87a0d3893..66e6963c3 100644 --- a/sdk/core/azure-core/test/ut/CMakeLists.txt +++ b/sdk/core/azure-core/test/ut/CMakeLists.txt @@ -31,7 +31,7 @@ add_executable ( azure-core-test base64.cpp bodystream.cpp - case_insensitive_map.cpp + case_insensitive_containers.cpp client_options.cpp context.cpp ${CURL_CONNECTION_POOL_TESTS} diff --git a/sdk/core/azure-core/test/ut/case_insensitive_map.cpp b/sdk/core/azure-core/test/ut/case_insensitive_containers.cpp similarity index 92% rename from sdk/core/azure-core/test/ut/case_insensitive_map.cpp rename to sdk/core/azure-core/test/ut/case_insensitive_containers.cpp index 797895e41..e02b99cd4 100644 --- a/sdk/core/azure-core/test/ut/case_insensitive_map.cpp +++ b/sdk/core/azure-core/test/ut/case_insensitive_containers.cpp @@ -3,7 +3,7 @@ #include -#include +#include using namespace Azure::Core; diff --git a/sdk/core/azure-core/test/ut/datetime.cpp b/sdk/core/azure-core/test/ut/datetime.cpp index 4fd985325..057a97355 100644 --- a/sdk/core/azure-core/test/ut/datetime.cpp +++ b/sdk/core/azure-core/test/ut/datetime.cpp @@ -48,7 +48,7 @@ template -#include #include -using namespace Azure::Core::Logging; -using namespace Azure::Core::Logging::Internal; +using Azure::Core::Logger; +using Azure::Core::Internal::Log; -TEST(Logging, Defaults) +TEST(Logger, Levels) { - EXPECT_FALSE(ShouldLog(LogLevel::Verbose)); - EXPECT_FALSE(ShouldLog(LogLevel::Informational)); - EXPECT_FALSE(ShouldLog(LogLevel::Warning)); - EXPECT_FALSE(ShouldLog(LogLevel::Error)); + Logger::SetListener([](auto, auto) {}); - SetLogListener([](auto, auto) {}); + Logger::SetLevel(Logger::Level::Verbose); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Verbose)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Informational)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Warning)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Error)); - EXPECT_TRUE(ShouldLog(LogLevel::Verbose)); - EXPECT_TRUE(ShouldLog(LogLevel::Informational)); - EXPECT_TRUE(ShouldLog(LogLevel::Warning)); - EXPECT_TRUE(ShouldLog(LogLevel::Error)); + Logger::SetLevel(Logger::Level::Informational); + EXPECT_FALSE(Log::ShouldWrite(Logger::Level::Verbose)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Informational)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Warning)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Error)); - SetLogListener(nullptr); + Logger::SetLevel(Logger::Level::Warning); + EXPECT_FALSE(Log::ShouldWrite(Logger::Level::Verbose)); + EXPECT_FALSE(Log::ShouldWrite(Logger::Level::Informational)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Warning)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Error)); - EXPECT_FALSE(ShouldLog(LogLevel::Verbose)); - EXPECT_FALSE(ShouldLog(LogLevel::Informational)); - EXPECT_FALSE(ShouldLog(LogLevel::Warning)); - EXPECT_FALSE(ShouldLog(LogLevel::Error)); + Logger::SetLevel(Logger::Level::Error); + EXPECT_FALSE(Log::ShouldWrite(Logger::Level::Verbose)); + EXPECT_FALSE(Log::ShouldWrite(Logger::Level::Informational)); + EXPECT_FALSE(Log::ShouldWrite(Logger::Level::Warning)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Error)); + + Logger::SetLevel(Logger::Level::Verbose); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Verbose)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Informational)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Warning)); + EXPECT_TRUE(Log::ShouldWrite(Logger::Level::Error)); + + Logger::SetListener(nullptr); } -TEST(Logging, Levels) -{ - SetLogListener([](auto, auto) {}); - - SetLogLevel(LogLevel::Verbose); - EXPECT_TRUE(ShouldLog(LogLevel::Verbose)); - EXPECT_TRUE(ShouldLog(LogLevel::Informational)); - EXPECT_TRUE(ShouldLog(LogLevel::Warning)); - EXPECT_TRUE(ShouldLog(LogLevel::Error)); - - SetLogLevel(LogLevel::Informational); - EXPECT_FALSE(ShouldLog(LogLevel::Verbose)); - EXPECT_TRUE(ShouldLog(LogLevel::Informational)); - EXPECT_TRUE(ShouldLog(LogLevel::Warning)); - EXPECT_TRUE(ShouldLog(LogLevel::Error)); - - 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) +TEST(Logger, Message) { try { - LogLevel level = LogLevel::Error; + Logger::Level level = Logger::Level::Error; std::string message; - SetLogListener([&](auto lvl, auto msg) { + Logger::SetListener([&](auto lvl, auto msg) { level = lvl; message = msg; }); - SetLogLevel(LogLevel::Verbose); + Logger::SetLevel(Logger::Level::Verbose); { - level = LogLevel::Error; + level = Logger::Level::Error; message = ""; - Log(LogLevel::Verbose, "Verbose"); - EXPECT_EQ(level, LogLevel::Verbose); + Log::Write(Logger::Level::Verbose, "Verbose"); + EXPECT_EQ(level, Logger::Level::Verbose); EXPECT_EQ(message, "Verbose"); - Log(LogLevel::Informational, "Informational"); - EXPECT_EQ(level, LogLevel::Informational); + Log::Write(Logger::Level::Informational, "Informational"); + EXPECT_EQ(level, Logger::Level::Informational); EXPECT_EQ(message, "Informational"); - Log(LogLevel::Warning, "Warning"); - EXPECT_EQ(level, LogLevel::Warning); + Log::Write(Logger::Level::Warning, "Warning"); + EXPECT_EQ(level, Logger::Level::Warning); EXPECT_EQ(message, "Warning"); - Log(LogLevel::Error, "Error"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Error, "Error"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, "Error"); } - SetLogLevel(LogLevel::Informational); + Logger::SetLevel(Logger::Level::Informational); { - level = LogLevel::Error; + level = Logger::Level::Error; message = ""; - Log(LogLevel::Verbose, "Verbose"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Verbose, "Verbose"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, ""); - Log(LogLevel::Informational, "Informational"); - EXPECT_EQ(level, LogLevel::Informational); + Log::Write(Logger::Level::Informational, "Informational"); + EXPECT_EQ(level, Logger::Level::Informational); EXPECT_EQ(message, "Informational"); - Log(LogLevel::Warning, "Warning"); - EXPECT_EQ(level, LogLevel::Warning); + Log::Write(Logger::Level::Warning, "Warning"); + EXPECT_EQ(level, Logger::Level::Warning); EXPECT_EQ(message, "Warning"); - Log(LogLevel::Error, "Error"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Error, "Error"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, "Error"); } - SetLogLevel(LogLevel::Warning); + Logger::SetLevel(Logger::Level::Warning); { - level = LogLevel::Error; + level = Logger::Level::Error; message = ""; - Log(LogLevel::Verbose, "Verbose"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Verbose, "Verbose"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, ""); - Log(LogLevel::Informational, "Informational"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Informational, "Informational"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, ""); - Log(LogLevel::Warning, "Warning"); - EXPECT_EQ(level, LogLevel::Warning); + Log::Write(Logger::Level::Warning, "Warning"); + EXPECT_EQ(level, Logger::Level::Warning); EXPECT_EQ(message, "Warning"); - Log(LogLevel::Error, "Error"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Error, "Error"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, "Error"); } - SetLogLevel(LogLevel::Error); + Logger::SetLevel(Logger::Level::Error); { - level = LogLevel::Error; + level = Logger::Level::Error; message = ""; - Log(LogLevel::Verbose, "Verbose"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Verbose, "Verbose"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, ""); - Log(LogLevel::Informational, "Informational"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Informational, "Informational"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, ""); - Log(LogLevel::Warning, "Warning"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Warning, "Warning"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, ""); - level = LogLevel::Verbose; + level = Logger::Level::Verbose; - Log(LogLevel::Error, "Error"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Error, "Error"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, "Error"); } // Verify that we can switch back to Verbose - SetLogLevel(LogLevel::Verbose); + Logger::SetLevel(Logger::Level::Verbose); { - level = LogLevel::Error; + level = Logger::Level::Error; message = ""; - Log(LogLevel::Verbose, "Verbose"); - EXPECT_EQ(level, LogLevel::Verbose); + Log::Write(Logger::Level::Verbose, "Verbose"); + EXPECT_EQ(level, Logger::Level::Verbose); EXPECT_EQ(message, "Verbose"); - Log(LogLevel::Informational, "Informational"); - EXPECT_EQ(level, LogLevel::Informational); + Log::Write(Logger::Level::Informational, "Informational"); + EXPECT_EQ(level, Logger::Level::Informational); EXPECT_EQ(message, "Informational"); - Log(LogLevel::Warning, "Warning"); - EXPECT_EQ(level, LogLevel::Warning); + Log::Write(Logger::Level::Warning, "Warning"); + EXPECT_EQ(level, Logger::Level::Warning); EXPECT_EQ(message, "Warning"); - Log(LogLevel::Error, "Error"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Error, "Error"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, "Error"); } - SetLogListener(nullptr); + Logger::SetListener(nullptr); - SetLogLevel(LogLevel::Verbose); + Logger::SetLevel(Logger::Level::Verbose); { - level = LogLevel::Error; + level = Logger::Level::Error; message = ""; - Log(LogLevel::Verbose, "Verbose"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Verbose, "Verbose"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, ""); - Log(LogLevel::Informational, "Informational"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Informational, "Informational"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, ""); - Log(LogLevel::Warning, "Warning"); - EXPECT_EQ(level, LogLevel::Error); + Log::Write(Logger::Level::Warning, "Warning"); + EXPECT_EQ(level, Logger::Level::Error); EXPECT_EQ(message, ""); - level = LogLevel::Verbose; + level = Logger::Level::Verbose; - Log(LogLevel::Error, "Error"); - EXPECT_EQ(level, LogLevel::Verbose); + Log::Write(Logger::Level::Error, "Error"); + EXPECT_EQ(level, Logger::Level::Verbose); EXPECT_EQ(message, ""); } } catch (...) { - SetLogListener(nullptr); + Logger::SetListener(nullptr); throw; } } diff --git a/sdk/core/azure-core/test/ut/simplified_header.cpp b/sdk/core/azure-core/test/ut/simplified_header.cpp index 97f18936e..4e9600536 100644 --- a/sdk/core/azure-core/test/ut/simplified_header.cpp +++ b/sdk/core/azure-core/test/ut/simplified_header.cpp @@ -28,6 +28,8 @@ class DllExportTest { TEST(SimplifiedHeader, core) { + EXPECT_NO_THROW(Azure::Core::CaseInsensitiveMap imap); + EXPECT_NO_THROW(Azure::Core::CaseInsensitiveSet iset); EXPECT_NO_THROW(Azure::Core::Context c); EXPECT_NO_THROW(Azure::Core::DateTime(2020, 11, 03, 15, 30, 44)); EXPECT_NO_THROW(Azure::Core::ETag e); @@ -35,6 +37,7 @@ TEST(SimplifiedHeader, core) EXPECT_NO_THROW(Azure::Core::Cryptography::Md5Hash m); EXPECT_NO_THROW(Azure::Core::Http::RawResponse r( 1, 1, Azure::Core::Http::HttpStatusCode::Accepted, "phrase")); + EXPECT_NO_THROW(Azure::Core::Logger::Listener ll = nullptr); EXPECT_NO_THROW(Azure::Core::MatchConditions mc); EXPECT_NO_THROW(Azure::Core::ModifiedConditions mc); EXPECT_NO_THROW(Azure::Core::Nullable n); diff --git a/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp b/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp index 54f3fd658..5ca0ef874 100644 --- a/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp +++ b/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp @@ -2856,7 +2856,10 @@ namespace Azure { namespace Storage { namespace Blobs { writer.Write(Storage::Details::XmlNode{ Storage::Details::XmlNodeType::Text, nullptr, - options.StartsOn.GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) + options.StartsOn + .ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) .data()}); writer.Write(Storage::Details::XmlNode{Storage::Details::XmlNodeType::EndTag}); writer.Write( @@ -2865,7 +2868,9 @@ namespace Azure { namespace Storage { namespace Blobs { Storage::Details::XmlNodeType::Text, nullptr, options.ExpiresOn - .GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) + .ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) .data()}); writer.Write(Storage::Details::XmlNode{Storage::Details::XmlNodeType::EndTag}); writer.Write(Storage::Details::XmlNode{Storage::Details::XmlNodeType::EndTag}); @@ -4805,7 +4810,9 @@ namespace Azure { namespace Storage { namespace Blobs { Storage::Details::XmlNodeType::Text, nullptr, options.StartsOn - .GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::AllDigits) + .ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::AllDigits) .data()}); writer.Write(Storage::Details::XmlNode{Storage::Details::XmlNodeType::EndTag}); writer.Write( @@ -4814,7 +4821,9 @@ namespace Azure { namespace Storage { namespace Blobs { Storage::Details::XmlNodeType::Text, nullptr, options.ExpiresOn - .GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::AllDigits) + .ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::AllDigits) .data()}); writer.Write(Storage::Details::XmlNode{Storage::Details::XmlNodeType::EndTag}); writer.Write( diff --git a/sdk/storage/azure-storage-blobs/src/blob_sas_builder.cpp b/sdk/storage/azure-storage-blobs/src/blob_sas_builder.cpp index 3b65c8e5f..d85facddb 100644 --- a/sdk/storage/azure-storage-blobs/src/blob_sas_builder.cpp +++ b/sdk/storage/azure-storage-blobs/src/blob_sas_builder.cpp @@ -128,12 +128,14 @@ namespace Azure { namespace Storage { namespace Sas { snapshotVersion = BlobVersionId; } - std::string startsOnStr = StartsOn.HasValue() - ? StartsOn.GetValue().GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) - : ""; - std::string expiresOnStr = Identifier.empty() - ? ExpiresOn.GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) - : ""; + std::string startsOnStr = StartsOn.HasValue() ? StartsOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) + : ""; + std::string expiresOnStr = Identifier.empty() ? ExpiresOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) + : ""; std::string stringToSign = Permissions + "\n" + startsOnStr + "\n" + expiresOnStr + "\n" + canonicalName + "\n" + Identifier + "\n" + (IPRange.HasValue() ? IPRange.GetValue() : "") @@ -222,14 +224,18 @@ namespace Azure { namespace Storage { namespace Sas { snapshotVersion = BlobVersionId; } - std::string startsOnStr = StartsOn.HasValue() - ? StartsOn.GetValue().GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) - : ""; - std::string expiresOnStr - = ExpiresOn.GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate); - std::string signedStartsOnStr = userDelegationKey.SignedStartsOn.GetRfc3339String( + std::string startsOnStr = StartsOn.HasValue() ? StartsOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) + : ""; + std::string expiresOnStr = ExpiresOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Azure::Core::DateTime::TimeFractionFormat::Truncate); - std::string signedExpiresOnStr = userDelegationKey.SignedExpiresOn.GetRfc3339String( + std::string signedStartsOnStr = userDelegationKey.SignedStartsOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate); + std::string signedExpiresOnStr = userDelegationKey.SignedExpiresOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Azure::Core::DateTime::TimeFractionFormat::Truncate); std::string stringToSign = Permissions + "\n" + startsOnStr + "\n" + expiresOnStr + "\n" diff --git a/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_common.hpp b/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_common.hpp index 02a174174..3950b9c23 100644 --- a/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_common.hpp +++ b/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_common.hpp @@ -10,7 +10,7 @@ #include #include -#include +#include #include #include "azure/storage/common/constants.hpp" diff --git a/sdk/storage/azure-storage-common/src/account_sas_builder.cpp b/sdk/storage/azure-storage-common/src/account_sas_builder.cpp index c29aa5991..82798afc1 100644 --- a/sdk/storage/azure-storage-common/src/account_sas_builder.cpp +++ b/sdk/storage/azure-storage-common/src/account_sas_builder.cpp @@ -91,11 +91,13 @@ namespace Azure { namespace Storage { namespace Sas { resourceTypes += "o"; } - std::string startsOnStr = StartsOn.HasValue() - ? StartsOn.GetValue().GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) - : ""; - std::string expiresOnStr - = ExpiresOn.GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate); + std::string startsOnStr = StartsOn.HasValue() ? StartsOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) + : ""; + std::string expiresOnStr = ExpiresOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate); std::string stringToSign = credential.AccountName + "\n" + Permissions + "\n" + services + "\n" + resourceTypes + "\n" + startsOnStr + "\n" + expiresOnStr + "\n" diff --git a/sdk/storage/azure-storage-files-datalake/src/datalake_sas_builder.cpp b/sdk/storage/azure-storage-files-datalake/src/datalake_sas_builder.cpp index b296202ac..42cf6fa9c 100644 --- a/sdk/storage/azure-storage-files-datalake/src/datalake_sas_builder.cpp +++ b/sdk/storage/azure-storage-files-datalake/src/datalake_sas_builder.cpp @@ -123,12 +123,14 @@ namespace Azure { namespace Storage { namespace Sas { std::string protocol = Details::SasProtocolToString(Protocol); std::string resource = DataLakeSasResourceToString(Resource); - std::string startsOnStr = StartsOn.HasValue() - ? StartsOn.GetValue().GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) - : ""; - std::string expiresOnStr = Identifier.empty() - ? ExpiresOn.GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) - : ""; + std::string startsOnStr = StartsOn.HasValue() ? StartsOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) + : ""; + std::string expiresOnStr = Identifier.empty() ? ExpiresOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) + : ""; std::string stringToSign = Permissions + "\n" + startsOnStr + "\n" + expiresOnStr + "\n" + canonicalName + "\n" + Identifier + "\n" + (IPRange.HasValue() ? IPRange.GetValue() : "") @@ -206,14 +208,18 @@ namespace Azure { namespace Storage { namespace Sas { std::string protocol = Details::SasProtocolToString(Protocol); std::string resource = DataLakeSasResourceToString(Resource); - std::string startsOnStr = StartsOn.HasValue() - ? StartsOn.GetValue().GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) - : ""; - std::string expiresOnStr - = ExpiresOn.GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate); - std::string signedStartsOnStr = userDelegationKey.SignedStartsOn.GetRfc3339String( + std::string startsOnStr = StartsOn.HasValue() ? StartsOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) + : ""; + std::string expiresOnStr = ExpiresOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Azure::Core::DateTime::TimeFractionFormat::Truncate); - std::string signedExpiresOnStr = userDelegationKey.SignedExpiresOn.GetRfc3339String( + std::string signedStartsOnStr = userDelegationKey.SignedStartsOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate); + std::string signedExpiresOnStr = userDelegationKey.SignedExpiresOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Azure::Core::DateTime::TimeFractionFormat::Truncate); std::string stringToSign = Permissions + "\n" + startsOnStr + "\n" + expiresOnStr + "\n" diff --git a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp index 9c04c2650..4dff30e36 100644 --- a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp +++ b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp @@ -3869,7 +3869,10 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { writer.Write(Storage::Details::XmlNode{ Storage::Details::XmlNodeType::Text, nullptr, - object.StartsOn.GetRfc3339String(Core::DateTime::TimeFractionFormat::AllDigits) + object.StartsOn + .ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Core::DateTime::TimeFractionFormat::AllDigits) .data()}); writer.Write(Storage::Details::XmlNode{Storage::Details::XmlNodeType::EndTag}); writer.Write( @@ -3877,7 +3880,10 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { writer.Write(Storage::Details::XmlNode{ Storage::Details::XmlNodeType::Text, nullptr, - object.ExpiresOn.GetRfc3339String(Core::DateTime::TimeFractionFormat::AllDigits) + object.ExpiresOn + .ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Core::DateTime::TimeFractionFormat::AllDigits) .data()}); writer.Write(Storage::Details::XmlNode{Storage::Details::XmlNodeType::EndTag}); writer.Write( diff --git a/sdk/storage/azure-storage-files-shares/src/share_directory_client.cpp b/sdk/storage/azure-storage-files-shares/src/share_directory_client.cpp index 60bdea9ff..c3528e289 100644 --- a/sdk/storage/azure-storage-files-shares/src/share_directory_client.cpp +++ b/sdk/storage/azure-storage-files-shares/src/share_directory_client.cpp @@ -117,9 +117,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } if (options.SmbProperties.CreatedOn.HasValue()) { - protocolLayerOptions.FileCreationTime - = options.SmbProperties.CreatedOn.GetValue().GetRfc3339String( - Core::DateTime::TimeFractionFormat::AllDigits); + protocolLayerOptions.FileCreationTime = options.SmbProperties.CreatedOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Core::DateTime::TimeFractionFormat::AllDigits); } else { @@ -128,7 +128,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { if (options.SmbProperties.LastWrittenOn.HasValue()) { protocolLayerOptions.FileLastWriteTime - = options.SmbProperties.LastWrittenOn.GetValue().GetRfc3339String( + = options.SmbProperties.LastWrittenOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Core::DateTime::TimeFractionFormat::AllDigits); } else @@ -242,7 +243,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { protocolLayerOptions.FileAttributes = smbProperties.Attributes.ToString(); if (smbProperties.CreatedOn.HasValue()) { - protocolLayerOptions.FileCreationTime = smbProperties.CreatedOn.GetValue().GetRfc3339String( + protocolLayerOptions.FileCreationTime = smbProperties.CreatedOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Core::DateTime::TimeFractionFormat::AllDigits); } else @@ -251,9 +253,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } if (smbProperties.LastWrittenOn.HasValue()) { - protocolLayerOptions.FileLastWriteTime - = smbProperties.LastWrittenOn.GetValue().GetRfc3339String( - Core::DateTime::TimeFractionFormat::AllDigits); + protocolLayerOptions.FileLastWriteTime = smbProperties.LastWrittenOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Core::DateTime::TimeFractionFormat::AllDigits); } else { diff --git a/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp b/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp index d1ee39ca4..ccb7b8289 100644 --- a/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp +++ b/sdk/storage/azure-storage-files-shares/src/share_file_client.cpp @@ -106,9 +106,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } if (options.SmbProperties.CreatedOn.HasValue()) { - protocolLayerOptions.FileCreationTime - = options.SmbProperties.CreatedOn.GetValue().GetRfc3339String( - Core::DateTime::TimeFractionFormat::AllDigits); + protocolLayerOptions.FileCreationTime = options.SmbProperties.CreatedOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Core::DateTime::TimeFractionFormat::AllDigits); } else { @@ -117,7 +117,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { if (options.SmbProperties.LastWrittenOn.HasValue()) { protocolLayerOptions.FileLastWriteTime - = options.SmbProperties.LastWrittenOn.GetValue().GetRfc3339String( + = options.SmbProperties.LastWrittenOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Core::DateTime::TimeFractionFormat::AllDigits); } else @@ -322,7 +323,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { if (options.SmbProperties.CreatedOn.HasValue()) { protocolLayerOptions.FileCopyFileCreationTime - = options.SmbProperties.CreatedOn.GetValue().GetRfc3339String( + = options.SmbProperties.CreatedOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Core::DateTime::TimeFractionFormat::AllDigits); } else @@ -332,7 +334,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { if (options.SmbProperties.LastWrittenOn.HasValue()) { protocolLayerOptions.FileCopyFileLastWriteTime - = options.SmbProperties.LastWrittenOn.GetValue().GetRfc3339String( + = options.SmbProperties.LastWrittenOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Core::DateTime::TimeFractionFormat::AllDigits); } else @@ -416,7 +419,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } if (smbProperties.CreatedOn.HasValue()) { - protocolLayerOptions.FileCreationTime = smbProperties.CreatedOn.GetValue().GetRfc3339String( + protocolLayerOptions.FileCreationTime = smbProperties.CreatedOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Core::DateTime::TimeFractionFormat::AllDigits); } else @@ -425,9 +429,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } if (smbProperties.LastWrittenOn.HasValue()) { - protocolLayerOptions.FileLastWriteTime - = smbProperties.LastWrittenOn.GetValue().GetRfc3339String( - Core::DateTime::TimeFractionFormat::AllDigits); + protocolLayerOptions.FileLastWriteTime = smbProperties.LastWrittenOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Core::DateTime::TimeFractionFormat::AllDigits); } else { @@ -843,9 +847,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } if (options.SmbProperties.CreatedOn.HasValue()) { - protocolLayerOptions.FileCreationTime - = options.SmbProperties.CreatedOn.GetValue().GetRfc3339String( - Core::DateTime::TimeFractionFormat::AllDigits); + protocolLayerOptions.FileCreationTime = options.SmbProperties.CreatedOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Core::DateTime::TimeFractionFormat::AllDigits); } else { @@ -854,7 +858,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { if (options.SmbProperties.LastWrittenOn.HasValue()) { protocolLayerOptions.FileLastWriteTime - = options.SmbProperties.LastWrittenOn.GetValue().GetRfc3339String( + = options.SmbProperties.LastWrittenOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Core::DateTime::TimeFractionFormat::AllDigits); } else @@ -948,9 +953,9 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { } if (options.SmbProperties.CreatedOn.HasValue()) { - protocolLayerOptions.FileCreationTime - = options.SmbProperties.CreatedOn.GetValue().GetRfc3339String( - Core::DateTime::TimeFractionFormat::AllDigits); + protocolLayerOptions.FileCreationTime = options.SmbProperties.CreatedOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Core::DateTime::TimeFractionFormat::AllDigits); } else { @@ -959,7 +964,8 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { if (options.SmbProperties.LastWrittenOn.HasValue()) { protocolLayerOptions.FileLastWriteTime - = options.SmbProperties.LastWrittenOn.GetValue().GetRfc3339String( + = options.SmbProperties.LastWrittenOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, Core::DateTime::TimeFractionFormat::AllDigits); } else diff --git a/sdk/storage/azure-storage-files-shares/src/share_sas_builder.cpp b/sdk/storage/azure-storage-files-shares/src/share_sas_builder.cpp index 8fdbc91c0..64c21d994 100644 --- a/sdk/storage/azure-storage-files-shares/src/share_sas_builder.cpp +++ b/sdk/storage/azure-storage-files-shares/src/share_sas_builder.cpp @@ -84,12 +84,14 @@ namespace Azure { namespace Storage { namespace Sas { std::string protocol = Details::SasProtocolToString(Protocol); std::string resource = ShareSasResourceToString(Resource); - std::string startsOnStr = StartsOn.HasValue() - ? StartsOn.GetValue().GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) - : ""; - std::string expiresOnStr = Identifier.empty() - ? ExpiresOn.GetRfc3339String(Azure::Core::DateTime::TimeFractionFormat::Truncate) - : ""; + std::string startsOnStr = StartsOn.HasValue() ? StartsOn.GetValue().ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) + : ""; + std::string expiresOnStr = Identifier.empty() ? ExpiresOn.ToString( + Azure::Core::DateTime::DateFormat::Rfc3339, + Azure::Core::DateTime::TimeFractionFormat::Truncate) + : ""; std::string stringToSign = Permissions + "\n" + startsOnStr + "\n" + expiresOnStr + "\n" + canonicalName + "\n" + Identifier + "\n" + (IPRange.HasValue() ? IPRange.GetValue() : "")