From 6a34583c8a6b3678a653b9819f1427a24c12af66 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Tue, 24 May 2022 17:37:53 -0700 Subject: [PATCH 01/28] Added telemetry support for HTTP pipeline elements --- sdk/attestation/test-resources.json | 4 +- .../tracing/opentelemetry/opentelemetry.hpp | 4 + .../src/opentelemetry.cpp | 15 + .../test/ut/service_support_test.cpp | 27 +- sdk/core/azure-core/CMakeLists.txt | 4 +- .../azure-core/inc/azure/core/context.hpp | 5 +- .../azure-core/inc/azure/core/http/http.hpp | 14 +- .../inc/azure/core/http/policies/policy.hpp | 33 ++ .../inc/azure/core/internal/http/pipeline.hpp | 10 +- .../core/internal/tracing/service_tracing.hpp | 21 + .../inc/azure/core/io/body_stream.hpp | 8 +- .../inc/azure/core/tracing/tracing.hpp | 483 +++++++++--------- sdk/core/azure-core/src/http/request.cpp | 20 +- .../src/http/request_activity_policy.cpp | 78 +++ sdk/core/azure-core/src/tracing/tracing.cpp | 12 +- sdk/core/azure-core/test/ut/CMakeLists.txt | 1 + .../test/ut/request_activity_policy_test.cpp | 231 +++++++++ .../test/ut/service_tracing_test.cpp | 17 +- 18 files changed, 729 insertions(+), 258 deletions(-) create mode 100644 sdk/core/azure-core/src/http/request_activity_policy.cpp create mode 100644 sdk/core/azure-core/test/ut/request_activity_policy_test.cpp diff --git a/sdk/attestation/test-resources.json b/sdk/attestation/test-resources.json index 987b5d769f..674f208a82 100644 --- a/sdk/attestation/test-resources.json +++ b/sdk/attestation/test-resources.json @@ -42,7 +42,7 @@ "metadata": { "description": "The application client secret used to run tests." } - }, + } }, "variables": { "isolatedTenantName": "[concat('cp', concat(parameters('baseName'), 'iso'))]", @@ -66,7 +66,7 @@ "type": "Microsoft.Attestation/attestationProviders", "apiVersion": "2020-10-01", "name": "[variables('aadTenantName')]", - "location": "[parameters('location')]", + "location": "[parameters('location')]" }, { "type": "Microsoft.Attestation/attestationProviders", diff --git a/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp b/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp index e840cfd2d4..0a4ba2c726 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp +++ b/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp @@ -114,6 +114,8 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { virtual void AddAttributes( Azure::Core::Tracing::_internal::AttributeSet const& attributeToAdd) override; + virtual void AddAttribute(std::string const& attributeName, std::string const& attributeValue) + override; /** * Add an Event to the span. An event is identified by a name and an optional set of @@ -129,6 +131,8 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { Azure::Core::Tracing::_internal::SpanStatus const& status, std::string const& statusMessage) override; + virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) override; + opentelemetry::trace::SpanContext GetContext() { return m_span->GetContext(); } }; diff --git a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp index 8f0cd0b480..baeaae1138 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp @@ -9,7 +9,9 @@ #pragma warning(push) #pragma warning(disable : 4100) #pragma warning(disable : 4244) +#pragma warning(disable : 6323) #endif +#include #include #include #if defined(_MSC_VER) @@ -185,5 +187,18 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { m_span->SetStatus(statusCode, statusMessage); } + void OpenTelemetrySpan::AddAttribute( + std::string const& attributeName, + std::string const& attributeValue) + { + m_span->SetAttribute(attributeName, opentelemetry::common::AttributeValue(attributeValue)); + } + + void OpenTelemetrySpan::PropagateToHttpHeaders(Azure::Core::Http::Request& request) + { + throw std::runtime_error("Not supported"); + request; + } + } // namespace _detail }}}} // namespace Azure::Core::Tracing::OpenTelemetry diff --git a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp index c48f4abfda..2ee82ad17a 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp @@ -275,7 +275,8 @@ TEST_F(OpenTelemetryServiceTests, SimplestTest) Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( clientOptions, "my-service-cpp", "1.0b2"); - auto contextAndSpan = serviceTrace.CreateSpan("My API", {}); + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); EXPECT_FALSE(contextAndSpan.first.IsCancelled()); } } @@ -311,7 +312,8 @@ TEST_F(OpenTelemetryServiceTests, CreateWithExplicitProvider) clientOptions, "my-service", "1.0beta-2"); Azure::Core::Context clientContext; - auto contextAndSpan = serviceTrace.CreateSpan("My API", clientContext); + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, clientContext); EXPECT_FALSE(contextAndSpan.first.IsCancelled()); } // Now let's verify what was logged via OpenTelemetry. @@ -321,6 +323,7 @@ TEST_F(OpenTelemetryServiceTests, CreateWithExplicitProvider) VerifySpan(spans[0], R"( { "name": "My API", + "kind": "internal", "attributes": { "az.namespace": "my-service" }, @@ -349,7 +352,8 @@ TEST_F(OpenTelemetryServiceTests, CreateWithImplicitProvider) clientOptions, "my-service", "1.0beta-2"); Azure::Core::Context clientContext; - auto contextAndSpan = serviceTrace.CreateSpan("My API", clientContext); + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, clientContext); EXPECT_FALSE(contextAndSpan.first.IsCancelled()); } @@ -360,6 +364,7 @@ TEST_F(OpenTelemetryServiceTests, CreateWithImplicitProvider) VerifySpan(spans[0], R"( { "name": "My API", + "kind": "internal", "attributes": { "az.namespace": "my-service" }, @@ -391,12 +396,14 @@ TEST_F(OpenTelemetryServiceTests, NestSpans) clientOptions, "my-service", "1.0beta-2"); Azure::Core::Context parentContext; - auto contextAndSpan = serviceTrace.CreateSpan("My API", parentContext); + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Client, parentContext); EXPECT_FALSE(contextAndSpan.first.IsCancelled()); parentContext = contextAndSpan.first; { - auto innerContextAndSpan = serviceTrace.CreateSpan("Nested API", parentContext); + auto innerContextAndSpan = serviceTrace.CreateSpan( + "Nested API", Azure::Core::Tracing::_internal::SpanKind::Server, parentContext); EXPECT_FALSE(innerContextAndSpan.first.IsCancelled()); } } @@ -455,7 +462,8 @@ class ServiceClient { std::string const& inputString, Azure::Core::Context const& context = Azure::Core::Context{}) { - auto contextAndSpan = m_serviceTrace.CreateSpan("GetConfigurationString", context); + auto contextAndSpan = m_serviceTrace.CreateSpan( + "GetConfigurationString", Azure::Core::Tracing::_internal::SpanKind::Internal, context); // std::unique_ptr response @@ -473,7 +481,7 @@ class ServiceClient { { auto contextAndSpan = Azure::Core::Tracing::_internal::DiagnosticTracingFactory::CreateSpanFromContext( - "HTTP GET#2", context); + "HTTP GET#2", Azure::Core::Tracing::_internal::SpanKind::Client, context); return std::make_unique( 1, 1, Azure::Core::Http::HttpStatusCode::Ok, "OK"); @@ -490,7 +498,7 @@ class ServiceClient { auto contextAndSpan = Azure::Core::Tracing::_internal::DiagnosticTracingFactory::CreateSpanFromContext( - "HTTP GET#1", context); + "HTTP GET#1", Azure::Core::Tracing::_internal::SpanKind::Client, context); std::unique_ptr response = ActuallySendHttpRequest(contextAndSpan.first); @@ -503,7 +511,8 @@ class ServiceClient { std::string const&, Azure::Core::Context const& context = Azure::Core::Context{}) { - auto contextAndSpan = m_serviceTrace.CreateSpan("ApiWhichThrows", context); + auto contextAndSpan = m_serviceTrace.CreateSpan( + "ApiWhichThrows", Azure::Core::Tracing::_internal::SpanKind::Internal, context); try { diff --git a/sdk/core/azure-core/CMakeLists.txt b/sdk/core/azure-core/CMakeLists.txt index 5bc4145465..6df669298e 100644 --- a/sdk/core/azure-core/CMakeLists.txt +++ b/sdk/core/azure-core/CMakeLists.txt @@ -99,7 +99,8 @@ set( inc/azure/core/tracing/tracing.hpp inc/azure/core/url.hpp inc/azure/core/uuid.hpp - inc/azure/core.hpp) + inc/azure/core.hpp + ) set( AZURE_CORE_SOURCE @@ -113,6 +114,7 @@ set( src/http/policy.cpp src/http/raw_response.cpp src/http/request.cpp + src/http/request_activity_policy.cpp src/http/retry_policy.cpp src/http/telemetry_policy.cpp src/http/transport_policy.cpp diff --git a/sdk/core/azure-core/inc/azure/core/context.hpp b/sdk/core/azure-core/inc/azure/core/context.hpp index 67d55fddb7..166f89f062 100644 --- a/sdk/core/azure-core/inc/azure/core/context.hpp +++ b/sdk/core/azure-core/inc/azure/core/context.hpp @@ -12,7 +12,6 @@ #include "azure/core/datetime.hpp" #include "azure/core/dll_import_export.hpp" #include "azure/core/rtti.hpp" -#include "azure/core/tracing/tracing.hpp" #include #include #include @@ -21,6 +20,10 @@ #include namespace Azure { namespace Core { + // Forward declare TracerProvider to resolve an include file dependency ordering problem. + namespace Tracing { + class TracerProvider; + } /** * @brief An exception thrown when an operation is cancelled. 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 5e88b9d996..a8c8247ab3 100644 --- a/sdk/core/azure-core/inc/azure/core/http/http.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/http.hpp @@ -273,6 +273,16 @@ namespace Azure { namespace Core { namespace Http { */ void SetHeader(std::string const& name, std::string const& value); + /** + * @brief Gets a specific HTTP header from an #Azure::Core::Http::Request. + * + * @param name The name for the header to be retrieved. + * @return The desired header, or an empty nullable if it is not found.. + * + * @throw if \p name is an invalid header key. + */ + Azure::Nullable GetHeader(std::string const& name); + /** * @brief Remove an HTTP header. * @@ -285,11 +295,13 @@ namespace Azure { namespace Core { namespace Http { * @brief Get HttpMethod. * */ - HttpMethod GetMethod() const; + HttpMethod const& GetMethod() const; /** * @brief Get HTTP headers. * + * @remark Note that this function return a COPY of the headers for this request. + * */ CaseInsensitiveMap GetHeaders() const; diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index 65c394d40b..5c4c599f3c 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -382,6 +382,39 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { } }; + /** + * @brief HTTP Request Activity policy. + * + * @details Registers an HTTP request with the distributed tracing infrastructure, adding + * the traceparent header to the request if necessary. + * + * This policy is intended to be inserted into the HTTP pipeline *after* the retry policy. + */ + class RequestActivityPolicy final : public HttpPolicy { + private: + // Count of times the "Send" method has been called on the RequestActivityPolicy. + // Because the retry policy is always before this policy in the pipeline, this + // is the retry count for the pipeline. + mutable unsigned int m_retryCount{0}; + + public: + /** + * @brief Constructs HTTP Request Activity policy. + * + */ + explicit RequestActivityPolicy() {} + + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } + + std::unique_ptr Send( + Request& request, + NextHttpPolicy nextPolicy, + Context const& context) const override; + }; + /** * @brief HTTP telemetry policy. * 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 3217ffc0c4..d11084d0c4 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 @@ -78,14 +78,15 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { { auto const& perCallClientPolicies = clientOptions.PerOperationPolicies; auto const& perRetryClientPolicies = clientOptions.PerRetryPolicies; - // Adding 5 for: + // Adding 6 for: // - TelemetryPolicy // - RequestIdPolicy // - RetryPolicy // - LogPolicy + // - RequestActivityPolicy // - TransportPolicy auto pipelineSize = perCallClientPolicies.size() + perRetryClientPolicies.size() - + perRetryPolicies.size() + perCallPolicies.size() + 5; + + perRetryPolicies.size() + perCallPolicies.size() + 6; m_policies.reserve(pipelineSize); @@ -98,6 +99,7 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { // Request Id m_policies.emplace_back( std::make_unique()); + // Telemetry m_policies.emplace_back( std::make_unique( @@ -124,6 +126,10 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { m_policies.emplace_back(policy->Clone()); } + // Add a request activity policy which will generate distributed traces for the pipeline. + m_policies.emplace_back( + std::make_unique < Azure::Core::Http::Policies::_internal::RequestActivityPolicy>()); + // logging - won't update request m_policies.emplace_back( std::make_unique(clientOptions.Log)); diff --git a/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp b/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp index 9b10b2a355..562f55b12a 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp @@ -65,6 +65,7 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { m_span->SetStatus(status, description); } } + /** * @brief Adds a set of attributes to the span. * @@ -78,6 +79,21 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { } } + /** + * @brief Adds a single attributes to the span. + * + * @param attributeName Name of the attribute to be added. + * @param attributeValue Value of the attribute to be added. + */ + virtual void AddAttribute(std::string const& attributeName, std::string const& attributeValue) + override + { + if (m_span) + { + m_span->AddAttribute(attributeName, attributeValue); + } + } + /** * @brief Adds an event to the span. * @@ -123,6 +139,9 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { m_span->AddEvent(exception); } } + virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& ) override { + throw std::runtime_error("Not implemented"); + } }; /** @@ -178,10 +197,12 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { ContextAndSpan CreateSpan( std::string const& spanName, + Azure::Core::Tracing::_internal::SpanKind const& spanKind, Azure::Core::Context const& clientContext); static ContextAndSpan CreateSpanFromContext( std::string const& spanName, + Azure::Core::Tracing::_internal::SpanKind const& spanKind, Azure::Core::Context const& clientContext); std::unique_ptr CreateAttributeSet(); diff --git a/sdk/core/azure-core/inc/azure/core/io/body_stream.hpp b/sdk/core/azure-core/inc/azure/core/io/body_stream.hpp index d805ab203d..f71d631945 100644 --- a/sdk/core/azure-core/inc/azure/core/io/body_stream.hpp +++ b/sdk/core/azure-core/inc/azure/core/io/body_stream.hpp @@ -162,8 +162,8 @@ namespace Azure { namespace Core { namespace IO { namespace _internal { /** - * @brief A concrete implementation of #Azure::Core::IO::BodyStream used for reading data from - * a file from any offset and length within it. + * @brief A concrete implementation of #Azure::Core::IO::BodyStream used for reading data + * from a file from any offset and length within it. */ class RandomAccessFileBodyStream final : public BodyStream { private: @@ -284,8 +284,8 @@ namespace Azure { namespace Core { namespace IO { }; /** - * @brief A concrete implementation of #Azure::Core::IO::BodyStream that wraps another stream and - * reports progress + * @brief A concrete implementation of #Azure::Core::IO::BodyStream that wraps another stream + * and reports progress */ class ProgressBodyStream : public BodyStream { private: diff --git a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp index 71663cf4fb..69d3d4001f 100644 --- a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp +++ b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp @@ -17,273 +17,294 @@ #include #include -namespace Azure { namespace Core { namespace Tracing { +//#include "azure/core/http/http.hpp" - namespace _internal { +namespace Azure { namespace Core { + // Forward declare Azure::Core::Http::Request to resolve an include file ordering problem. + namespace Http { + class Request; + } + namespace Tracing { - /** The set of attributes to be applied to a Span or Event. - * - * @details - * An AttributeSet represents a set of attributes to be added to a span or - * event. - * - * @note Note that AttributeSets do *NOT* take a copy of their input values, - * it is the responsibility of the caller to ensure that the object remains - * valid between when it is added to the AttributeSet and when it is added to - * a span or event. - * - * OpenTelemetry property bags can hold: - * - bool - * - int32_t - * - int64_t - * - uint64_t - * - double - * - const char * - * - std::string/std::string_view *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - uint64_t (not fully supported) *** Not Implemented - * - std::span (not fully supported) *** Not Implemented - * - std::span (not fully supported) *** Not Implemented. - * - */ - class AttributeSet { - public: - /** - * @brief Adds a Boolean attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, bool value) = 0; - /** - * @brief Adds a 32bit integer attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, int32_t value) = 0; - /** - * @brief Adds a 64bit integer attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, int64_t value) = 0; - /** - * @brief Adds an unsigned 64bit integer attribute to the attribute set. + namespace _internal { + + /** The set of attributes to be applied to a Span or Event. * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, uint64_t value) = 0; - /** - * @brief Adds a 64bit floating point attribute to the attribute set. + * @details + * An AttributeSet represents a set of attributes to be added to a span or + * event. * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, double value) = 0; - /** - * @brief Adds a C style string attribute to the attribute set. + * @note Note that AttributeSets do *NOT* take a copy of their input values, + * it is the responsibility of the caller to ensure that the object remains + * valid between when it is added to the AttributeSet and when it is added to + * a span or event. * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, const char* value) = 0; - /** - * @brief Adds a C++ string attribute to the attribute set. + * OpenTelemetry property bags can hold: + * - bool + * - int32_t + * - int64_t + * - uint64_t + * - double + * - const char * + * - std::string/std::string_view *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - uint64_t (not fully supported) *** Not Implemented + * - std::span (not fully supported) *** Not Implemented + * - std::span (not fully supported) *** Not Implemented. * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. */ - virtual void AddAttribute(std::string const& attributeName, std::string const& value) = 0; + class AttributeSet { + public: + /** + * @brief Adds a Boolean attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, bool value) = 0; + /** + * @brief Adds a 32bit integer attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, int32_t value) = 0; + /** + * @brief Adds a 64bit integer attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, int64_t value) = 0; + /** + * @brief Adds an unsigned 64bit integer attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, uint64_t value) = 0; + /** + * @brief Adds a 64bit floating point attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, double value) = 0; + /** + * @brief Adds a C style string attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, const char* value) = 0; + /** + * @brief Adds a C++ string attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, std::string const& value) = 0; - /** - * @brief destroys an AttributeSet - virtual destructor to enable base class users to destroy - * derived classes. + /** + * @brief destroys an AttributeSet - virtual destructor to enable base class users to + * destroy derived classes. + */ + virtual ~AttributeSet(){}; + }; + + /** @brief The Type of Span. */ - virtual ~AttributeSet(){}; - }; + class SpanKind final : public Azure::Core::_internal::ExtendableEnumeration { + public: + explicit SpanKind(std::string const& kind) : ExtendableEnumeration(kind) {} + SpanKind() = default; - /** @brief The Type of Span. - */ - class SpanKind final : public Azure::Core::_internal::ExtendableEnumeration { - public: - explicit SpanKind(std::string const& kind) : ExtendableEnumeration(kind) {} - SpanKind() = default; + /** + * @brief Represents an "Internal" operation. + * + */ + AZ_CORE_DLLEXPORT const static SpanKind Internal; + /** + * @brief Represents a request to a remote service. + * + */ + AZ_CORE_DLLEXPORT const static SpanKind Client; + /** + * @brief Represents a span covering the server side handling of an API call. + * + */ + AZ_CORE_DLLEXPORT const static SpanKind Server; + /** + * @brief Represents the initiator of an asynchronous request. + * + */ + AZ_CORE_DLLEXPORT const static SpanKind Producer; + /** + * @brief Represents a span which describes a child of a producer request. + * + */ + AZ_CORE_DLLEXPORT const static SpanKind Consumer; + }; /** - * @brief Represents an "Internal" operation. - * - */ - AZ_CORE_DLLEXPORT const static SpanKind Internal; - /** - * @brief Represents a request to a remote service. - * - */ - AZ_CORE_DLLEXPORT const static SpanKind Client; - /** - * @brief Represents a span covering the server side handling of an API call. - * - */ - AZ_CORE_DLLEXPORT const static SpanKind Server; - /** - * @brief Represents the initiator of an asynchronous request. - * - */ - AZ_CORE_DLLEXPORT const static SpanKind Producer; - /** - * @brief Represents a span which describes a child of a producer request. - * + * @brief Represents the status of a span. */ - AZ_CORE_DLLEXPORT const static SpanKind Consumer; - }; + class SpanStatus final : public Azure::Core::_internal::ExtendableEnumeration { - /** - * @brief Represents the status of a span. - */ - class SpanStatus final : public Azure::Core::_internal::ExtendableEnumeration { + public: + explicit SpanStatus(std::string const& status) : ExtendableEnumeration(status) {} + SpanStatus() = default; - public: - explicit SpanStatus(std::string const& status) : ExtendableEnumeration(status) {} - SpanStatus() = default; + /** + * @brief The default status of a span. + */ + AZ_CORE_DLLEXPORT const static SpanStatus Unset; + /** + * @brief The operation has completed successfully. + */ + AZ_CORE_DLLEXPORT const static SpanStatus Ok; + /** + * @brief The operation contains an error. + */ + AZ_CORE_DLLEXPORT const static SpanStatus Error; + }; /** - * @brief The default status of a span. + * @brief Span - represents a span in tracing. */ - AZ_CORE_DLLEXPORT const static SpanStatus Unset; - /** - * @brief The operation has completed successfully. - */ - AZ_CORE_DLLEXPORT const static SpanStatus Ok; - /** - * @brief The operation contains an error. - */ - AZ_CORE_DLLEXPORT const static SpanStatus Error; - }; + class Span { + public: + /** + * @brief Signals that the span has now ended. + */ + virtual void End(Azure::Nullable endTime = {}) = 0; - /** - * @brief Span - represents a span in tracing. - */ - class Span { - public: - /** - * @brief Signals that the span has now ended. - */ - virtual void End(Azure::Nullable endTime = {}) = 0; + /** + * @brief Adds a set of attributes to the span. + * + * @param attributeToAdd Attributes to be added to the span. + */ + virtual void AddAttributes(AttributeSet const& attributeToAdd) = 0; - /** - * @brief Adds a set of attributes to the span. - * - * @param attributeToAdd Attributes to be added to the span. - */ - virtual void AddAttributes(AttributeSet const& attributeToAdd) = 0; + /** + * @brief Adds a single string valued attribute to the span. + * + * @param attributeName Name of the attribute to add. + * @param attributeValue value of the attribute. + */ + virtual void AddAttribute( + std::string const& atributeName, + std::string const& attributeValue) + = 0; - /** - * @brief Adds an event to the span. - * - * Add an Event to the span. An event is identified by a name and an optional set of - * attributes associated with the event. - * - * @param eventName Name of the event to add. - * @param eventAttributes Attributes associated with the event. - */ - virtual void AddEvent(std::string const& eventName, AttributeSet const& eventAttributes) = 0; + /** + * @brief Adds an event to the span. + * + * Add an Event to the span. An event is identified by a name and an optional set of + * attributes associated with the event. + * + * @param eventName Name of the event to add. + * @param eventAttributes Attributes associated with the event. + */ + virtual void AddEvent(std::string const& eventName, AttributeSet const& eventAttributes) + = 0; - /** - * @brief Adds an event to the span. - * - * Add an Event to the span. An event is identified by a name - * - * @param eventName Name of the event to add. - */ - virtual void AddEvent(std::string const& eventName) = 0; - /** - * @brief Records an exception occurring in the span. - * - * @param exception Exception which has occurred. - */ - virtual void AddEvent(std::exception const& exception) = 0; + /** + * @brief Adds an event to the span. + * + * Add an Event to the span. An event is identified by a name + * + * @param eventName Name of the event to add. + */ + virtual void AddEvent(std::string const& eventName) = 0; + /** + * @brief Records an exception occurring in the span. + * + * @param exception Exception which has occurred. + */ + virtual void AddEvent(std::exception const& exception) = 0; - /** - * @brief Set the Status of the span - * - * @param status Updated status of the span. - * @param description A description associated with the Status. - */ - virtual void SetStatus(SpanStatus const& status, std::string const& description = "") = 0; - }; + /** + * @brief Set the Status of the span + * + * @param status Updated status of the span. + * @param description A description associated with the Status. + */ + virtual void SetStatus(SpanStatus const& status, std::string const& description = "") = 0; + virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) = 0; + }; - /** - * @brief Options used while creating a span. - * - */ - struct CreateSpanOptions final - { /** - * @brief The kind of span to be created. + * @brief Options used while creating a span. * */ - SpanKind Kind{SpanKind::Internal}; + struct CreateSpanOptions final + { + /** + * @brief The kind of span to be created. + * + */ + SpanKind Kind{SpanKind::Internal}; + /** + * @brief Attributes associated with the span. + * + */ + std::unique_ptr Attributes; + + /** + * @brief Parent for the newly created span. + */ + std::shared_ptr ParentSpan; + }; + /** - * @brief Attributes associated with the span. + * @brief Tracer - factory for creating span objects. * */ - std::unique_ptr Attributes; + class Tracer { + public: + /** + * @brief Create new Span object. + * + * @details Creates a new span object. + * + * @note There is no concept of a "current" span, each span created is a top level span, + * unless the CreateSpanOptions has ParentSpan member, in which case the ParentSpan member + * will be the parent of the newly created span. + * + * @param spanName The name of the span to create. + * @param options Options to be used when creating the span. + * @return std::shared_ptr Newly created span. + */ + virtual std::shared_ptr CreateSpan( + std::string const& spanName, + CreateSpanOptions const& options = {}) const = 0; - /** - * @brief Parent for the newly created span. - */ - std::shared_ptr ParentSpan; - }; + virtual std::unique_ptr CreateAttributeSet() const = 0; + }; + } // namespace _internal /** - * @brief Tracer - factory for creating span objects. - * + * @brief Trace Provider - factory for creating Tracer objects. */ - class Tracer { + class TracerProvider { public: /** - * @brief Create new Span object. - * - * @details Creates a new span object. + * @brief Create a Tracer object * - * @note There is no concept of a "current" span, each span created is a top level span, - * unless the CreateSpanOptions has ParentSpan member, in which case the ParentSpan member - * will be the parent of the newly created span. - * - * @param spanName The name of the span to create. - * @param options Options to be used when creating the span. - * @return std::shared_ptr Newly created span. + * @param name Name of the tracer object, typically the name of the Service client + * (Azure.Storage.Blobs, for example) + * @param version Version of the service client. + * @return std::shared_ptr */ - virtual std::shared_ptr CreateSpan( - std::string const& spanName, - CreateSpanOptions const& options = {}) const = 0; - - virtual std::unique_ptr CreateAttributeSet() const = 0; + virtual std::shared_ptr CreateTracer( + std::string const& name, + std::string const& version = "") const = 0; }; - } // namespace _internal - - /** - * @brief Trace Provider - factory for creating Tracer objects. - */ - class TracerProvider { - public: - /** - * @brief Create a Tracer object - * - * @param name Name of the tracer object, typically the name of the Service client - * (Azure.Storage.Blobs, for example) - * @param version Version of the service client. - * @return std::shared_ptr - */ - virtual std::shared_ptr CreateTracer( - std::string const& name, - std::string const& version = "") const = 0; - }; -}}} // namespace Azure::Core::Tracing + } // namespace Tracing +}} // namespace Azure::Core diff --git a/sdk/core/azure-core/src/http/request.cpp b/sdk/core/azure-core/src/http/request.cpp index 0fa5035076..ba768de9a8 100644 --- a/sdk/core/azure-core/src/http/request.cpp +++ b/sdk/core/azure-core/src/http/request.cpp @@ -22,6 +22,24 @@ static Azure::Core::CaseInsensitiveMap MergeMaps( } } // namespace +Azure::Nullable Request::GetHeader(std::string const& name) +{ + std::vector returnedHeaders; + auto headerNameLowerCase = Azure::Core::_internal::StringExtensions::ToLower(name); + + auto retryHeader = this->m_retryHeaders.find(headerNameLowerCase); + if (retryHeader != this->m_retryHeaders.end()) + { + return retryHeader->second; + } + auto header = this->m_headers.find(headerNameLowerCase); + if (header != this->m_headers.end()) + { + return header->second; + } + return Azure::Nullable{}; +} + void Request::SetHeader(std::string const& name, std::string const& value) { auto headerNameLowerCase = Azure::Core::_internal::StringExtensions::ToLower(name); @@ -50,7 +68,7 @@ void Request::StartTry() } } -HttpMethod Request::GetMethod() const { return this->m_method; } +HttpMethod const& Request::GetMethod() const { return this->m_method; } Azure::Core::CaseInsensitiveMap Request::GetHeaders() const { diff --git a/sdk/core/azure-core/src/http/request_activity_policy.cpp b/sdk/core/azure-core/src/http/request_activity_policy.cpp new file mode 100644 index 0000000000..ada1834f09 --- /dev/null +++ b/sdk/core/azure-core/src/http/request_activity_policy.cpp @@ -0,0 +1,78 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#include "azure/core/http/policies/policy.hpp" +#include "azure/core/internal/diagnostics/log.hpp" +#include "azure/core/internal/tracing/service_tracing.hpp" + +#include +#include +#include +#include +#include + +using Azure::Core::Context; +using namespace Azure::Core::Http; +using namespace Azure::Core::Http::Policies; +using namespace Azure::Core::Http::Policies::_internal; +using namespace Azure::Core::Tracing::_internal; + +std::unique_ptr RequestActivityPolicy::Send( + Request& request, + NextHttpPolicy nextPolicy, + Context const& context) const +{ + // Create a tracing span over the HTTP request. + std::stringstream ss; + ss << "HTTP " << request.GetMethod().ToString() << " #" << m_retryCount; + auto contextAndSpan + = Azure::Core::Tracing::_internal::DiagnosticTracingFactory::CreateSpanFromContext( + ss.str(), SpanKind::Client, context); + auto scope = std::move(contextAndSpan.second); + + scope.AddAttribute(TracingAttributes::HttpMethod.ToString(), request.GetMethod().ToString()); + scope.AddAttribute( + "http.url", + /* _sanitizer.SanitizeUrl(message.Request.Uri.ToString())*/ + request.GetUrl().GetAbsoluteUrl()); + { + Azure::Nullable requestId = request.GetHeader("x-ms-client-request-id"); + if (requestId.HasValue()) + { + scope.AddAttribute(TracingAttributes::RequestId.ToString(), requestId.Value()); + } + } + + { + auto userAgent = request.GetHeader("User-Agent"); + if (userAgent.HasValue()) + { + scope.AddAttribute(TracingAttributes::HttpUserAgent.ToString(), userAgent.Value()); + } + } + try + { + auto response = nextPolicy.Send(request, contextAndSpan.first); + + scope.AddAttribute( + TracingAttributes::HttpStatusCode.ToString(), + std::to_string(static_cast(response->GetStatusCode()))); + auto& responseHeaders = response->GetHeaders(); + auto serviceRequestId = responseHeaders.find("serviceRequestId"); + if (serviceRequestId != responseHeaders.end()) + { + scope.AddAttribute(TracingAttributes::ServiceRequestId.ToString(), serviceRequestId->second); + } + + m_retryCount += 1; + + return response; + } + catch (const TransportException& e) + { + scope.AddEvent(e); + + // Rethrow the exception. + throw; + } +} \ No newline at end of file diff --git a/sdk/core/azure-core/src/tracing/tracing.cpp b/sdk/core/azure-core/src/tracing/tracing.cpp index d36a5301a3..920c51c7d0 100644 --- a/sdk/core/azure-core/src/tracing/tracing.cpp +++ b/sdk/core/azure-core/src/tracing/tracing.cpp @@ -14,9 +14,16 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { const SpanStatus SpanStatus::Error("Error"); const TracingAttributes TracingAttributes::AzNamespace("az.namespace"); + const TracingAttributes TracingAttributes::ServiceRequestId("serviceRequestId"); + const TracingAttributes TracingAttributes::HttpUserAgent("http.user_agent"); + const TracingAttributes TracingAttributes::HttpMethod("http.method"); + const TracingAttributes TracingAttributes::HttpUrl("http.url"); + const TracingAttributes TracingAttributes::RequestId("requestId"); + const TracingAttributes TracingAttributes::HttpStatusCode("http.status_code"); DiagnosticTracingFactory::ContextAndSpan DiagnosticTracingFactory::CreateSpan( std::string const& methodName, + Azure::Core::Tracing::_internal::SpanKind const& spanKind, Azure::Core::Context const& context) { CreateSpanOptions createOptions; @@ -47,6 +54,8 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { createOptions.Attributes->AddAttribute( TracingAttributes::AzNamespace.ToString(), m_serviceName); + createOptions.Kind = spanKind; + std::shared_ptr newSpan(m_serviceTracer->CreateSpan(methodName, createOptions)); TracingContext tracingContext = newSpan; Azure::Core::Context newContext = contextToUse.WithValue(ContextSpanKey, tracingContext); @@ -61,13 +70,14 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { } DiagnosticTracingFactory::ContextAndSpan DiagnosticTracingFactory::CreateSpanFromContext( std::string const& spanName, + Azure::Core::Tracing::_internal::SpanKind const& spanKind, Azure::Core::Context const& context) { DiagnosticTracingFactory* tracingFactory = DiagnosticTracingFactory::DiagnosticFactoryFromContext(context); if (tracingFactory) { - return tracingFactory->CreateSpan(spanName, context); + return tracingFactory->CreateSpan(spanName, spanKind, context); } else { diff --git a/sdk/core/azure-core/test/ut/CMakeLists.txt b/sdk/core/azure-core/test/ut/CMakeLists.txt index f50f787a1a..bd75517ce3 100644 --- a/sdk/core/azure-core/test/ut/CMakeLists.txt +++ b/sdk/core/azure-core/test/ut/CMakeLists.txt @@ -66,6 +66,7 @@ add_executable ( operation_status_test.cpp pipeline_test.cpp policy_test.cpp + request_activity_policy_test.cpp request_id_policy_test.cpp response_t_test.cpp retry_policy_test.cpp diff --git a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp new file mode 100644 index 0000000000..d1f118d723 --- /dev/null +++ b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp @@ -0,0 +1,231 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#include "azure/core/http/policies/policy.hpp" +#include "azure/core/internal/http/pipeline.hpp" +#include "azure/core/internal/tracing/service_tracing.hpp" +#include "azure/core/tracing/tracing.hpp" +#include + +using namespace Azure::Core; +using namespace Azure::Core::Http; +using namespace Azure::Core::Http::Policies; +using namespace Azure::Core::Http::Policies::_internal; +using namespace Azure::Core::Tracing::_internal; +using namespace Azure::Core::Tracing; + +namespace { +class NoOpPolicy final : public HttpPolicy { +public: + std::unique_ptr Clone() const override { return std::make_unique(*this); } + + std::unique_ptr Send(Request&, NextHttpPolicy, Azure::Core::Context const&) + const override + { + return std::make_unique(1, 1, HttpStatusCode::Ok, "Something"); + + } +}; + +} // namespace + +// Dummy service tracing class. +class TestSpan final : public Azure::Core::Tracing::_internal::Span { + std::vector m_events; + std::map m_stringAttributes; + std::string m_spanName; + +public: + TestSpan(std::string const& spanName) + : Azure::Core::Tracing::_internal::Span(), m_spanName(spanName) + { + } + + // Inherited via Span + virtual void AddAttributes(AttributeSet const&) override {} + virtual void AddAttribute(std::string const& attributeName, std::string const& attributeValue) + override + { + m_stringAttributes.emplace(std::make_pair(attributeName, attributeValue)); + } + virtual void AddEvent(std::string const& eventName, AttributeSet const&) override + { + m_events.push_back(eventName); + } + virtual void AddEvent(std::string const& eventName) override { m_events.push_back(eventName); } + virtual void AddEvent(std::exception const& ex) override { m_events.push_back(ex.what()); } + virtual void SetStatus(SpanStatus const&, std::string const&) override {} + + // Inherited via Span + virtual void End(Azure::Nullable) override {} + + // Inherited via Span + virtual void PropagateToHttpHeaders(Azure::Core::Http::Request&) override {} + + std::string const& GetName() { return m_spanName; } + std::vector const& GetEvents() { return m_events; } + std::map const& GetAttributes() { return m_stringAttributes; } +}; + +class TestAttributeSet : public Azure::Core::Tracing::_internal::AttributeSet { +public: + TestAttributeSet() : Azure::Core::Tracing::_internal::AttributeSet() {} + + // Inherited via AttributeSet + virtual void AddAttribute(std::string const&, bool) override {} + virtual void AddAttribute(std::string const&, int32_t) override {} + virtual void AddAttribute(std::string const&, int64_t) override {} + virtual void AddAttribute(std::string const&, uint64_t) override {} + virtual void AddAttribute(std::string const&, double) override {} + virtual void AddAttribute(std::string const&, const char*) override {} + virtual void AddAttribute(std::string const&, std::string const&) override {} +}; + +class TestTracer final : public Azure::Core::Tracing::_internal::Tracer { + mutable std::vector> m_spans; + +public: + TestTracer(std::string const&, std::string const&) : Azure::Core::Tracing::_internal::Tracer() {} + std::shared_ptr CreateSpan(std::string const& spanName, CreateSpanOptions const&) + const override + { + auto returnSpan(std::make_shared(spanName)); + m_spans.push_back(returnSpan); + return returnSpan; + } + + std::unique_ptr CreateAttributeSet() const override + { + return std::make_unique(); + }; + + std::vector> const& GetSpans() { return m_spans; } +}; + +class TestTracingProvider final : public Azure::Core::Tracing::TracerProvider { + mutable std::list> m_tracers; + +public: + TestTracingProvider() : TracerProvider() {} + ~TestTracingProvider() {} + std::shared_ptr CreateTracer( + std::string const& serviceName, + std::string const& serviceVersion) const override + { + auto returnTracer = std::make_shared(serviceName, serviceVersion); + m_tracers.push_back(returnTracer); + return returnTracer; + }; + + std::list> const& GetTracers() { return m_tracers; } +}; + +TEST(RequestActivityPolicy, Basic) +{ + { + auto testTracer = std::make_shared(); + + Azure::Core::_internal::ClientOptions clientOptions; + clientOptions.Telemetry.TracingProvider = testTracer; + Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( + clientOptions, "my-service-cpp", "1.0b2"); + + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); + Azure::Core::Context callContext = std::move(contextAndSpan.first); + Request request(HttpMethod::Get, Url("https://www.microsoft.com")); + + { + std::vector> policies; + // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. + policies.emplace_back(std::make_unique()); + // Final policy - equivalent to HTTP policy. + policies.emplace_back(std::make_unique()); + + Azure::Core::Http::_internal::HttpPipeline(policies).Send(request, callContext); + } + + EXPECT_EQ(1ul, testTracer->GetTracers().size()); + auto tracer = testTracer->GetTracers().front(); + EXPECT_EQ(2ul, tracer->GetSpans().size()); + EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); + EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); + EXPECT_EQ("GET", tracer->GetSpans()[1]->GetAttributes().at("http.method")); + } + + // Now try with the request ID and telemetry policies (simulating a more complete pipeline). + { + auto testTracer = std::make_shared(); + + Azure::Core::_internal::ClientOptions clientOptions; + clientOptions.Telemetry.TracingProvider = testTracer; + Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( + clientOptions, "my-service-cpp", "1.0b2"); + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); + Azure::Core::Context callContext = std::move(contextAndSpan.first); + Request request(HttpMethod::Get, Url("https://www.microsoft.com")); + + { + std::vector> policies; + // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. + policies.emplace_back(std::make_unique()); + policies.emplace_back( + std::make_unique("my-service-cpp", "1.0b2", clientOptions.Telemetry)); + policies.emplace_back(std::make_unique()); + // Final policy - equivalent to HTTP policy. + policies.emplace_back(std::make_unique()); + + Azure::Core::Http::_internal::HttpPipeline(policies).Send(request, callContext); + } + + EXPECT_EQ(1ul, testTracer->GetTracers().size()); + auto tracer = testTracer->GetTracers().front(); + EXPECT_EQ(2ul, tracer->GetSpans().size()); + EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); + EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); + EXPECT_EQ("GET", tracer->GetSpans()[1]->GetAttributes().at("http.method")); + + } +} + +TEST(RequestActivityPolicy, TryRetries) +{ + { + auto testTracer = std::make_shared(); + + Azure::Core::_internal::ClientOptions clientOptions; + clientOptions.Telemetry.TracingProvider = testTracer; + Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( + clientOptions, "my-service-cpp", "1.0b2"); + + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); + Azure::Core::Context callContext = std::move(contextAndSpan.first); + Request request(HttpMethod::Get, Url("https://www.microsoft.com")); + + { + std::vector> policies; + // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. + policies.emplace_back(std::make_unique()); + // Final policy - equivalent to HTTP policy. + policies.emplace_back(std::make_unique()); + + Azure::Core::Http::_internal::HttpPipeline pipeline(policies); + // Simulate retrying an HTTP operation 3 times on the pipeline: + pipeline.Send(request, callContext); + pipeline.Send(request, callContext); + pipeline.Send(request, callContext); + } + + EXPECT_EQ(1ul, testTracer->GetTracers().size()); + auto tracer = testTracer->GetTracers().front(); + EXPECT_EQ(4ul, tracer->GetSpans().size()); + EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); + EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); + EXPECT_EQ("HTTP GET #1", tracer->GetSpans()[2]->GetName()); + EXPECT_EQ("HTTP GET #2", tracer->GetSpans()[3]->GetName()); + EXPECT_EQ("GET", tracer->GetSpans()[1]->GetAttributes().at("http.method")); + EXPECT_EQ("200", tracer->GetSpans()[1]->GetAttributes().at("http.status_code")); + } +} diff --git a/sdk/core/azure-core/test/ut/service_tracing_test.cpp b/sdk/core/azure-core/test/ut/service_tracing_test.cpp index 68e205134d..70e06e20dc 100644 --- a/sdk/core/azure-core/test/ut/service_tracing_test.cpp +++ b/sdk/core/azure-core/test/ut/service_tracing_test.cpp @@ -48,7 +48,8 @@ TEST(DiagnosticTracingFactory, SimpleServiceSpanTests) Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( clientOptions, "my-service-cpp", "1.0b2"); - auto contextAndSpan = serviceTrace.CreateSpan("My API", {}); + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); EXPECT_FALSE(contextAndSpan.first.IsCancelled()); } } @@ -60,6 +61,7 @@ class TestSpan final : public Azure::Core::Tracing::_internal::Span { // Inherited via Span virtual void AddAttributes(AttributeSet const&) override {} + virtual void AddAttribute(std::string const&, std::string const&) override {} virtual void AddEvent(std::string const&, AttributeSet const&) override {} virtual void AddEvent(std::string const&) override {} virtual void AddEvent(std::exception const&) override {} @@ -67,6 +69,9 @@ class TestSpan final : public Azure::Core::Tracing::_internal::Span { // Inherited via Span virtual void End(Azure::Nullable) override {} + + // Inherited via Span + virtual void PropagateToHttpHeaders(Azure::Core::Http::Request&) override {} }; class TestAttributeSet : public Azure::Core::Tracing::_internal::AttributeSet { @@ -115,8 +120,9 @@ TEST(DiagnosticTracingFactory, BasicServiceSpanTests) Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( clientOptions, "my-service-cpp", "1.0b2"); - auto contextAndSpan = serviceTrace.CreateSpan("My API", {}); - auto span = std::move(contextAndSpan.second); + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); + ServiceSpan span = std::move(contextAndSpan.second); span.End(); span.AddEvent("New Event"); @@ -130,8 +136,9 @@ TEST(DiagnosticTracingFactory, BasicServiceSpanTests) Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( clientOptions, "my-service-cpp", "1.0b2"); - auto contextAndSpan = serviceTrace.CreateSpan("My API", {}); - auto span = std::move(contextAndSpan.second); + auto contextAndSpan = serviceTrace.CreateSpan( + "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); + ServiceSpan span = std::move(contextAndSpan.second); span.End(); span.AddEvent("New Event"); From 6c716272f72e7c58dac4a292e559bdce13d84f9b Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 25 May 2022 14:27:11 -0700 Subject: [PATCH 02/28] Finish OpenTelemetry implementation --- .../src/interceptor_manager.cpp | 27 +-- .../CHANGELOG.md | 1 + .../src/opentelemetry.cpp | 42 ++++- .../test/ut/service_support_test.cpp | 178 ++++++++++++------ sdk/core/azure-core/CHANGELOG.md | 2 +- sdk/core/azure-core/CMakeLists.txt | 53 +++--- .../inc/azure/core/http/policies/policy.hpp | 7 +- .../inc/azure/core/internal/http/pipeline.hpp | 2 +- .../azure/core/internal/request_sanitizer.hpp | 16 ++ .../core/internal/tracing/service_tracing.hpp | 7 +- .../inc/azure/core/tracing/tracing.hpp | 2 +- .../src/http/request_activity_policy.cpp | 24 ++- .../src/private/request_sanitizer.cpp | 36 ++++ .../test/ut/request_activity_policy_test.cpp | 47 ++++- 14 files changed, 312 insertions(+), 132 deletions(-) create mode 100644 sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp create mode 100644 sdk/core/azure-core/src/private/request_sanitizer.cpp diff --git a/sdk/core/azure-core-test/src/interceptor_manager.cpp b/sdk/core/azure-core-test/src/interceptor_manager.cpp index 60065922d2..98d81f54b0 100644 --- a/sdk/core/azure-core-test/src/interceptor_manager.cpp +++ b/sdk/core/azure-core-test/src/interceptor_manager.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include "azure/core/test/interceptor_manager.hpp" @@ -81,29 +82,5 @@ void InterceptorManager::LoadTestData() Url InterceptorManager::RedactUrl(Url const& url) { - Azure::Core::Url redactedUrl; - redactedUrl.SetScheme(url.GetScheme()); - auto host = url.GetHost(); - auto hostWithNoAccount = std::find(host.begin(), host.end(), '.'); - redactedUrl.SetHost("REDACTED" + std::string(hostWithNoAccount, host.end())); - // replace any uniqueID from the path for a hardcoded id - // For the regex, we should not assume anything about the version of UUID format being used. So, - // using the most general regex to get any uuid version. - redactedUrl.SetPath(std::regex_replace( - url.GetPath(), - std::regex("[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}"), - "33333333-3333-3333-3333-333333333333")); - // Query parameters - for (auto const& qp : url.GetQueryParameters()) - { - if (qp.first == "sig") - { - redactedUrl.AppendQueryParameter("sig", "REDACTED"); - } - else - { - redactedUrl.AppendQueryParameter(qp.first, qp.second); - } - } - return redactedUrl; + return Azure::Core::_internal::InputSanitizer::SanitizeUrl(url); } diff --git a/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md b/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md index 0877171bac..e953124acf 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md +++ b/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md @@ -2,4 +2,5 @@ ## 1.0.0-beta.1 (Unreleased) +### New Features - Initial release diff --git a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp index baeaae1138..cd59cae510 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp @@ -1,5 +1,8 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT #include "azure/core/tracing/opentelemetry/opentelemetry.hpp" +#include #include #include #include @@ -194,10 +197,45 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { m_span->SetAttribute(attributeName, opentelemetry::common::AttributeValue(attributeValue)); } + class HttpRequestTextMapPropogater + : public opentelemetry::context::propagation::TextMapCarrier { + Azure::Core::Http::Request& m_request; + // Inherited via TextMapCarrier + virtual opentelemetry::nostd::string_view Get( + opentelemetry::nostd::string_view key) const noexcept override + { + auto header = m_request.GetHeader(std::string(key)); + if (header) + { + return header.Value(); + } + return std::string(); + } + + virtual void Set( + opentelemetry::nostd::string_view key, + opentelemetry::nostd::string_view value) noexcept override + { + m_request.SetHeader(std::string(key), std::string(value)); + } + + public: + HttpRequestTextMapPropogater(Azure::Core::Http::Request& request) : m_request(request) {} + }; + void OpenTelemetrySpan::PropagateToHttpHeaders(Azure::Core::Http::Request& request) { - throw std::runtime_error("Not supported"); - request; + if (m_span) + { + HttpRequestTextMapPropogater propogater(request); + + // Establish the current runtime context from the span. + auto scope = opentelemetry::trace::Tracer::WithActiveSpan(m_span); + auto currentContext = opentelemetry::context::RuntimeContext::GetCurrent(); + + opentelemetry::trace::propagation::HttpTraceContext httpTraceContext; + httpTraceContext.Inject(propogater, currentContext); + } } } // namespace _detail diff --git a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp index 2ee82ad17a..bf68cacde1 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp @@ -4,6 +4,7 @@ #define USE_MEMORY_EXPORTER 1 #include "azure/core/internal/tracing/service_tracing.hpp" #include "azure/core/tracing/opentelemetry/opentelemetry.hpp" +#include #include #include @@ -22,11 +23,17 @@ #include #include #include +#include #if defined(_MSC_VER) #pragma warning(pop) #endif #include #include +#include + +using namespace Azure::Core::Http::Policies; +using namespace Azure::Core::Http::Policies::_internal; +using namespace Azure::Core::Http; class CustomLogHandler : public opentelemetry::sdk::common::internal_log::LogHandler { void Handle( @@ -165,7 +172,7 @@ class OpenTelemetryServiceTests : public Azure::Core::Test::TestBase { switch (span->GetSpanKind()) { case opentelemetry::trace::SpanKind::kClient: - EXPECT_EQ(expectedKind, "internal"); + EXPECT_EQ(expectedKind, "client"); break; case opentelemetry::trace::SpanKind::kConsumer: EXPECT_EQ(expectedKind, "consumer"); @@ -207,7 +214,11 @@ class OpenTelemetryServiceTests : public Azure::Core::Test::TestBase { case opentelemetry::common::kTypeString: { EXPECT_TRUE(expectedAttributes[foundAttribute.first].is_string()); const auto& actualVal = opentelemetry::nostd::get(foundAttribute.second); - EXPECT_EQ(expectedAttributes[foundAttribute.first].get(), actualVal); + std::string expectedVal(expectedAttributes[foundAttribute.first].get()); + std::regex expectedRegex(expectedVal); + GTEST_LOG_(INFO) << "expected Regex: " << expectedVal << std::endl; + GTEST_LOG_(INFO) << "actual val: " << actualVal << std::endl; + EXPECT_TRUE(std::regex_match(actualVal, expectedRegex)); break; } case opentelemetry::common::kTypeDouble: { @@ -388,6 +399,10 @@ TEST_F(OpenTelemetryServiceTests, NestSpans) Azure::Core::Context::ApplicationContext.SetTracerProvider(provider); + Azure::Core::Http::Request outerRequest( + HttpMethod::Post, Azure::Core::Url("https://www.microsoft.com")); + Azure::Core::Http::Request innerRequest( + HttpMethod::Post, Azure::Core::Url("https://www.microsoft.com")); { Azure::Core::_internal::ClientOptions clientOptions; clientOptions.Telemetry.ApplicationId = "MyApplication"; @@ -400,11 +415,13 @@ TEST_F(OpenTelemetryServiceTests, NestSpans) "My API", Azure::Core::Tracing::_internal::SpanKind::Client, parentContext); EXPECT_FALSE(contextAndSpan.first.IsCancelled()); parentContext = contextAndSpan.first; + contextAndSpan.second.PropagateToHttpHeaders(outerRequest); { auto innerContextAndSpan = serviceTrace.CreateSpan( "Nested API", Azure::Core::Tracing::_internal::SpanKind::Server, parentContext); EXPECT_FALSE(innerContextAndSpan.first.IsCancelled()); + innerContextAndSpan.second.PropagateToHttpHeaders(innerRequest); } } // Now let's verify what was logged via OpenTelemetry. @@ -438,9 +455,73 @@ TEST_F(OpenTelemetryServiceTests, NestSpans) EXPECT_EQ("my-service", spans[1]->GetInstrumentationLibrary().GetName()); EXPECT_EQ("1.0beta-2", spans[0]->GetInstrumentationLibrary().GetVersion()); EXPECT_EQ("1.0beta-2", spans[1]->GetInstrumentationLibrary().GetVersion()); + + // The trace ID for the inner and outer requests must be the same, the parent-id/span-id must be + // different. + // + // Returns a 4 element array. + // Array[0] is the version of the TraceParent header. + // Array[1] is the trace-id of the TraceParent header. + // Array[2] is the parent-id/span-id of the TraceParent header. + // Array[3] is the trace-flags of the TraceParent header. + auto ParseTraceParent = [](const std::string& traceParent) -> std::array { + std::array returnedComponents; + std::string component; + size_t index = 0; + for (auto ch : traceParent) + { + if (ch != '-') + { + component.push_back(ch); + } + else + { + returnedComponents[index] = component; + component.clear(); + index += 1; + } + } + EXPECT_EQ(index, 3); + returnedComponents[3] = component; + return returnedComponents; + }; + auto outerTraceId = ParseTraceParent(outerRequest.GetHeader("traceparent").Value()); + auto innerTraceId = ParseTraceParent(innerRequest.GetHeader("traceparent").Value()); + // Version should always match. + EXPECT_EQ(outerTraceId[0], innerTraceId[0]); + // Trace ID should always match. + EXPECT_EQ(outerTraceId[1], innerTraceId[1]); + // Span-Id should never match. + EXPECT_NE(outerTraceId[2], innerTraceId[2]); } } +namespace { +class NoOpPolicy final : public HttpPolicy { + std::function(Request&)> m_createResponse{}; + +public: + std::unique_ptr Clone() const override { return std::make_unique(*this); } + + std::unique_ptr Send(Request& request, NextHttpPolicy, Azure::Core::Context const&) + const override + { + if (m_createResponse) + { + return m_createResponse(request); + } + else + { + return std::make_unique(1, 1, HttpStatusCode::Ok, "Something"); + } + } + + NoOpPolicy() = default; + NoOpPolicy(std::function(Request&)> createResponse) + : HttpPolicy(), m_createResponse(createResponse){}; +}; +} // namespace + // Create a serviceTrace and span using a provider specified in the ClientOptions. class ServiceClientOptions : public Azure::Core::_internal::ClientOptions { public: @@ -451,11 +532,32 @@ class ServiceClient { private: ServiceClientOptions m_clientOptions; Azure::Core::Tracing::_internal::DiagnosticTracingFactory m_serviceTrace; + std::unique_ptr m_pipeline; public: explicit ServiceClient(ServiceClientOptions const& clientOptions = ServiceClientOptions{}) : m_serviceTrace(clientOptions, "Azure.Core.OpenTelemetry.Test.Service", "1.0.0.beta-2") { + std::vector> policies; + policies.emplace_back(std::make_unique( + "Azure.Core.OpenTelemetry.Test.Service", "1.0.0.beta-2", clientOptions.Telemetry)); + policies.emplace_back(std::make_unique()); + policies.emplace_back(std::make_unique(RetryOptions{})); + + // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. + policies.emplace_back(std::make_unique()); + + // Final policy - functions as the HTTP transport policy. + policies.emplace_back(std::make_unique([&](Request& request) { + // If the request is for port 12345, throw an exception. + if (request.GetUrl().GetPort() == 12345) + { + throw Azure::Core::RequestFailedException("it all goes wrong here."); + } + return std::make_unique(1, 1, HttpStatusCode::Ok, "Something"); + })); + + m_pipeline = std::make_unique(policies); } Azure::Response GetConfigurationString( @@ -466,8 +568,11 @@ class ServiceClient { "GetConfigurationString", Azure::Core::Tracing::_internal::SpanKind::Internal, context); // + Azure::Core::Http::Request requestToSend( + HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com/")); + std::unique_ptr response - = SendHttpRequest(false, contextAndSpan.first); + = m_pipeline->Send(requestToSend, contextAndSpan.first); // Reflect that the operation was successful. contextAndSpan.second.SetStatus(Azure::Core::Tracing::_internal::SpanStatus::Ok); @@ -476,37 +581,6 @@ class ServiceClient { // When contextAndSpan.second goes out of scope, it ends the span, which will record it. } - std::unique_ptr ActuallySendHttpRequest( - Azure::Core::Context const& context) - { - auto contextAndSpan - = Azure::Core::Tracing::_internal::DiagnosticTracingFactory::CreateSpanFromContext( - "HTTP GET#2", Azure::Core::Tracing::_internal::SpanKind::Client, context); - - return std::make_unique( - 1, 1, Azure::Core::Http::HttpStatusCode::Ok, "OK"); - } - - std::unique_ptr SendHttpRequest( - bool throwException, - Azure::Core::Context const& context) - { - if (throwException) - { - throw Azure::Core::RequestFailedException("it all goes wrong here."); - } - - auto contextAndSpan - = Azure::Core::Tracing::_internal::DiagnosticTracingFactory::CreateSpanFromContext( - "HTTP GET#1", Azure::Core::Tracing::_internal::SpanKind::Client, context); - - std::unique_ptr response - = ActuallySendHttpRequest(contextAndSpan.first); - - return std::make_unique( - 1, 1, Azure::Core::Http::HttpStatusCode::Ok, "OK"); - } - Azure::Response ApiWhichThrows( std::string const&, Azure::Core::Context const& context = Azure::Core::Context{}) @@ -516,8 +590,13 @@ class ServiceClient { try { - auto rawResponse = SendHttpRequest(false, contextAndSpan.first); - return Azure::Response("", std::move(rawResponse)); + // + Azure::Core::Http::Request requestToSend( + HttpMethod::Get, Azure::Core::Url("https://www.microsoft.com/:12345/index.html")); + + std::unique_ptr response + = m_pipeline->Send(requestToSend, contextAndSpan.first); + return Azure::Response("", std::move(response)); } catch (std::exception const& ex) { @@ -550,15 +629,20 @@ TEST_F(OpenTelemetryServiceTests, ServiceApiImplementation) } // Now let's verify what was logged via OpenTelemetry. auto spans = m_spanData->GetSpans(); - EXPECT_EQ(3ul, spans.size()); + EXPECT_EQ(2ul, spans.size()); VerifySpan(spans[0], R"( { - "name": "HTTP GET#2", - "kind": "internal", + "name": "HTTP GET #0", + "kind": "client", "statusCode": "unset", "attributes": { - "az.namespace": "Azure.Core.OpenTelemetry.Test.Service" + "az.namespace": "Azure.Core.OpenTelemetry.Test.Service", + "http.method": "GET", + "http.url": "https://REDACTED.microsoft.com", + "requestId": ".*", + "http.user_agent": "MyApplication azsdk-cpp-Azure.Core.OpenTelemetry.Test.Service/1.0.0.beta-2.*", + "http.status_code": "200" }, "library": { "name": "Azure.Core.OpenTelemetry.Test.Service", @@ -567,20 +651,6 @@ TEST_F(OpenTelemetryServiceTests, ServiceApiImplementation) })"); VerifySpan(spans[1], R"( -{ - "name": "HTTP GET#1", - "kind": "internal", - "statusCode": "unset", - "attributes": { - "az.namespace": "Azure.Core.OpenTelemetry.Test.Service" - }, - "library": { - "name": "Azure.Core.OpenTelemetry.Test.Service", - "version": "1.0.0.beta-2" - } -})"); - - VerifySpan(spans[2], R"( { "name": "GetConfigurationString", "kind": "internal", diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index ac1c85cb31..52045afd77 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -3,7 +3,7 @@ ## 1.7.0-beta.1 (Unreleased) ### Features Added -Added implementation for Distributed Tracing. +- Added prototypes and initial service support for Distributed Tracing. ### Breaking Changes diff --git a/sdk/core/azure-core/CMakeLists.txt b/sdk/core/azure-core/CMakeLists.txt index 6df669298e..95f9a8d81c 100644 --- a/sdk/core/azure-core/CMakeLists.txt +++ b/sdk/core/azure-core/CMakeLists.txt @@ -38,10 +38,10 @@ endif() if(BUILD_TRANSPORT_CURL) SET(CURL_TRANSPORT_ADAPTER_SRC + src/http/curl/curl.cpp src/http/curl/curl_connection_pool_private.hpp src/http/curl/curl_connection_private.hpp src/http/curl/curl_session_private.hpp - src/http/curl/curl.cpp ) SET(CURL_TRANSPORT_ADAPTER_INC inc/azure/core/http/curl_transport.hpp @@ -56,37 +56,38 @@ set( AZURE_CORE_HEADER ${CURL_TRANSPORT_ADAPTER_INC} ${WIN_TRANSPORT_ADAPTER_INC} + inc/azure/core.hpp + inc/azure/core/azure_assert.hpp + inc/azure/core/base64.hpp + inc/azure/core/case_insensitive_containers.hpp + inc/azure/core/context.hpp inc/azure/core/credentials/credentials.hpp inc/azure/core/credentials/token_credential_options.hpp inc/azure/core/cryptography/hash.hpp + inc/azure/core/datetime.hpp inc/azure/core/diagnostics/logger.hpp - inc/azure/core/http/policies/policy.hpp + inc/azure/core/dll_import_export.hpp + inc/azure/core/etag.hpp + inc/azure/core/exception.hpp inc/azure/core/http/http.hpp inc/azure/core/http/http_status_code.hpp + inc/azure/core/http/policies/policy.hpp inc/azure/core/http/raw_response.hpp inc/azure/core/http/transport.hpp + inc/azure/core/internal/client_options.hpp + inc/azure/core/internal/contract.hpp inc/azure/core/internal/cryptography/sha_hash.hpp inc/azure/core/internal/diagnostics/log.hpp + inc/azure/core/internal/environment.hpp + inc/azure/core/internal/extendable_enumeration.hpp inc/azure/core/internal/http/pipeline.hpp inc/azure/core/internal/io/null_body_stream.hpp inc/azure/core/internal/json/json.hpp inc/azure/core/internal/json/json_optional.hpp inc/azure/core/internal/json/json_serializable.hpp - inc/azure/core/internal/client_options.hpp - inc/azure/core/internal/contract.hpp - inc/azure/core/internal/environment.hpp - inc/azure/core/internal/extendable_enumeration.hpp inc/azure/core/internal/strings.hpp inc/azure/core/internal/tracing/service_tracing.hpp inc/azure/core/io/body_stream.hpp - inc/azure/core/azure_assert.hpp - inc/azure/core/base64.hpp - inc/azure/core/case_insensitive_containers.hpp - inc/azure/core/context.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/match_conditions.hpp inc/azure/core/modified_conditions.hpp inc/azure/core/nullable.hpp @@ -99,15 +100,22 @@ set( inc/azure/core/tracing/tracing.hpp inc/azure/core/url.hpp inc/azure/core/uuid.hpp - inc/azure/core.hpp ) set( AZURE_CORE_SOURCE ${CURL_TRANSPORT_ADAPTER_SRC} ${WIN_TRANSPORT_ADAPTER_SRC} + src/azure_assert.cpp + src/base64.cpp + src/context.cpp src/cryptography/md5.cpp src/cryptography/sha_hash.cpp + src/datetime.cpp + src/environment.cpp + src/environment_log_level_listener.cpp + src/etag.cpp + src/exception.cpp src/http/bearer_token_authentication_policy.cpp src/http/http.cpp src/http/log_policy.cpp @@ -121,21 +129,14 @@ set( src/http/url.cpp src/io/body_stream.cpp src/io/random_access_file_body_stream.cpp - src/private/environment_log_level_listener.hpp - src/private/package_version.hpp - src/azure_assert.cpp - src/base64.cpp - src/context.cpp - src/datetime.cpp - src/environment.cpp - src/environment_log_level_listener.cpp - src/etag.cpp - src/exception.cpp src/logger.cpp src/operation_status.cpp + src/private/environment_log_level_listener.hpp + src/private/package_version.hpp src/strings.cpp + src/tracing/tracing.cpp src/uuid.cpp - src/tracing/tracing.cpp) + "src/private/request_sanitizer.cpp") add_library(azure-core ${AZURE_CORE_HEADER} ${AZURE_CORE_SOURCE}) diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index 5c4c599f3c..db318ac9e1 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -17,6 +17,7 @@ #include "azure/core/tracing/tracing.hpp" #include "azure/core/uuid.hpp" +#include #include #include #include @@ -387,15 +388,11 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { * * @details Registers an HTTP request with the distributed tracing infrastructure, adding * the traceparent header to the request if necessary. - * + * * This policy is intended to be inserted into the HTTP pipeline *after* the retry policy. */ class RequestActivityPolicy final : public HttpPolicy { private: - // Count of times the "Send" method has been called on the RequestActivityPolicy. - // Because the retry policy is always before this policy in the pipeline, this - // is the retry count for the pipeline. - mutable unsigned int m_retryCount{0}; public: /** 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 d11084d0c4..73e22ce77c 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 @@ -128,7 +128,7 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { // Add a request activity policy which will generate distributed traces for the pipeline. m_policies.emplace_back( - std::make_unique < Azure::Core::Http::Policies::_internal::RequestActivityPolicy>()); + std::make_unique()); // logging - won't update request m_policies.emplace_back( diff --git a/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp b/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp new file mode 100644 index 0000000000..47e480960a --- /dev/null +++ b/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#pragma once + +#include +#include "azure/core/url.hpp" + +namespace Azure { namespace Core { namespace _internal { + class InputSanitizer final { + + public: + static Url SanitizeUrl(Url const& url); + + }; +}}} // namespace Azure::Core::_internal diff --git a/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp b/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp index 562f55b12a..78a3160e50 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp @@ -139,8 +139,11 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { m_span->AddEvent(exception); } } - virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& ) override { - throw std::runtime_error("Not implemented"); + virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) override { + if (m_span) + { + m_span->PropagateToHttpHeaders(request); + } } }; diff --git a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp index 69d3d4001f..502a4d68f2 100644 --- a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp +++ b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp @@ -198,7 +198,7 @@ namespace Azure { namespace Core { * @param attributeValue value of the attribute. */ virtual void AddAttribute( - std::string const& atributeName, + std::string const& attributeName, std::string const& attributeValue) = 0; diff --git a/sdk/core/azure-core/src/http/request_activity_policy.cpp b/sdk/core/azure-core/src/http/request_activity_policy.cpp index ada1834f09..f08a9be111 100644 --- a/sdk/core/azure-core/src/http/request_activity_policy.cpp +++ b/sdk/core/azure-core/src/http/request_activity_policy.cpp @@ -3,6 +3,7 @@ #include "azure/core/http/policies/policy.hpp" #include "azure/core/internal/diagnostics/log.hpp" +#include "azure/core/internal/request_sanitizer.hpp" #include "azure/core/internal/tracing/service_tracing.hpp" #include @@ -24,7 +25,15 @@ std::unique_ptr RequestActivityPolicy::Send( { // Create a tracing span over the HTTP request. std::stringstream ss; - ss << "HTTP " << request.GetMethod().ToString() << " #" << m_retryCount; + // We know that the retry policy MUST be above us in the hierarchy, so ask it for the current + // retry count. + auto retryCount = RetryPolicy::GetRetryCount(context); + if (retryCount == -1) + { + // We don't have a RetryPolicy in the policy stack - just assume this is request 0. + retryCount = 0; + } + ss << "HTTP " << request.GetMethod().ToString() << " #" << retryCount; auto contextAndSpan = Azure::Core::Tracing::_internal::DiagnosticTracingFactory::CreateSpanFromContext( ss.str(), SpanKind::Client, context); @@ -33,8 +42,7 @@ std::unique_ptr RequestActivityPolicy::Send( scope.AddAttribute(TracingAttributes::HttpMethod.ToString(), request.GetMethod().ToString()); scope.AddAttribute( "http.url", - /* _sanitizer.SanitizeUrl(message.Request.Uri.ToString())*/ - request.GetUrl().GetAbsoluteUrl()); + Azure::Core::_internal::InputSanitizer::SanitizeUrl(request.GetUrl()).GetAbsoluteUrl()); { Azure::Nullable requestId = request.GetHeader("x-ms-client-request-id"); if (requestId.HasValue()) @@ -50,6 +58,12 @@ std::unique_ptr RequestActivityPolicy::Send( scope.AddAttribute(TracingAttributes::HttpUserAgent.ToString(), userAgent.Value()); } } + + // Propogate information from the scope to the HTTP headers. + // + // This will add the "traceparent" header and any other OpenTelemetry related headers. + scope.PropagateToHttpHeaders(request); + try { auto response = nextPolicy.Send(request, contextAndSpan.first); @@ -58,14 +72,12 @@ std::unique_ptr RequestActivityPolicy::Send( TracingAttributes::HttpStatusCode.ToString(), std::to_string(static_cast(response->GetStatusCode()))); auto& responseHeaders = response->GetHeaders(); - auto serviceRequestId = responseHeaders.find("serviceRequestId"); + auto serviceRequestId = responseHeaders.find("x-ms-request-id"); if (serviceRequestId != responseHeaders.end()) { scope.AddAttribute(TracingAttributes::ServiceRequestId.ToString(), serviceRequestId->second); } - m_retryCount += 1; - return response; } catch (const TransportException& e) diff --git a/sdk/core/azure-core/src/private/request_sanitizer.cpp b/sdk/core/azure-core/src/private/request_sanitizer.cpp new file mode 100644 index 0000000000..7f5d1fce54 --- /dev/null +++ b/sdk/core/azure-core/src/private/request_sanitizer.cpp @@ -0,0 +1,36 @@ + +#include "azure/core/internal/request_sanitizer.hpp" +#include "azure/core/url.hpp" +#include + +namespace Azure { namespace Core { namespace _internal { + + Azure::Core::Url InputSanitizer::SanitizeUrl(Azure::Core::Url const& url) + { + Azure::Core::Url redactedUrl; + redactedUrl.SetScheme(url.GetScheme()); + auto host = url.GetHost(); + auto hostWithNoAccount = std::find(host.begin(), host.end(), '.'); + redactedUrl.SetHost("REDACTED" + std::string(hostWithNoAccount, host.end())); + // replace any uniqueID from the path for a hardcoded id + // For the regex, we should not assume anything about the version of UUID format being used. So, + // using the most general regex to get any uuid version. + redactedUrl.SetPath(std::regex_replace( + url.GetPath(), + std::regex("[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}"), + "33333333-3333-3333-3333-333333333333")); + // Query parameters + for (auto const& qp : url.GetQueryParameters()) + { + if (qp.first == "sig") + { + redactedUrl.AppendQueryParameter("sig", "REDACTED"); + } + else + { + redactedUrl.AppendQueryParameter(qp.first, qp.second); + } + } + return redactedUrl; + } +}}} // namespace Azure::Core::_internal \ No newline at end of file diff --git a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp index d1f118d723..de9204b912 100644 --- a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp @@ -16,17 +16,28 @@ using namespace Azure::Core::Tracing; namespace { class NoOpPolicy final : public HttpPolicy { + std::function(Request&)> m_createResponse{}; + public: std::unique_ptr Clone() const override { return std::make_unique(*this); } - std::unique_ptr Send(Request&, NextHttpPolicy, Azure::Core::Context const&) + std::unique_ptr Send(Request& request, NextHttpPolicy, Azure::Core::Context const&) const override { - return std::make_unique(1, 1, HttpStatusCode::Ok, "Something"); - + if (m_createResponse) + { + return m_createResponse(request); + } + else + { + return std::make_unique(1, 1, HttpStatusCode::Ok, "Something"); + } } -}; + NoOpPolicy() = default; + NoOpPolicy(std::function(Request&)> createResponse) + : HttpPolicy(), m_createResponse(createResponse){}; +}; } // namespace // Dummy service tracing class. @@ -172,6 +183,7 @@ TEST(RequestActivityPolicy, Basic) policies.emplace_back(std::make_unique()); policies.emplace_back( std::make_unique("my-service-cpp", "1.0b2", clientOptions.Telemetry)); + policies.emplace_back(std::make_unique(RetryOptions{})); policies.emplace_back(std::make_unique()); // Final policy - equivalent to HTTP policy. policies.emplace_back(std::make_unique()); @@ -185,7 +197,6 @@ TEST(RequestActivityPolicy, Basic) EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); EXPECT_EQ("GET", tracer->GetSpans()[1]->GetAttributes().at("http.method")); - } } @@ -206,16 +217,32 @@ TEST(RequestActivityPolicy, TryRetries) { std::vector> policies; + + policies.emplace_back(std::make_unique()); + policies.emplace_back(std::make_unique(RetryOptions{})); + // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. policies.emplace_back(std::make_unique()); // Final policy - equivalent to HTTP policy. - policies.emplace_back(std::make_unique()); + int retryCount = 0; + policies.emplace_back(std::make_unique([&](Request&) { + retryCount += 1; + if (retryCount < 3) + { + // Return a response which should trigger a response. + return std::make_unique( + 1, 1, *RetryOptions().StatusCodes.begin(), "Something"); + } + else + { + // Return success. + return std::make_unique(1, 1, HttpStatusCode::Ok, "Something"); + } + })); Azure::Core::Http::_internal::HttpPipeline pipeline(policies); // Simulate retrying an HTTP operation 3 times on the pipeline: pipeline.Send(request, callContext); - pipeline.Send(request, callContext); - pipeline.Send(request, callContext); } EXPECT_EQ(1ul, testTracer->GetTracers().size()); @@ -226,6 +253,8 @@ TEST(RequestActivityPolicy, TryRetries) EXPECT_EQ("HTTP GET #1", tracer->GetSpans()[2]->GetName()); EXPECT_EQ("HTTP GET #2", tracer->GetSpans()[3]->GetName()); EXPECT_EQ("GET", tracer->GetSpans()[1]->GetAttributes().at("http.method")); - EXPECT_EQ("200", tracer->GetSpans()[1]->GetAttributes().at("http.status_code")); + EXPECT_EQ("408", tracer->GetSpans()[1]->GetAttributes().at("http.status_code")); + EXPECT_EQ("408", tracer->GetSpans()[2]->GetAttributes().at("http.status_code")); + EXPECT_EQ("200", tracer->GetSpans()[3]->GetAttributes().at("http.status_code")); } } From 863227aab1e9bedaee6016074dd9f887d24e96db Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 25 May 2022 14:32:06 -0700 Subject: [PATCH 03/28] clang-format and added doxygen comments --- .../azure-core/inc/azure/core/http/policies/policy.hpp | 1 - .../inc/azure/core/internal/request_sanitizer.hpp | 3 +-- .../inc/azure/core/internal/tracing/service_tracing.hpp | 3 ++- sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp | 7 +++++++ 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index db318ac9e1..f82410d396 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -393,7 +393,6 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { */ class RequestActivityPolicy final : public HttpPolicy { private: - public: /** * @brief Constructs HTTP Request Activity policy. diff --git a/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp b/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp index 47e480960a..ed817997d8 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp @@ -3,14 +3,13 @@ #pragma once -#include #include "azure/core/url.hpp" +#include namespace Azure { namespace Core { namespace _internal { class InputSanitizer final { public: static Url SanitizeUrl(Url const& url); - }; }}} // namespace Azure::Core::_internal diff --git a/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp b/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp index 78a3160e50..3469ca5664 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp @@ -139,7 +139,8 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { m_span->AddEvent(exception); } } - virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) override { + virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) override + { if (m_span) { m_span->PropagateToHttpHeaders(request); diff --git a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp index 502a4d68f2..3c29281584 100644 --- a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp +++ b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp @@ -236,6 +236,13 @@ namespace Azure { namespace Core { * @param description A description associated with the Status. */ virtual void SetStatus(SpanStatus const& status, std::string const& description = "") = 0; + + /** + * @brief Propogate information from the current span to the HTTP request headers. + * + * @param request HTTP Request to the service. If there is an active tracing span, this will + * add required headers to the HTTP Request. + */ virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) = 0; }; From 77a1ad67e771d6639828a8f6d3fe141bf5f86022 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 25 May 2022 14:55:32 -0700 Subject: [PATCH 04/28] Noise reduction in diff --- .../tracing/opentelemetry/opentelemetry.hpp | 6 + .../azure-core/inc/azure/core/context.hpp | 9 +- .../core/internal/tracing/service_tracing.hpp | 7 + .../inc/azure/core/tracing/tracing.hpp | 506 +++++++++--------- 4 files changed, 268 insertions(+), 260 deletions(-) diff --git a/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp b/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp index 0a4ba2c726..ef97d9fcf7 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp +++ b/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp @@ -131,6 +131,12 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { Azure::Core::Tracing::_internal::SpanStatus const& status, std::string const& statusMessage) override; + /** + * @brief Propogate information from the current span to the HTTP request headers. + * + * @param request HTTP Request to the service. If there is an active tracing span, this will + * add required headers to the HTTP Request. + */ virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) override; opentelemetry::trace::SpanContext GetContext() { return m_span->GetContext(); } diff --git a/sdk/core/azure-core/inc/azure/core/context.hpp b/sdk/core/azure-core/inc/azure/core/context.hpp index 166f89f062..5a610bb3d9 100644 --- a/sdk/core/azure-core/inc/azure/core/context.hpp +++ b/sdk/core/azure-core/inc/azure/core/context.hpp @@ -19,11 +19,12 @@ #include #include +// Forward declare TracerProvider to resolve an include file dependency ordering problem. +namespace Azure { namespace Core { namespace Tracing { + class TracerProvider; +}}} // namespace Azure::Core::Tracing + namespace Azure { namespace Core { - // Forward declare TracerProvider to resolve an include file dependency ordering problem. - namespace Tracing { - class TracerProvider; - } /** * @brief An exception thrown when an operation is cancelled. diff --git a/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp b/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp index 3469ca5664..9f9217c7d7 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp @@ -139,6 +139,13 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal { m_span->AddEvent(exception); } } + + /** + * @brief Propogate information from the current span to the HTTP request headers. + * + * @param request HTTP Request to the service. If there is an active tracing span, this will + * add required headers to the HTTP Request. + */ virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) override { if (m_span) diff --git a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp index 3c29281584..763a247bb9 100644 --- a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp +++ b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp @@ -17,301 +17,295 @@ #include #include -//#include "azure/core/http/http.hpp" +// Forward declare Azure::Core::Http::Request to resolve an include file ordering problem. +namespace Azure { namespace Core { namespace Http { + class Request; +}}} // namespace Azure::Core::Http -namespace Azure { namespace Core { - // Forward declare Azure::Core::Http::Request to resolve an include file ordering problem. - namespace Http { - class Request; - } - namespace Tracing { +namespace Azure { namespace Core { namespace Tracing { - namespace _internal { + namespace _internal { - /** The set of attributes to be applied to a Span or Event. + /** The set of attributes to be applied to a Span or Event. + * + * @details + * An AttributeSet represents a set of attributes to be added to a span or + * event. + * + * @note Note that AttributeSets do *NOT* take a copy of their input values, + * it is the responsibility of the caller to ensure that the object remains + * valid between when it is added to the AttributeSet and when it is added to + * a span or event. + * + * OpenTelemetry property bags can hold: + * - bool + * - int32_t + * - int64_t + * - uint64_t + * - double + * - const char * + * - std::string/std::string_view *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - std::span *** Not Implemented + * - uint64_t (not fully supported) *** Not Implemented + * - std::span (not fully supported) *** Not Implemented + * - std::span (not fully supported) *** Not Implemented. + * + */ + class AttributeSet { + public: + /** + * @brief Adds a Boolean attribute to the attribute set. * - * @details - * An AttributeSet represents a set of attributes to be added to a span or - * event. + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, bool value) = 0; + /** + * @brief Adds a 32bit integer attribute to the attribute set. * - * @note Note that AttributeSets do *NOT* take a copy of their input values, - * it is the responsibility of the caller to ensure that the object remains - * valid between when it is added to the AttributeSet and when it is added to - * a span or event. + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, int32_t value) = 0; + /** + * @brief Adds a 64bit integer attribute to the attribute set. * - * OpenTelemetry property bags can hold: - * - bool - * - int32_t - * - int64_t - * - uint64_t - * - double - * - const char * - * - std::string/std::string_view *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - std::span *** Not Implemented - * - uint64_t (not fully supported) *** Not Implemented - * - std::span (not fully supported) *** Not Implemented - * - std::span (not fully supported) *** Not Implemented. + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, int64_t value) = 0; + /** + * @brief Adds an unsigned 64bit integer attribute to the attribute set. * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. */ - class AttributeSet { - public: - /** - * @brief Adds a Boolean attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, bool value) = 0; - /** - * @brief Adds a 32bit integer attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, int32_t value) = 0; - /** - * @brief Adds a 64bit integer attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, int64_t value) = 0; - /** - * @brief Adds an unsigned 64bit integer attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, uint64_t value) = 0; - /** - * @brief Adds a 64bit floating point attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, double value) = 0; - /** - * @brief Adds a C style string attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, const char* value) = 0; - /** - * @brief Adds a C++ string attribute to the attribute set. - * - * @param attributeName Name of attribute to add. - * @param value Value of attribute. - */ - virtual void AddAttribute(std::string const& attributeName, std::string const& value) = 0; - - /** - * @brief destroys an AttributeSet - virtual destructor to enable base class users to - * destroy derived classes. - */ - virtual ~AttributeSet(){}; - }; + virtual void AddAttribute(std::string const& attributeName, uint64_t value) = 0; + /** + * @brief Adds a 64bit floating point attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, double value) = 0; + /** + * @brief Adds a C style string attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, const char* value) = 0; + /** + * @brief Adds a C++ string attribute to the attribute set. + * + * @param attributeName Name of attribute to add. + * @param value Value of attribute. + */ + virtual void AddAttribute(std::string const& attributeName, std::string const& value) = 0; - /** @brief The Type of Span. + /** + * @brief destroys an AttributeSet - virtual destructor to enable base class users to + * destroy derived classes. */ - class SpanKind final : public Azure::Core::_internal::ExtendableEnumeration { - public: - explicit SpanKind(std::string const& kind) : ExtendableEnumeration(kind) {} - SpanKind() = default; + virtual ~AttributeSet(){}; + }; - /** - * @brief Represents an "Internal" operation. - * - */ - AZ_CORE_DLLEXPORT const static SpanKind Internal; - /** - * @brief Represents a request to a remote service. - * - */ - AZ_CORE_DLLEXPORT const static SpanKind Client; - /** - * @brief Represents a span covering the server side handling of an API call. - * - */ - AZ_CORE_DLLEXPORT const static SpanKind Server; - /** - * @brief Represents the initiator of an asynchronous request. - * - */ - AZ_CORE_DLLEXPORT const static SpanKind Producer; - /** - * @brief Represents a span which describes a child of a producer request. - * - */ - AZ_CORE_DLLEXPORT const static SpanKind Consumer; - }; + /** @brief The Type of Span. + */ + class SpanKind final : public Azure::Core::_internal::ExtendableEnumeration { + public: + explicit SpanKind(std::string const& kind) : ExtendableEnumeration(kind) {} + SpanKind() = default; /** - * @brief Represents the status of a span. + * @brief Represents an "Internal" operation. + * + */ + AZ_CORE_DLLEXPORT const static SpanKind Internal; + /** + * @brief Represents a request to a remote service. + * + */ + AZ_CORE_DLLEXPORT const static SpanKind Client; + /** + * @brief Represents a span covering the server side handling of an API call. + * + */ + AZ_CORE_DLLEXPORT const static SpanKind Server; + /** + * @brief Represents the initiator of an asynchronous request. + * */ - class SpanStatus final : public Azure::Core::_internal::ExtendableEnumeration { + AZ_CORE_DLLEXPORT const static SpanKind Producer; + /** + * @brief Represents a span which describes a child of a producer request. + * + */ + AZ_CORE_DLLEXPORT const static SpanKind Consumer; + }; - public: - explicit SpanStatus(std::string const& status) : ExtendableEnumeration(status) {} - SpanStatus() = default; + /** + * @brief Represents the status of a span. + */ + class SpanStatus final : public Azure::Core::_internal::ExtendableEnumeration { - /** - * @brief The default status of a span. - */ - AZ_CORE_DLLEXPORT const static SpanStatus Unset; - /** - * @brief The operation has completed successfully. - */ - AZ_CORE_DLLEXPORT const static SpanStatus Ok; - /** - * @brief The operation contains an error. - */ - AZ_CORE_DLLEXPORT const static SpanStatus Error; - }; + public: + explicit SpanStatus(std::string const& status) : ExtendableEnumeration(status) {} + SpanStatus() = default; /** - * @brief Span - represents a span in tracing. + * @brief The default status of a span. */ - class Span { - public: - /** - * @brief Signals that the span has now ended. - */ - virtual void End(Azure::Nullable endTime = {}) = 0; - - /** - * @brief Adds a set of attributes to the span. - * - * @param attributeToAdd Attributes to be added to the span. - */ - virtual void AddAttributes(AttributeSet const& attributeToAdd) = 0; + AZ_CORE_DLLEXPORT const static SpanStatus Unset; + /** + * @brief The operation has completed successfully. + */ + AZ_CORE_DLLEXPORT const static SpanStatus Ok; + /** + * @brief The operation contains an error. + */ + AZ_CORE_DLLEXPORT const static SpanStatus Error; + }; - /** - * @brief Adds a single string valued attribute to the span. - * - * @param attributeName Name of the attribute to add. - * @param attributeValue value of the attribute. - */ - virtual void AddAttribute( - std::string const& attributeName, - std::string const& attributeValue) - = 0; + /** + * @brief Span - represents a span in tracing. + */ + class Span { + public: + /** + * @brief Signals that the span has now ended. + */ + virtual void End(Azure::Nullable endTime = {}) = 0; - /** - * @brief Adds an event to the span. - * - * Add an Event to the span. An event is identified by a name and an optional set of - * attributes associated with the event. - * - * @param eventName Name of the event to add. - * @param eventAttributes Attributes associated with the event. - */ - virtual void AddEvent(std::string const& eventName, AttributeSet const& eventAttributes) - = 0; + /** + * @brief Adds a set of attributes to the span. + * + * @param attributeToAdd Attributes to be added to the span. + */ + virtual void AddAttributes(AttributeSet const& attributeToAdd) = 0; - /** - * @brief Adds an event to the span. - * - * Add an Event to the span. An event is identified by a name - * - * @param eventName Name of the event to add. - */ - virtual void AddEvent(std::string const& eventName) = 0; - /** - * @brief Records an exception occurring in the span. - * - * @param exception Exception which has occurred. - */ - virtual void AddEvent(std::exception const& exception) = 0; + /** + * @brief Adds a single string valued attribute to the span. + * + * @param attributeName Name of the attribute to add. + * @param attributeValue value of the attribute. + */ + virtual void AddAttribute(std::string const& attributeName, std::string const& attributeValue) + = 0; - /** - * @brief Set the Status of the span - * - * @param status Updated status of the span. - * @param description A description associated with the Status. - */ - virtual void SetStatus(SpanStatus const& status, std::string const& description = "") = 0; + /** + * @brief Adds an event to the span. + * + * Add an Event to the span. An event is identified by a name and an optional set of + * attributes associated with the event. + * + * @param eventName Name of the event to add. + * @param eventAttributes Attributes associated with the event. + */ + virtual void AddEvent(std::string const& eventName, AttributeSet const& eventAttributes) = 0; - /** - * @brief Propogate information from the current span to the HTTP request headers. - * - * @param request HTTP Request to the service. If there is an active tracing span, this will - * add required headers to the HTTP Request. - */ - virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) = 0; - }; + /** + * @brief Adds an event to the span. + * + * Add an Event to the span. An event is identified by a name + * + * @param eventName Name of the event to add. + */ + virtual void AddEvent(std::string const& eventName) = 0; + /** + * @brief Records an exception occurring in the span. + * + * @param exception Exception which has occurred. + */ + virtual void AddEvent(std::exception const& exception) = 0; /** - * @brief Options used while creating a span. + * @brief Set the Status of the span * + * @param status Updated status of the span. + * @param description A description associated with the Status. */ - struct CreateSpanOptions final - { - /** - * @brief The kind of span to be created. - * - */ - SpanKind Kind{SpanKind::Internal}; - /** - * @brief Attributes associated with the span. - * - */ - std::unique_ptr Attributes; + virtual void SetStatus(SpanStatus const& status, std::string const& description = "") = 0; - /** - * @brief Parent for the newly created span. - */ - std::shared_ptr ParentSpan; - }; + /** + * @brief Propogate information from the current span to the HTTP request headers. + * + * @param request HTTP Request to the service. If there is an active tracing span, this will + * add required headers to the HTTP Request. + */ + virtual void PropagateToHttpHeaders(Azure::Core::Http::Request& request) = 0; + }; + /** + * @brief Options used while creating a span. + * + */ + struct CreateSpanOptions final + { /** - * @brief Tracer - factory for creating span objects. + * @brief The kind of span to be created. * */ - class Tracer { - public: - /** - * @brief Create new Span object. - * - * @details Creates a new span object. - * - * @note There is no concept of a "current" span, each span created is a top level span, - * unless the CreateSpanOptions has ParentSpan member, in which case the ParentSpan member - * will be the parent of the newly created span. - * - * @param spanName The name of the span to create. - * @param options Options to be used when creating the span. - * @return std::shared_ptr Newly created span. - */ - virtual std::shared_ptr CreateSpan( - std::string const& spanName, - CreateSpanOptions const& options = {}) const = 0; + SpanKind Kind{SpanKind::Internal}; + /** + * @brief Attributes associated with the span. + * + */ + std::unique_ptr Attributes; - virtual std::unique_ptr CreateAttributeSet() const = 0; - }; - } // namespace _internal + /** + * @brief Parent for the newly created span. + */ + std::shared_ptr ParentSpan; + }; /** - * @brief Trace Provider - factory for creating Tracer objects. + * @brief Tracer - factory for creating span objects. + * */ - class TracerProvider { + class Tracer { public: /** - * @brief Create a Tracer object + * @brief Create new Span object. + * + * @details Creates a new span object. * - * @param name Name of the tracer object, typically the name of the Service client - * (Azure.Storage.Blobs, for example) - * @param version Version of the service client. - * @return std::shared_ptr + * @note There is no concept of a "current" span, each span created is a top level span, + * unless the CreateSpanOptions has ParentSpan member, in which case the ParentSpan member + * will be the parent of the newly created span. + * + * @param spanName The name of the span to create. + * @param options Options to be used when creating the span. + * @return std::shared_ptr Newly created span. */ - virtual std::shared_ptr CreateTracer( - std::string const& name, - std::string const& version = "") const = 0; + virtual std::shared_ptr CreateSpan( + std::string const& spanName, + CreateSpanOptions const& options = {}) const = 0; + + virtual std::unique_ptr CreateAttributeSet() const = 0; }; - } // namespace Tracing -}} // namespace Azure::Core + } // namespace _internal + + /** + * @brief Trace Provider - factory for creating Tracer objects. + */ + class TracerProvider { + public: + /** + * @brief Create a Tracer object + * + * @param name Name of the tracer object, typically the name of the Service client + * (Azure.Storage.Blobs, for example) + * @param version Version of the service client. + * @return std::shared_ptr + */ + virtual std::shared_ptr CreateTracer( + std::string const& name, + std::string const& version = "") const = 0; + }; +}}} // namespace Azure::Core::Tracing From 00216855494660b5302dc2cd91d468c0112ceec2 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 25 May 2022 15:02:33 -0700 Subject: [PATCH 05/28] cspell --- .../src/opentelemetry.cpp | 8 ++++---- sdk/core/azure-core/src/http/request_activity_policy.cpp | 2 +- sdk/core/azure-core/src/private/request_sanitizer.cpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp index cd59cae510..21704ea06e 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp @@ -197,7 +197,7 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { m_span->SetAttribute(attributeName, opentelemetry::common::AttributeValue(attributeValue)); } - class HttpRequestTextMapPropogater + class HttpRequestTextMapPropagator : public opentelemetry::context::propagation::TextMapCarrier { Azure::Core::Http::Request& m_request; // Inherited via TextMapCarrier @@ -220,21 +220,21 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { } public: - HttpRequestTextMapPropogater(Azure::Core::Http::Request& request) : m_request(request) {} + HttpRequestTextMapPropagator(Azure::Core::Http::Request& request) : m_request(request) {} }; void OpenTelemetrySpan::PropagateToHttpHeaders(Azure::Core::Http::Request& request) { if (m_span) { - HttpRequestTextMapPropogater propogater(request); + HttpRequestTextMapPropagator propagator(request); // Establish the current runtime context from the span. auto scope = opentelemetry::trace::Tracer::WithActiveSpan(m_span); auto currentContext = opentelemetry::context::RuntimeContext::GetCurrent(); opentelemetry::trace::propagation::HttpTraceContext httpTraceContext; - httpTraceContext.Inject(propogater, currentContext); + httpTraceContext.Inject(propagator, currentContext); } } diff --git a/sdk/core/azure-core/src/http/request_activity_policy.cpp b/sdk/core/azure-core/src/http/request_activity_policy.cpp index f08a9be111..314db61fdc 100644 --- a/sdk/core/azure-core/src/http/request_activity_policy.cpp +++ b/sdk/core/azure-core/src/http/request_activity_policy.cpp @@ -87,4 +87,4 @@ std::unique_ptr RequestActivityPolicy::Send( // Rethrow the exception. throw; } -} \ No newline at end of file +} diff --git a/sdk/core/azure-core/src/private/request_sanitizer.cpp b/sdk/core/azure-core/src/private/request_sanitizer.cpp index 7f5d1fce54..1d0ca99931 100644 --- a/sdk/core/azure-core/src/private/request_sanitizer.cpp +++ b/sdk/core/azure-core/src/private/request_sanitizer.cpp @@ -33,4 +33,4 @@ namespace Azure { namespace Core { namespace _internal { } return redactedUrl; } -}}} // namespace Azure::Core::_internal \ No newline at end of file +}}} // namespace Azure::Core::_internal From 52c4de197f11470770b9c0e8052e27d05018bad8 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 25 May 2022 15:16:31 -0700 Subject: [PATCH 06/28] #include list --- sdk/core/azure-core/test/ut/request_activity_policy_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp index de9204b912..4196d23271 100644 --- a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp @@ -6,6 +6,7 @@ #include "azure/core/internal/tracing/service_tracing.hpp" #include "azure/core/tracing/tracing.hpp" #include +#include using namespace Azure::Core; using namespace Azure::Core::Http; From a26e87f845ff969b1d5e4394935bb0dacdfafbfb Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 25 May 2022 15:33:52 -0700 Subject: [PATCH 07/28] Signed/unsigned issue --- .../test/ut/service_support_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp index bf68cacde1..583077bdee 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp @@ -481,7 +481,7 @@ TEST_F(OpenTelemetryServiceTests, NestSpans) index += 1; } } - EXPECT_EQ(index, 3); + EXPECT_EQ(3ul, index); returnedComponents[3] = component; return returnedComponents; }; From b55e04e50bc09a8c0537db9d7099ab19395b3ef0 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 25 May 2022 16:36:51 -0700 Subject: [PATCH 08/28] Added printfs to try to track down ci failure --- sdk/core/azure-core/test/ut/service_tracing_test.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sdk/core/azure-core/test/ut/service_tracing_test.cpp b/sdk/core/azure-core/test/ut/service_tracing_test.cpp index 70e06e20dc..43f86648be 100644 --- a/sdk/core/azure-core/test/ut/service_tracing_test.cpp +++ b/sdk/core/azure-core/test/ut/service_tracing_test.cpp @@ -115,6 +115,8 @@ class TestTracingProvider final : public Azure::Core::Tracing::TracerProvider { TEST(DiagnosticTracingFactory, BasicServiceSpanTests) { + GTEST_LOG_(INFO) << "BasicServiceSpanTests. Create A dummy (nop) span and add a couple of events." + << std::endl; { Azure::Core::_internal::ClientOptions clientOptions; Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( @@ -129,17 +131,24 @@ TEST(DiagnosticTracingFactory, BasicServiceSpanTests) span.AddEvent(std::runtime_error("Exception")); span.SetStatus(SpanStatus::Error); } + GTEST_LOG_(INFO) + << "BasicServiceSpanTests. Create Span with test tracing provider and add a couple of events." + << std::endl; { Azure::Core::_internal::ClientOptions clientOptions; auto testTracer = std::make_shared(); clientOptions.Telemetry.TracingProvider = testTracer; + GTEST_LOG_(INFO) << "BasicServiceSpanTests. Create ServiceTrace " << std::endl; Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( clientOptions, "my-service-cpp", "1.0b2"); + GTEST_LOG_(INFO) << "BasicServiceSpanTests. Create Span " << std::endl; auto contextAndSpan = serviceTrace.CreateSpan( "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); ServiceSpan span = std::move(contextAndSpan.second); + GTEST_LOG_(INFO) << "BasicServiceSpanTests. End Span" << std::endl; + span.End(); span.AddEvent("New Event"); span.AddEvent(std::runtime_error("Exception")); @@ -149,5 +158,6 @@ TEST(DiagnosticTracingFactory, BasicServiceSpanTests) span.AddEvent("AttributeEvent", *attributeSet); span.AddAttributes(*attributeSet); span.SetStatus(SpanStatus::Error); + GTEST_LOG_(INFO) << "BasicServiceSpanTests. Done." << std::endl; } } From 228ac73efcb597ae607b28f956e02846643460ab Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Wed, 25 May 2022 17:47:35 -0700 Subject: [PATCH 09/28] More traces --- .../azure-core/test/ut/request_activity_policy_test.cpp | 6 +++--- sdk/core/azure-core/test/ut/service_tracing_test.cpp | 8 +++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp index 4196d23271..00aad4c2ad 100644 --- a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp @@ -158,7 +158,7 @@ TEST(RequestActivityPolicy, Basic) } EXPECT_EQ(1ul, testTracer->GetTracers().size()); - auto tracer = testTracer->GetTracers().front(); + auto &tracer = testTracer->GetTracers().front(); EXPECT_EQ(2ul, tracer->GetSpans().size()); EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); @@ -193,7 +193,7 @@ TEST(RequestActivityPolicy, Basic) } EXPECT_EQ(1ul, testTracer->GetTracers().size()); - auto tracer = testTracer->GetTracers().front(); + auto &tracer = testTracer->GetTracers().front(); EXPECT_EQ(2ul, tracer->GetSpans().size()); EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); @@ -247,7 +247,7 @@ TEST(RequestActivityPolicy, TryRetries) } EXPECT_EQ(1ul, testTracer->GetTracers().size()); - auto tracer = testTracer->GetTracers().front(); + auto &tracer = testTracer->GetTracers().front(); EXPECT_EQ(4ul, tracer->GetSpans().size()); EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); diff --git a/sdk/core/azure-core/test/ut/service_tracing_test.cpp b/sdk/core/azure-core/test/ut/service_tracing_test.cpp index 43f86648be..eed0368c72 100644 --- a/sdk/core/azure-core/test/ut/service_tracing_test.cpp +++ b/sdk/core/azure-core/test/ut/service_tracing_test.cpp @@ -89,7 +89,10 @@ class TestAttributeSet : public Azure::Core::Tracing::_internal::AttributeSet { }; class TestTracer final : public Azure::Core::Tracing::_internal::Tracer { public: - TestTracer(std::string const&, std::string const&) : Azure::Core::Tracing::_internal::Tracer() {} + TestTracer(std::string const&, std::string const&) : Azure::Core::Tracing::_internal::Tracer() + { + GTEST_LOG_(INFO) << "TestTracer::TestTracer" << std::endl; + } std::shared_ptr CreateSpan(std::string const&, CreateSpanOptions const&) const override { return std::make_shared(); @@ -109,6 +112,9 @@ class TestTracingProvider final : public Azure::Core::Tracing::TracerProvider { std::string const& serviceName, std::string const& serviceVersion) const override { + GTEST_LOG_(INFO) << "TestTracerProvider::CreateTracer " << serviceName << serviceVersion + << std::endl; + return std::make_shared(serviceName, serviceVersion); }; }; From 4661f4181ecf966ccc7a2eb4c7e066100b39cc4b Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 26 May 2022 09:52:07 -0700 Subject: [PATCH 10/28] Fixed tracing tests in Release builds (ODR violation) --- doc/DistributedTracing.md | 43 +++++++++++++++++++ .../test/ut/request_activity_policy_test.cpp | 8 ++-- .../test/ut/service_tracing_test.cpp | 24 +++-------- 3 files changed, 52 insertions(+), 23 deletions(-) create mode 100644 doc/DistributedTracing.md diff --git a/doc/DistributedTracing.md b/doc/DistributedTracing.md new file mode 100644 index 0000000000..a0c704cc8f --- /dev/null +++ b/doc/DistributedTracing.md @@ -0,0 +1,43 @@ +# Distributed Tracing in the C++ SDK +Azure has adopted [W3C Distributed Tracing](https://www.w3.org/TR/trace-context/) as a paradigm for correlating +requests from clients across multiple services. + +This document explains how the Azure C++ SDK implements distributed tracing, how clients integrate with distributed tracing, how +services should integrate with distributed tracing and finally how the network pipeline and other functionality should +integrate with distributed tracing. + +## Tracing Overview +The Azure SDK for C++ Tracing APIs are modeled after the opentelemetry-cpp API surface defined in the [OpenTelemetry Tracing Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md). +Additional architectural information about OpenTelemetry can be found in [OpenTelemetry Concepts](https://opentelemetry.io/docs/concepts/). + +There are three major components which the Azure SDK components interact with: +- `TracerProvider` - this is a factory which creates `Tracer` objects. +- `Tracer` - this is a factory which creates `Span` objects. +- `Span` - Span objects are the APIs which allow tracing an operation. +Each `span` has a name, a type and a "status". `Spans` also contain "attributes" and "events" which describe an operation. + +There is typically a single `TracerProvider` for each application, and for the Azure SDK, each +service will have a `Tracer` implementation which creates `Span` objects for each service client. + +A `Span` can be considered a "unit of work" for a service. Each service method (method which calls into the service) will have a single `Span` reflecting the client method which +was called. + +`Span`'s are hierarchical and each span can have multiple children (each `Span` can only have a single parent). The typical way that this manifests itself during a +service method call is: +* Service Method "MyServiceMethod" creates a span named "MyServiceMethod" and starts an HTTP request to communicate with the service. + * The HTTP pipeline (specifically the `RequestActivityPolicy`) will create a child `span` under the service method `span` named `"HTTP #0"`. This span + reflects the HTTP call into the service. + * If the HTTP call needs to be retried, the existing `span` will be closed an a new span named `HTTP #1` will be created for the retry. + + + +## Distributed Tracing Client Integration. +Applications which wish to integrate Distributed Tracing are strongly encouraged +to use the [opentelemetry-cpp](https://github.com/open-telemetry/opentelemetry-cpp) vcpkg package. + +There are numerous examples on the OpenTelemetry web site which demonstrate how to integrate +opentelemetry into a customer application and integrate the generated traces +with Azure monitoring infrastructure such as Geneva Monitoring. + +# Distributed Tracing components. +In order to maintain flexibility, the actual distributed tracing mechanism is \ No newline at end of file diff --git a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp index 00aad4c2ad..517d1edf18 100644 --- a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp @@ -39,7 +39,6 @@ class NoOpPolicy final : public HttpPolicy { NoOpPolicy(std::function(Request&)> createResponse) : HttpPolicy(), m_createResponse(createResponse){}; }; -} // namespace // Dummy service tracing class. class TestSpan final : public Azure::Core::Tracing::_internal::Span { @@ -131,6 +130,7 @@ class TestTracingProvider final : public Azure::Core::Tracing::TracerProvider { std::list> const& GetTracers() { return m_tracers; } }; +} // namespace TEST(RequestActivityPolicy, Basic) { @@ -158,7 +158,7 @@ TEST(RequestActivityPolicy, Basic) } EXPECT_EQ(1ul, testTracer->GetTracers().size()); - auto &tracer = testTracer->GetTracers().front(); + auto& tracer = testTracer->GetTracers().front(); EXPECT_EQ(2ul, tracer->GetSpans().size()); EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); @@ -193,7 +193,7 @@ TEST(RequestActivityPolicy, Basic) } EXPECT_EQ(1ul, testTracer->GetTracers().size()); - auto &tracer = testTracer->GetTracers().front(); + auto& tracer = testTracer->GetTracers().front(); EXPECT_EQ(2ul, tracer->GetSpans().size()); EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); @@ -247,7 +247,7 @@ TEST(RequestActivityPolicy, TryRetries) } EXPECT_EQ(1ul, testTracer->GetTracers().size()); - auto &tracer = testTracer->GetTracers().front(); + auto& tracer = testTracer->GetTracers().front(); EXPECT_EQ(4ul, tracer->GetSpans().size()); EXPECT_EQ("My API", tracer->GetSpans()[0]->GetName()); EXPECT_EQ("HTTP GET #0", tracer->GetSpans()[1]->GetName()); diff --git a/sdk/core/azure-core/test/ut/service_tracing_test.cpp b/sdk/core/azure-core/test/ut/service_tracing_test.cpp index eed0368c72..d933cebf6c 100644 --- a/sdk/core/azure-core/test/ut/service_tracing_test.cpp +++ b/sdk/core/azure-core/test/ut/service_tracing_test.cpp @@ -53,7 +53,7 @@ TEST(DiagnosticTracingFactory, SimpleServiceSpanTests) EXPECT_FALSE(contextAndSpan.first.IsCancelled()); } } - +namespace { // Dummy service tracing class. class TestSpan final : public Azure::Core::Tracing::_internal::Span { public: @@ -89,10 +89,7 @@ class TestAttributeSet : public Azure::Core::Tracing::_internal::AttributeSet { }; class TestTracer final : public Azure::Core::Tracing::_internal::Tracer { public: - TestTracer(std::string const&, std::string const&) : Azure::Core::Tracing::_internal::Tracer() - { - GTEST_LOG_(INFO) << "TestTracer::TestTracer" << std::endl; - } + TestTracer(std::string const&, std::string const&) : Azure::Core::Tracing::_internal::Tracer() {} std::shared_ptr CreateSpan(std::string const&, CreateSpanOptions const&) const override { return std::make_shared(); @@ -112,17 +109,12 @@ class TestTracingProvider final : public Azure::Core::Tracing::TracerProvider { std::string const& serviceName, std::string const& serviceVersion) const override { - GTEST_LOG_(INFO) << "TestTracerProvider::CreateTracer " << serviceName << serviceVersion - << std::endl; - return std::make_shared(serviceName, serviceVersion); }; }; - +} // namespace TEST(DiagnosticTracingFactory, BasicServiceSpanTests) { - GTEST_LOG_(INFO) << "BasicServiceSpanTests. Create A dummy (nop) span and add a couple of events." - << std::endl; { Azure::Core::_internal::ClientOptions clientOptions; Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( @@ -137,24 +129,19 @@ TEST(DiagnosticTracingFactory, BasicServiceSpanTests) span.AddEvent(std::runtime_error("Exception")); span.SetStatus(SpanStatus::Error); } - GTEST_LOG_(INFO) - << "BasicServiceSpanTests. Create Span with test tracing provider and add a couple of events." - << std::endl; + { Azure::Core::_internal::ClientOptions clientOptions; auto testTracer = std::make_shared(); clientOptions.Telemetry.TracingProvider = testTracer; - GTEST_LOG_(INFO) << "BasicServiceSpanTests. Create ServiceTrace " << std::endl; Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace( clientOptions, "my-service-cpp", "1.0b2"); - GTEST_LOG_(INFO) << "BasicServiceSpanTests. Create Span " << std::endl; auto contextAndSpan = serviceTrace.CreateSpan( "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); ServiceSpan span = std::move(contextAndSpan.second); - GTEST_LOG_(INFO) << "BasicServiceSpanTests. End Span" << std::endl; - + span.End(); span.AddEvent("New Event"); span.AddEvent(std::runtime_error("Exception")); @@ -164,6 +151,5 @@ TEST(DiagnosticTracingFactory, BasicServiceSpanTests) span.AddEvent("AttributeEvent", *attributeSet); span.AddAttributes(*attributeSet); span.SetStatus(SpanStatus::Error); - GTEST_LOG_(INFO) << "BasicServiceSpanTests. Done." << std::endl; } } From 30519e8dc262b276bd3b46ed51ec3607eb469055 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Thu, 26 May 2022 11:01:35 -0700 Subject: [PATCH 11/28] clang-format --- sdk/core/azure-core/test/ut/service_tracing_test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/core/azure-core/test/ut/service_tracing_test.cpp b/sdk/core/azure-core/test/ut/service_tracing_test.cpp index d933cebf6c..7675a62363 100644 --- a/sdk/core/azure-core/test/ut/service_tracing_test.cpp +++ b/sdk/core/azure-core/test/ut/service_tracing_test.cpp @@ -141,7 +141,6 @@ TEST(DiagnosticTracingFactory, BasicServiceSpanTests) "My API", Azure::Core::Tracing::_internal::SpanKind::Internal, {}); ServiceSpan span = std::move(contextAndSpan.second); - span.End(); span.AddEvent("New Event"); span.AddEvent(std::runtime_error("Exception")); From e7d4fa5838e97d19aa2ce8bd735aca79ae11dd99 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 09:51:14 -0700 Subject: [PATCH 12/28] Updated input sanitizer --- .../src/interceptor_manager.cpp | 27 +++++- .../tracing/opentelemetry/opentelemetry.hpp | 1 + .../test/ut/azure_core_otel_test.cpp | 1 + .../test/ut/service_support_test.cpp | 4 +- sdk/core/azure-core/CHANGELOG.md | 1 + sdk/core/azure-core/CMakeLists.txt | 4 +- .../inc/azure/core/http/policies/policy.hpp | 20 +++- .../inc/azure/core/internal/http/pipeline.hpp | 7 +- .../azure/core/internal/input_sanitizer.hpp | 50 ++++++++++ .../azure/core/internal/request_sanitizer.hpp | 15 --- .../inc/azure/core/tracing/tracing.hpp | 2 +- sdk/core/azure-core/src/http/log_policy.cpp | 95 ++++--------------- .../src/http/request_activity_policy.cpp | 4 +- .../src/private/input_sanitizer.cpp | 87 +++++++++++++++++ .../src/private/request_sanitizer.cpp | 36 ------- .../test/ut/request_activity_policy_test.cpp | 9 +- 16 files changed, 222 insertions(+), 141 deletions(-) create mode 100644 sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp delete mode 100644 sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp create mode 100644 sdk/core/azure-core/src/private/input_sanitizer.cpp delete mode 100644 sdk/core/azure-core/src/private/request_sanitizer.cpp diff --git a/sdk/core/azure-core-test/src/interceptor_manager.cpp b/sdk/core/azure-core-test/src/interceptor_manager.cpp index 98d81f54b0..60065922d2 100644 --- a/sdk/core/azure-core-test/src/interceptor_manager.cpp +++ b/sdk/core/azure-core-test/src/interceptor_manager.cpp @@ -3,7 +3,6 @@ #include #include -#include #include #include "azure/core/test/interceptor_manager.hpp" @@ -82,5 +81,29 @@ void InterceptorManager::LoadTestData() Url InterceptorManager::RedactUrl(Url const& url) { - return Azure::Core::_internal::InputSanitizer::SanitizeUrl(url); + Azure::Core::Url redactedUrl; + redactedUrl.SetScheme(url.GetScheme()); + auto host = url.GetHost(); + auto hostWithNoAccount = std::find(host.begin(), host.end(), '.'); + redactedUrl.SetHost("REDACTED" + std::string(hostWithNoAccount, host.end())); + // replace any uniqueID from the path for a hardcoded id + // For the regex, we should not assume anything about the version of UUID format being used. So, + // using the most general regex to get any uuid version. + redactedUrl.SetPath(std::regex_replace( + url.GetPath(), + std::regex("[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}"), + "33333333-3333-3333-3333-333333333333")); + // Query parameters + for (auto const& qp : url.GetQueryParameters()) + { + if (qp.first == "sig") + { + redactedUrl.AppendQueryParameter("sig", "REDACTED"); + } + else + { + redactedUrl.AppendQueryParameter(qp.first, qp.second); + } + } + return redactedUrl; } diff --git a/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp b/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp index ef97d9fcf7..7ac27cdbe0 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp +++ b/sdk/core/azure-core-tracing-opentelemetry/inc/azure/core/tracing/opentelemetry/opentelemetry.hpp @@ -11,6 +11,7 @@ #pragma warning(push) #pragma warning(disable : 4100) #pragma warning(disable : 4244) +#pragma warning(disable : 6323) // Disable "Use of arithmetic operator on Boolean type" warning. #endif #include #include diff --git a/sdk/core/azure-core-tracing-opentelemetry/test/ut/azure_core_otel_test.cpp b/sdk/core/azure-core-tracing-opentelemetry/test/ut/azure_core_otel_test.cpp index 4d1735581b..d51ad72513 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/test/ut/azure_core_otel_test.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/test/ut/azure_core_otel_test.cpp @@ -11,6 +11,7 @@ #pragma warning(push) #pragma warning(disable : 4100) #pragma warning(disable : 4244) +#pragma warning(disable : 6323) // Disable "Use of arithmetic operator on Boolean type" warning. #endif #include #include diff --git a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp index 583077bdee..985c2fbcc6 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp @@ -14,6 +14,7 @@ #pragma warning(push) #pragma warning(disable : 4100) #pragma warning(disable : 4244) +#pragma warning(disable : 6323) // Disable "Use of arithmetic operator on Boolean type" warning. #endif #include #include @@ -545,7 +546,8 @@ class ServiceClient { policies.emplace_back(std::make_unique(RetryOptions{})); // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. - policies.emplace_back(std::make_unique()); + policies.emplace_back( + std::make_unique(Azure::Core::_internal::InputSanitizer{})); // Final policy - functions as the HTTP transport policy. policies.emplace_back(std::make_unique([&](Request& request) { diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 52045afd77..d8e42ac48f 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -3,6 +3,7 @@ ## 1.7.0-beta.1 (Unreleased) ### Features Added + - Added prototypes and initial service support for Distributed Tracing. ### Breaking Changes diff --git a/sdk/core/azure-core/CMakeLists.txt b/sdk/core/azure-core/CMakeLists.txt index 95f9a8d81c..90857472f0 100644 --- a/sdk/core/azure-core/CMakeLists.txt +++ b/sdk/core/azure-core/CMakeLists.txt @@ -87,6 +87,7 @@ set( inc/azure/core/internal/json/json_serializable.hpp inc/azure/core/internal/strings.hpp inc/azure/core/internal/tracing/service_tracing.hpp + inc/azure/core/internal/input_sanitizer.hpp inc/azure/core/io/body_stream.hpp inc/azure/core/match_conditions.hpp inc/azure/core/modified_conditions.hpp @@ -133,10 +134,11 @@ set( src/operation_status.cpp src/private/environment_log_level_listener.hpp src/private/package_version.hpp + src/private/input_sanitizer.cpp src/strings.cpp src/tracing/tracing.cpp src/uuid.cpp - "src/private/request_sanitizer.cpp") +) add_library(azure-core ${AZURE_CORE_HEADER} ${AZURE_CORE_SOURCE}) diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index f82410d396..2262537a25 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -14,6 +14,7 @@ #include "azure/core/dll_import_export.hpp" #include "azure/core/http/http.hpp" #include "azure/core/http/transport.hpp" +#include "azure/core/internal/input_sanitizer.hpp" #include "azure/core/tracing/tracing.hpp" #include "azure/core/uuid.hpp" @@ -393,12 +394,22 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { */ class RequestActivityPolicy final : public HttpPolicy { private: + Azure::Core::_internal::InputSanitizer m_inputSanitizer; + public: + /** + * @brief Constructs HTTP Request Activity policy. + */ + // explicit RequestActivityPolicy() = default; /** * @brief Constructs HTTP Request Activity policy. * + * @param inputSanitizer for sanitizing data before it is logged. */ - explicit RequestActivityPolicy() {} + explicit RequestActivityPolicy(Azure::Core::_internal::InputSanitizer const& inputSanitizer) + : m_inputSanitizer(inputSanitizer) + { + } std::unique_ptr Clone() const override { @@ -504,13 +515,18 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { */ class LogPolicy final : public HttpPolicy { LogOptions m_options; + Azure::Core::_internal::InputSanitizer m_inputSanitizer; public: /** * @brief Constructs HTTP logging policy. * */ - explicit LogPolicy(LogOptions options) : m_options(std::move(options)) {} + explicit LogPolicy(LogOptions options) + : m_options(std::move(options)), + m_inputSanitizer(m_options.AllowedHttpQueryParameters, m_options.AllowedHttpHeaders) + { + } std::unique_ptr Clone() const override { 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 73e22ce77c..eec0515fa6 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 @@ -12,6 +12,7 @@ #include "azure/core/context.hpp" #include "azure/core/http/http.hpp" #include "azure/core/http/policies/policy.hpp" +#include "azure/core/internal/input_sanitizer.hpp" #include "azure/core/http/transport.hpp" #include "azure/core/internal/client_options.hpp" @@ -76,6 +77,10 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { std::vector>&& perRetryPolicies, std::vector>&& perCallPolicies) { + Azure::Core::_internal::InputSanitizer inputSanitizer( + clientOptions.Log.AllowedHttpQueryParameters, + clientOptions.Log.AllowedHttpHeaders); + auto const& perCallClientPolicies = clientOptions.PerOperationPolicies; auto const& perRetryClientPolicies = clientOptions.PerRetryPolicies; // Adding 6 for: @@ -128,7 +133,7 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { // Add a request activity policy which will generate distributed traces for the pipeline. m_policies.emplace_back( - std::make_unique()); + std::make_unique(inputSanitizer)); // logging - won't update request m_policies.emplace_back( diff --git a/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp b/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp new file mode 100644 index 0000000000..c0a9bd89de --- /dev/null +++ b/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#pragma once + +#include "azure/core/url.hpp" +#include + +namespace Azure { namespace Core { namespace _internal { + class InputSanitizer final { + /** + * @brief HTTP query parameter names that are allowed to be logged. + */ + std::set m_allowedHttpQueryParameters; + + /** + * @brief HTTP header names that are allowed to be logged. + */ + Azure::Core::CaseInsensitiveSet m_allowedHttpHeaders; + + // Manifest constant indicating a field was redacted. + constexpr static const char* m_RedactedPlaceholder = "REDACTED"; + + public: + InputSanitizer() = default; + InputSanitizer( + std::set const& allowedHttpQueryParameters, + Azure::Core::CaseInsensitiveSet const& allowedHttpHeaders) + : m_allowedHttpHeaders(allowedHttpHeaders), + m_allowedHttpQueryParameters(allowedHttpQueryParameters) + { + } + /** + * @brief Sanitizes the specified URL according to the sanitization rules configured. + * + * @param url Url to sanitize. Specified elements will be redacted from the URL. + * @return sanitized URL. + */ + Azure::Core::Url SanitizeUrl(Url const& url) const; + /** + * @brief Sanitizes the provided HTTP header value according to the sanitization rules + * configured. + * + * @param headerName Name of the header to sanitize. + * @param headerValue Current value of the header to sanitize. + * @return Sanitized header value. + */ + std::string SanitizeHeader(std::string const& headerName, std::string const& headerValue) const; + }; +}}} // namespace Azure::Core::_internal diff --git a/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp b/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp deleted file mode 100644 index ed817997d8..0000000000 --- a/sdk/core/azure-core/inc/azure/core/internal/request_sanitizer.hpp +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// SPDX-License-Identifier: MIT - -#pragma once - -#include "azure/core/url.hpp" -#include - -namespace Azure { namespace Core { namespace _internal { - class InputSanitizer final { - - public: - static Url SanitizeUrl(Url const& url); - }; -}}} // namespace Azure::Core::_internal diff --git a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp index 763a247bb9..9385277b75 100644 --- a/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp +++ b/sdk/core/azure-core/inc/azure/core/tracing/tracing.hpp @@ -112,7 +112,7 @@ namespace Azure { namespace Core { namespace Tracing { * @brief destroys an AttributeSet - virtual destructor to enable base class users to * destroy derived classes. */ - virtual ~AttributeSet(){}; + virtual ~AttributeSet() = default; }; /** @brief The Type of Span. diff --git a/sdk/core/azure-core/src/http/log_policy.cpp b/sdk/core/azure-core/src/http/log_policy.cpp index 1b824fcf8c..5c646ed2f7 100644 --- a/sdk/core/azure-core/src/http/log_policy.cpp +++ b/sdk/core/azure-core/src/http/log_policy.cpp @@ -21,8 +21,8 @@ std::string RedactedPlaceholder = "REDACTED"; inline void AppendHeaders( std::ostringstream& log, - Azure::Core::CaseInsensitiveMap const& headers, - Azure::Core::CaseInsensitiveSet const& allowedHaders) + Azure::Core::_internal::InputSanitizer const& inputSanitizer, + Azure::Core::CaseInsensitiveMap const& headers) { for (auto const& header : headers) { @@ -30,90 +30,27 @@ inline void AppendHeaders( if (!header.second.empty()) { - log - << ((allowedHaders.find(header.first) != allowedHaders.end()) ? header.second - : RedactedPlaceholder); + log << inputSanitizer.SanitizeHeader(header.first, header.second); } } } -inline void LogUrlWithoutQuery(std::ostringstream& log, Url const& url) +inline std::string GetRequestLogMessage( + Azure::Core::_internal::InputSanitizer const& inputSanitizer, + Request const& request) { - if (!url.GetScheme().empty()) - { - log << url.GetScheme() << "://"; - } - log << url.GetHost(); - if (url.GetPort() != 0) - { - log << ":" << url.GetPort(); - } - if (!url.GetPath().empty()) - { - log << "/" << url.GetPath(); - } -} - -inline std::string GetRequestLogMessage(LogOptions const& options, Request const& request) -{ - auto const& requestUrl = request.GetUrl(); - std::ostringstream log; log << "HTTP Request : " << request.GetMethod().ToString() << " "; - LogUrlWithoutQuery(log, requestUrl); - - { - auto encodedRequestQueryParams = requestUrl.GetQueryParameters(); - std::remove_const::type>::type - loggedQueryParams; + Azure::Core::Url urlToLog(inputSanitizer.SanitizeUrl(request.GetUrl())); + log << urlToLog.GetAbsoluteUrl(); - if (!encodedRequestQueryParams.empty()) - { - auto const& unencodedAllowedQueryParams = options.AllowedHttpQueryParameters; - if (!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); }); - - for (auto const& encodedRequestQueryParam : encodedRequestQueryParams) - { - if (encodedRequestQueryParam.second.empty() - || (encodedAllowedQueryParams.find(encodedRequestQueryParam.first) - != encodedAllowedQueryParams.end())) - { - loggedQueryParams.insert(encodedRequestQueryParam); - } - else - { - loggedQueryParams.insert( - std::make_pair(encodedRequestQueryParam.first, RedactedPlaceholder)); - } - } - } - else - { - for (auto const& encodedRequestQueryParam : encodedRequestQueryParams) - { - loggedQueryParams.insert( - std::make_pair(encodedRequestQueryParam.first, RedactedPlaceholder)); - } - } - - log << Azure::Core::_detail::FormatEncodedUrlQueryParameters(loggedQueryParams); - } - } - AppendHeaders(log, request.GetHeaders(), options.AllowedHttpHeaders); + AppendHeaders(log, inputSanitizer, request.GetHeaders()); return log.str(); } inline std::string GetResponseLogMessage( - LogOptions const& options, + Azure::Core::_internal::InputSanitizer const& inputSanitizer, RawResponse const& response, std::chrono::system_clock::duration const& duration) { @@ -124,14 +61,16 @@ inline std::string GetResponseLogMessage( << "ms) : " << static_cast(response.GetStatusCode()) << " " << response.GetReasonPhrase(); - AppendHeaders(log, response.GetHeaders(), options.AllowedHttpHeaders); + AppendHeaders(log, inputSanitizer, response.GetHeaders()); return log.str(); } } // namespace Azure::Core::CaseInsensitiveSet const Azure::Core::Http::Policies::_detail::g_defaultAllowedHttpHeaders - = {"x-ms-request-id", + = {"traceparent", + "tracestate", + "x-ms-request-id", "x-ms-client-request-id", "x-ms-return-client-request-id", "traceparent", @@ -165,7 +104,8 @@ std::unique_ptr LogPolicy::Send( if (Log::ShouldWrite(Logger::Level::Verbose)) { - Log::Write(Logger::Level::Informational, GetRequestLogMessage(m_options, request)); + Log::Write( + Logger::Level::Informational, GetRequestLogMessage(m_inputSanitizer, request)); } else { @@ -177,7 +117,8 @@ std::unique_ptr LogPolicy::Send( auto const end = std::chrono::system_clock::now(); Log::Write( - Logger::Level::Informational, GetResponseLogMessage(m_options, *response, end - start)); + Logger::Level::Informational, + GetResponseLogMessage(m_inputSanitizer, *response, end - start)); return response; } diff --git a/sdk/core/azure-core/src/http/request_activity_policy.cpp b/sdk/core/azure-core/src/http/request_activity_policy.cpp index 314db61fdc..8a480558d2 100644 --- a/sdk/core/azure-core/src/http/request_activity_policy.cpp +++ b/sdk/core/azure-core/src/http/request_activity_policy.cpp @@ -3,7 +3,7 @@ #include "azure/core/http/policies/policy.hpp" #include "azure/core/internal/diagnostics/log.hpp" -#include "azure/core/internal/request_sanitizer.hpp" +#include "azure/core/internal/input_sanitizer.hpp" #include "azure/core/internal/tracing/service_tracing.hpp" #include @@ -42,7 +42,7 @@ std::unique_ptr RequestActivityPolicy::Send( scope.AddAttribute(TracingAttributes::HttpMethod.ToString(), request.GetMethod().ToString()); scope.AddAttribute( "http.url", - Azure::Core::_internal::InputSanitizer::SanitizeUrl(request.GetUrl()).GetAbsoluteUrl()); + m_inputSanitizer.SanitizeUrl(request.GetUrl()).GetAbsoluteUrl()); { Azure::Nullable requestId = request.GetHeader("x-ms-client-request-id"); if (requestId.HasValue()) diff --git a/sdk/core/azure-core/src/private/input_sanitizer.cpp b/sdk/core/azure-core/src/private/input_sanitizer.cpp new file mode 100644 index 0000000000..7544eeaf2d --- /dev/null +++ b/sdk/core/azure-core/src/private/input_sanitizer.cpp @@ -0,0 +1,87 @@ + +#include "azure/core/internal/input_sanitizer.hpp" +#include "azure/core/url.hpp" +#include +#include + +namespace Azure { namespace Core { namespace _internal { + + + Azure::Core::Url InputSanitizer::SanitizeUrl(Azure::Core::Url const& url) const + { + std::ostringstream ss; + + // Sanitize the non-query part of the URL (remove username and password). + if (!url.GetScheme().empty()) + { + ss << url.GetScheme() << "://"; + } + ss << url.GetHost(); + if (url.GetPort() != 0) + { + ss << ":" << url.GetPort(); + } + if (!url.GetPath().empty()) + { + ss << "/" << url.GetPath(); + } + + + { + auto encodedRequestQueryParams = url.GetQueryParameters(); + + std::remove_const::type>::type + loggedQueryParams; + + if (!encodedRequestQueryParams.empty()) + { + auto const& unencodedAllowedQueryParams = m_allowedHttpQueryParameters; + if (!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); }); + + for (auto const& encodedRequestQueryParam : encodedRequestQueryParams) + { + if (encodedRequestQueryParam.second.empty() + || (encodedAllowedQueryParams.find(encodedRequestQueryParam.first) + != encodedAllowedQueryParams.end())) + { + loggedQueryParams.insert(encodedRequestQueryParam); + } + else + { + loggedQueryParams.insert( + std::make_pair(encodedRequestQueryParam.first, m_RedactedPlaceholder)); + } + } + } + else + { + for (auto const& encodedRequestQueryParam : encodedRequestQueryParams) + { + loggedQueryParams.insert( + std::make_pair(encodedRequestQueryParam.first, m_RedactedPlaceholder)); + } + } + + ss << Azure::Core::_detail::FormatEncodedUrlQueryParameters(loggedQueryParams); + } + } + return Azure::Core::Url(ss.str()); + } + + std::string InputSanitizer::SanitizeHeader(std::string const &header, std::string const& value) const + { + if (m_allowedHttpHeaders.find(header) != m_allowedHttpHeaders.end()) + { + return value; + } + return m_RedactedPlaceholder; + } +}}} // namespace Azure::Core::_internal diff --git a/sdk/core/azure-core/src/private/request_sanitizer.cpp b/sdk/core/azure-core/src/private/request_sanitizer.cpp deleted file mode 100644 index 1d0ca99931..0000000000 --- a/sdk/core/azure-core/src/private/request_sanitizer.cpp +++ /dev/null @@ -1,36 +0,0 @@ - -#include "azure/core/internal/request_sanitizer.hpp" -#include "azure/core/url.hpp" -#include - -namespace Azure { namespace Core { namespace _internal { - - Azure::Core::Url InputSanitizer::SanitizeUrl(Azure::Core::Url const& url) - { - Azure::Core::Url redactedUrl; - redactedUrl.SetScheme(url.GetScheme()); - auto host = url.GetHost(); - auto hostWithNoAccount = std::find(host.begin(), host.end(), '.'); - redactedUrl.SetHost("REDACTED" + std::string(hostWithNoAccount, host.end())); - // replace any uniqueID from the path for a hardcoded id - // For the regex, we should not assume anything about the version of UUID format being used. So, - // using the most general regex to get any uuid version. - redactedUrl.SetPath(std::regex_replace( - url.GetPath(), - std::regex("[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}"), - "33333333-3333-3333-3333-333333333333")); - // Query parameters - for (auto const& qp : url.GetQueryParameters()) - { - if (qp.first == "sig") - { - redactedUrl.AppendQueryParameter("sig", "REDACTED"); - } - else - { - redactedUrl.AppendQueryParameter(qp.first, qp.second); - } - } - return redactedUrl; - } -}}} // namespace Azure::Core::_internal diff --git a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp index 517d1edf18..6a001a1e4f 100644 --- a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp @@ -150,7 +150,8 @@ TEST(RequestActivityPolicy, Basic) { std::vector> policies; // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. - policies.emplace_back(std::make_unique()); + policies.emplace_back( + std::make_unique(Azure::Core::_internal::InputSanitizer{})); // Final policy - equivalent to HTTP policy. policies.emplace_back(std::make_unique()); @@ -185,7 +186,8 @@ TEST(RequestActivityPolicy, Basic) policies.emplace_back( std::make_unique("my-service-cpp", "1.0b2", clientOptions.Telemetry)); policies.emplace_back(std::make_unique(RetryOptions{})); - policies.emplace_back(std::make_unique()); + policies.emplace_back( + std::make_unique(Azure::Core::_internal::InputSanitizer{})); // Final policy - equivalent to HTTP policy. policies.emplace_back(std::make_unique()); @@ -223,7 +225,8 @@ TEST(RequestActivityPolicy, TryRetries) policies.emplace_back(std::make_unique(RetryOptions{})); // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. - policies.emplace_back(std::make_unique()); + policies.emplace_back( + std::make_unique(Azure::Core::_internal::InputSanitizer{})); // Final policy - equivalent to HTTP policy. int retryCount = 0; policies.emplace_back(std::make_unique([&](Request&) { From b73b110dd9cfd2b09f22e5b25cbb7ca01d99198f Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 09:58:18 -0700 Subject: [PATCH 13/28] clang-format --- .../src/private/attestation_client_private.hpp | 2 +- .../azure-core/inc/azure/core/internal/http/pipeline.hpp | 8 ++++---- sdk/core/azure-core/src/http/log_policy.cpp | 3 +-- sdk/core/azure-core/src/http/request_activity_policy.cpp | 4 +--- sdk/core/azure-core/src/private/input_sanitizer.cpp | 5 ++--- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp index bfc68430b3..c0b1ae8ea0 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp @@ -445,7 +445,7 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail /** * @brief Convert the internal attestation token to a public AttestationToken object. */ - operator Models::AttestationToken&() { return m_token; } + operator Models::AttestationToken &() { return m_token; } /** * @brief Convert the internal attestation token to a public AttestationToken object. */ 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 eec0515fa6..f2ee822752 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 @@ -12,9 +12,9 @@ #include "azure/core/context.hpp" #include "azure/core/http/http.hpp" #include "azure/core/http/policies/policy.hpp" -#include "azure/core/internal/input_sanitizer.hpp" #include "azure/core/http/transport.hpp" #include "azure/core/internal/client_options.hpp" +#include "azure/core/internal/input_sanitizer.hpp" #include #include @@ -78,8 +78,7 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { std::vector>&& perCallPolicies) { Azure::Core::_internal::InputSanitizer inputSanitizer( - clientOptions.Log.AllowedHttpQueryParameters, - clientOptions.Log.AllowedHttpHeaders); + clientOptions.Log.AllowedHttpQueryParameters, clientOptions.Log.AllowedHttpHeaders); auto const& perCallClientPolicies = clientOptions.PerOperationPolicies; auto const& perRetryClientPolicies = clientOptions.PerRetryPolicies; @@ -133,7 +132,8 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { // Add a request activity policy which will generate distributed traces for the pipeline. m_policies.emplace_back( - std::make_unique(inputSanitizer)); + std::make_unique( + inputSanitizer)); // logging - won't update request m_policies.emplace_back( diff --git a/sdk/core/azure-core/src/http/log_policy.cpp b/sdk/core/azure-core/src/http/log_policy.cpp index 5c646ed2f7..ae73d2c054 100644 --- a/sdk/core/azure-core/src/http/log_policy.cpp +++ b/sdk/core/azure-core/src/http/log_policy.cpp @@ -104,8 +104,7 @@ std::unique_ptr LogPolicy::Send( if (Log::ShouldWrite(Logger::Level::Verbose)) { - Log::Write( - Logger::Level::Informational, GetRequestLogMessage(m_inputSanitizer, request)); + Log::Write(Logger::Level::Informational, GetRequestLogMessage(m_inputSanitizer, request)); } else { diff --git a/sdk/core/azure-core/src/http/request_activity_policy.cpp b/sdk/core/azure-core/src/http/request_activity_policy.cpp index 8a480558d2..bf8f0f360a 100644 --- a/sdk/core/azure-core/src/http/request_activity_policy.cpp +++ b/sdk/core/azure-core/src/http/request_activity_policy.cpp @@ -40,9 +40,7 @@ std::unique_ptr RequestActivityPolicy::Send( auto scope = std::move(contextAndSpan.second); scope.AddAttribute(TracingAttributes::HttpMethod.ToString(), request.GetMethod().ToString()); - scope.AddAttribute( - "http.url", - m_inputSanitizer.SanitizeUrl(request.GetUrl()).GetAbsoluteUrl()); + scope.AddAttribute("http.url", m_inputSanitizer.SanitizeUrl(request.GetUrl()).GetAbsoluteUrl()); { Azure::Nullable requestId = request.GetHeader("x-ms-client-request-id"); if (requestId.HasValue()) diff --git a/sdk/core/azure-core/src/private/input_sanitizer.cpp b/sdk/core/azure-core/src/private/input_sanitizer.cpp index 7544eeaf2d..1adf130184 100644 --- a/sdk/core/azure-core/src/private/input_sanitizer.cpp +++ b/sdk/core/azure-core/src/private/input_sanitizer.cpp @@ -6,7 +6,6 @@ namespace Azure { namespace Core { namespace _internal { - Azure::Core::Url InputSanitizer::SanitizeUrl(Azure::Core::Url const& url) const { std::ostringstream ss; @@ -26,7 +25,6 @@ namespace Azure { namespace Core { namespace _internal { ss << "/" << url.GetPath(); } - { auto encodedRequestQueryParams = url.GetQueryParameters(); @@ -76,7 +74,8 @@ namespace Azure { namespace Core { namespace _internal { return Azure::Core::Url(ss.str()); } - std::string InputSanitizer::SanitizeHeader(std::string const &header, std::string const& value) const + std::string InputSanitizer::SanitizeHeader(std::string const& header, std::string const& value) + const { if (m_allowedHttpHeaders.find(header) != m_allowedHttpHeaders.end()) { From 85f01c088d122c75ff505ce96641a66320343464 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 10:11:51 -0700 Subject: [PATCH 14/28] Fixed unit test; added documentation on header propagation. --- .../CHANGELOG.md | 3 ++- .../src/opentelemetry.cpp | 18 ++++++++++++++++-- .../test/ut/service_support_test.cpp | 2 +- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md b/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md index e953124acf..3f2cb968cb 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md +++ b/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md @@ -2,5 +2,6 @@ ## 1.0.0-beta.1 (Unreleased) -### New Features +### Features Added + - Initial release diff --git a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp index 21704ea06e..673caec6af 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp @@ -197,10 +197,22 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { m_span->SetAttribute(attributeName, opentelemetry::common::AttributeValue(attributeValue)); } + /** + * @brief Text map propagator used to read or write properties from an HTTP request. + * + * @detail OpenTelemetry defines a `TextMapCarrier` class as a class which allows reading and + * writing to a map of text elements. The OpenTelemetry + * [HttpTraceContext](https://opentelemetry-cpp.readthedocs.io/en/latest/otel_docs/classopentelemetry_1_1trace_1_1propagation_1_1HttpTraceContext.html) + * uses a TextMapCarrier to propogate the required HTTP headers from an OpenTelemetry context + * into an HTTP request. + */ class HttpRequestTextMapPropagator : public opentelemetry::context::propagation::TextMapCarrier { Azure::Core::Http::Request& m_request; // Inherited via TextMapCarrier + + /** @brief Retrieves the value of an HTTP header from the request. + */ virtual opentelemetry::nostd::string_view Get( opentelemetry::nostd::string_view key) const noexcept override { @@ -212,6 +224,8 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { return std::string(); } + /** @brief Sets the value of an HTTP header in the request. + */ virtual void Set( opentelemetry::nostd::string_view key, opentelemetry::nostd::string_view value) noexcept override @@ -233,8 +247,8 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { auto scope = opentelemetry::trace::Tracer::WithActiveSpan(m_span); auto currentContext = opentelemetry::context::RuntimeContext::GetCurrent(); - opentelemetry::trace::propagation::HttpTraceContext httpTraceContext; - httpTraceContext.Inject(propagator, currentContext); + // And inject all required headers into the Request. + opentelemetry::trace::propagation::HttpTraceContext().Inject(propagator, currentContext); } } diff --git a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp index 985c2fbcc6..6e6495adda 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp @@ -641,7 +641,7 @@ TEST_F(OpenTelemetryServiceTests, ServiceApiImplementation) "attributes": { "az.namespace": "Azure.Core.OpenTelemetry.Test.Service", "http.method": "GET", - "http.url": "https://REDACTED.microsoft.com", + "http.url": "https://www.microsoft.com", "requestId": ".*", "http.user_agent": "MyApplication azsdk-cpp-Azure.Core.OpenTelemetry.Test.Service/1.0.0.beta-2.*", "http.status_code": "200" From 48abd746b82ae59512f1327315065800bfca550e Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 12:35:59 -0700 Subject: [PATCH 15/28] More distributed tracing documentation --- doc/DistributedTracing.md | 222 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 213 insertions(+), 9 deletions(-) diff --git a/doc/DistributedTracing.md b/doc/DistributedTracing.md index a0c704cc8f..533d8826d9 100644 --- a/doc/DistributedTracing.md +++ b/doc/DistributedTracing.md @@ -1,16 +1,19 @@ # Distributed Tracing in the C++ SDK + Azure has adopted [W3C Distributed Tracing](https://www.w3.org/TR/trace-context/) as a paradigm for correlating requests from clients across multiple services. -This document explains how the Azure C++ SDK implements distributed tracing, how clients integrate with distributed tracing, how +This document explains how the Azure C++ SDK implements distributed tracing, how clients integrate with distributed tracing, how services should integrate with distributed tracing and finally how the network pipeline and other functionality should integrate with distributed tracing. ## Tracing Overview + The Azure SDK for C++ Tracing APIs are modeled after the opentelemetry-cpp API surface defined in the [OpenTelemetry Tracing Specification](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md). Additional architectural information about OpenTelemetry can be found in [OpenTelemetry Concepts](https://opentelemetry.io/docs/concepts/). There are three major components which the Azure SDK components interact with: + - `TracerProvider` - this is a factory which creates `Tracer` objects. - `Tracer` - this is a factory which creates `Span` objects. - `Span` - Span objects are the APIs which allow tracing an operation. @@ -22,16 +25,17 @@ service will have a `Tracer` implementation which creates `Span` objects for eac A `Span` can be considered a "unit of work" for a service. Each service method (method which calls into the service) will have a single `Span` reflecting the client method which was called. -`Span`'s are hierarchical and each span can have multiple children (each `Span` can only have a single parent). The typical way that this manifests itself during a +`Span`'s are hierarchical and each span can have multiple children (each `Span` can only have a single parent). The typical way that this manifests itself during a service method call is: -* Service Method "MyServiceMethod" creates a span named "MyServiceMethod" and starts an HTTP request to communicate with the service. - * The HTTP pipeline (specifically the `RequestActivityPolicy`) will create a child `span` under the service method `span` named `"HTTP #0"`. This span - reflects the HTTP call into the service. - * If the HTTP call needs to be retried, the existing `span` will be closed an a new span named `HTTP #1` will be created for the retry. +- Service Method "MyServiceMethod" creates a span named "MyServiceMethod" and starts an HTTP request to communicate with the service. + - The HTTP pipeline (specifically the `RequestActivityPolicy`) will create a child `span` under the service method `span` named `"HTTP #0"`. This span + reflects the HTTP call into the service. + - If the HTTP call needs to be retried, the existing `span` will be closed an a new span named `HTTP #1` will be created for the retry. + +## Distributed Tracing Client Integration -## Distributed Tracing Client Integration. Applications which wish to integrate Distributed Tracing are strongly encouraged to use the [opentelemetry-cpp](https://github.com/open-telemetry/opentelemetry-cpp) vcpkg package. @@ -39,5 +43,205 @@ There are numerous examples on the OpenTelemetry web site which demonstrate how opentelemetry into a customer application and integrate the generated traces with Azure monitoring infrastructure such as Geneva Monitoring. -# Distributed Tracing components. -In order to maintain flexibility, the actual distributed tracing mechanism is \ No newline at end of file +Following the examples from opentelemetry-cpp, the following can be used +to establish an OpenTelemetry exporter which logs to the console or to an +in-memory logger. + +```c++ +opentelemetry::nostd::shared_ptr +CreateOpenTelemetryProvider() +{ +#if USE_MEMORY_EXPORTER + auto exporter = std::make_unique(); +#else + auto exporter = std::make_unique(); +#endif + + // simple processor + auto simple_processor = std::unique_ptr( + new opentelemetry::sdk::trace::SimpleSpanProcessor(std::move(exporter))); + + auto always_on_sampler = std::unique_ptr( + new opentelemetry::sdk::trace::AlwaysOnSampler); + + auto resource_attributes = opentelemetry::sdk::resource::ResourceAttributes{ + {"service.name", "telemetryTest"}, {"service.instance.id", "instance-1"}}; + auto resource = opentelemetry::sdk::resource::Resource::Create(resource_attributes); + // Create using SDK configurations as parameter + return opentelemetry::nostd::shared_ptr( + new opentelemetry::sdk::trace::TracerProvider( + std::move(simple_processor), resource, std::move(always_on_sampler))); +} +``` + +Other exporters exist to export to [Jaeger](https://github.com/open-telemetry/opentelemetry-cpp/tree/main/exporters/jaeger), +[Windows ETW](https://github.com/open-telemetry/opentelemetry-cpp/tree/main/exporters/etw) and others. + +Once the `opentelemetry::trace::TracerProvider` has been created, The client needs to create a new `Azure::Core::Tracing::OpenTelemetry::OpenTelemetryProvider` which +functions as an abstract class integration between OpenTelemetry and Azure Core: + +```c++ +std::shared_ptr traceProvider + = std::make_shared(CreateOpenTelemetryProvider()); +``` + +To finish the integration with Azure clients, there are two mechanisms to integrate OpenTelemetry into a client application: + +1) `Azure::Core::Context` integration. +1) Service Client Options integration. + +### Integrate an OpenTelemetryProvider via the ApplicationContext + +To integrate OpenTelemetry for all Azure Clients in the application, the customer can call `Azure::Core::Context::ApplicationContext.SetTracerProvider` to establish the +tracer provider for the application. + +```c++ + Azure::Core::Context::ApplicationContext.SetTracerProvider(provider); +``` + +### Integrate an OpenTelemetryProvider via Service ClientOptions + +While using the ApplicationContext is the simplest mechanism for integration OpenTelemetry with a customer application, there may be times the customer needs more flexibility when creating service clients. +To enable customers to further customize how tracing works, the application can set the `Telemetry.TracingProvider` field in the service client options, which will establish the tracer provider used by +the service client. + +```c++ +auto tracerProvider(CreateOpenTelemetryProvider()); +auto provider(std::make_shared(tracerProvider)); + +ServiceClientOptions clientOptions; +clientOptions.Telemetry.TracingProvider = provider; +clientOptions.Telemetry.ApplicationId = "MyApplication"; +ServiceClient myServiceClient(clientOptions); +``` + +## Distributed Tracing Service Integration + +There are two steps needed to integrate Distributed Tracing with a Service Client. + +1. Add a `DiagnosticTracingFactory` object to the ServiceClient object +1. Update each service method as follows: + 1. Add a call to the `CreateSpan` method on the diagnostic tracing factory. This will create a new span for the client operation. + 1. Call `SetStatus` on the created span when the service method successfully completes. + 1. Wrap the client method code with a try/catch handler which catches exceptions and call AddEvent with the value of the exception. + +### Add a `DiagnosticTracingFactory` to the serviceClient class + +To add a new `DiagnosticTracingFactory` to the client, simply add the class as a member: + +```c++ + Azure::Core::Tracing::_internal::DiagnosticTracingFactory m_tracingFactory; + +``` + +And construct the new tracing factory in the service constructor: + +```c++ + explicit ServiceClient(ServiceClientOptions const& clientOptions = ServiceClientOptions{}) + : m_tracingFactory(clientOptions, "Azure.Core.OpenTelemetry.Test.Service", PackageVersion::ToString()) + ``` + +### Update Each Service Method + + There are three methods of interest when updating the service method: + + 1. `DiagnosticTracingFactory::CreateSpan` - this creates and returns a `Span` and `Context` object for the service method. The returned Context object must be used for subsequent service operations. + 1. `Span::AddEvent(std::exception&)` - This registers the exception with the distributed tracing infrastructure. + 1. `Span::SetStatus` - This sets the status of the operation in the trace. + + ```c++ + Azure::Response ServiceMethod( + std::string const&, + Azure::Core::Context const& context = Azure::Core::Context{}) + { + // Create a new context and span for this request. + auto contextAndSpan = m_tracingFactory.CreateSpan( + "ServiceMethod", Azure::Core::Tracing::_internal::SpanKind::Internal, context); + + // contextAndSpan.first is the new context for the operation. + // contextAndSpan.second is the new span for the operation. + + try + { + // + Azure::Core::Http::Request requestToSend( + HttpMethod::Get, Azure::Core::Url("")); + + std::unique_ptr response + = m_pipeline->Send(requestToSend, contextAndSpan.first); + contextAndSpan.second.SetStatus(Azure::Core::Tracing::_internal::SpanStatus::Ok); + return Azure::Response("", std::move(response)); + } + catch (std::exception const& ex) + { + // Register that the exception has happened and that the span is now in error. + contextAndSpan.second.AddEvent(ex); + contextAndSpan.second.SetStatus(Azure::Core::Tracing::_internal::SpanStatus::Error); + throw; + } + + // When contextAndSpan.second goes out of scope, it ends the span, which will record it. + } +}; +``` + +## Implementation Details + +### Distributed Tracing components + +In order to maintain flexibility, the opentelemetry-cpp APIs are implemented in a separate package - azure-core-tracing-opentelemetry. +This is consistent with how opentelemetry is distributed for +the other Azure SDKs. + +The Azure Core API surface interacts with a set of pure virtual base classes (aka "interfaces") in +the `Azure::Core::Tracing` and `Azure::Core::Tracing::_internal` namespace. These allow a level of separation +between the Azure Core API surface and the OpenTelemetry API surface - an alternative tracing mechanism needs +to provide APIs consistent with the `Azure::Core::Tracing` APIs. + +The azure-core-tracing-openetelemetry-cpp package implements a set of APIs in the `Azure::Core::Tracing::OpenTelemetry` +and `Azure::Core::Tracing::OpenTelemetry::_detail` namespace. These provide an Azure Core compatable API surface for distributed tracing. + +The core service client interface is the `DiagnosticTracingFactory` class which implements two APIs: `CreateSpan` and +`CreateSpanFromContext`. `CreateSpan` is intended to be used by service methods which have direct access to a +`DiagnosticTracingFactory` object, `CreateSpanFromContext` in intended to be used from code which does NOT have +direct access to the `DiagnosticTracingFactory`. + +The final significant piece of the distributed tracing infrastructure is the `RequestActivityPolicy` - this policy MUST be +inserted into the HTTP pipeline AFTER the `RetryPolicy`. It is responsible for creating the span associated with the HTTP request, it will +also propagate the W3C distributed tracing headers from the span into the HTTP request. + +### Generated traces + +The Azure standards for distributed tracing are define in [Azure Distributed Tracing Conventions](https://github.com/Azure/azure-sdk/blob/main/docs/tracing/distributed-tracing-conventions.md). +The actual tracing elements generated by Azure services are defined in [Azure Tracing Conventions YAML](https://github.com/Azure/azure-sdk/blob/main/docs/tracing/distributed-tracing-conventions.yml). + +In summary, these are the traces and attributes which should be generated +for azure services: + +#### Spans + +The distributed tracing standards define the following traces: + +##### Public APIs + +All public APIs MUST create a span which will describes the API. +The name of the span MUST be the API name. + +##### HTTP Calls + +Each HTTP request sent to the service MUST create a span describing the request to the service. +The name of the span MUST be of the form `HTTP #`. + +#### Attributes + +Generated traces have the following attributes: + +| Attribute Name | Semantics | Where Used +|-----------|--------|------- +| `az.namespace` |Namespace of the azure service request| All spans. +| `http.method`| HTTP Method ("GET", "PUT", etc)| HTTP Spans. +| `http.url`| URL being retrieved (sanitized)| HTTP Spans. +| `http.status_code` | HTTP status code returned by the service | HTTP Spans. +| `http.user_agent` | The value of the `User-Agent` HTTP header sent to the service | HTTP Spans. +| `requestId` | The value of the `x-ms-client-request-id` header sent by the client | HTTP Spans. +| `serviceRequestId` | The value -f the `x-ms-request-id` sent by the server | HTTP Spans. From 72550ee6af8707c861305d331f98d24756f522ea Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 12:36:47 -0700 Subject: [PATCH 16/28] Fixed some typos --- .../test/ut/service_support_test.cpp | 8 +-- sdk/core/azure-core/src/http/log_policy.cpp | 51 ++++++++++--------- .../src/http/request_activity_policy.cpp | 6 ++- 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp index 6e6495adda..38b0a42d17 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp @@ -532,12 +532,12 @@ class ServiceClientOptions : public Azure::Core::_internal::ClientOptions { class ServiceClient { private: ServiceClientOptions m_clientOptions; - Azure::Core::Tracing::_internal::DiagnosticTracingFactory m_serviceTrace; + Azure::Core::Tracing::_internal::DiagnosticTracingFactory m_tracingFactory; std::unique_ptr m_pipeline; public: explicit ServiceClient(ServiceClientOptions const& clientOptions = ServiceClientOptions{}) - : m_serviceTrace(clientOptions, "Azure.Core.OpenTelemetry.Test.Service", "1.0.0.beta-2") + : m_tracingFactory(clientOptions, "Azure.Core.OpenTelemetry.Test.Service", "1.0.0.beta-2") { std::vector> policies; policies.emplace_back(std::make_unique( @@ -566,7 +566,7 @@ class ServiceClient { std::string const& inputString, Azure::Core::Context const& context = Azure::Core::Context{}) { - auto contextAndSpan = m_serviceTrace.CreateSpan( + auto contextAndSpan = m_tracingFactory.CreateSpan( "GetConfigurationString", Azure::Core::Tracing::_internal::SpanKind::Internal, context); // @@ -587,7 +587,7 @@ class ServiceClient { std::string const&, Azure::Core::Context const& context = Azure::Core::Context{}) { - auto contextAndSpan = m_serviceTrace.CreateSpan( + auto contextAndSpan = m_tracingFactory.CreateSpan( "ApiWhichThrows", Azure::Core::Tracing::_internal::SpanKind::Internal, context); try diff --git a/sdk/core/azure-core/src/http/log_policy.cpp b/sdk/core/azure-core/src/http/log_policy.cpp index ae73d2c054..2fe4cf5884 100644 --- a/sdk/core/azure-core/src/http/log_policy.cpp +++ b/sdk/core/azure-core/src/http/log_policy.cpp @@ -68,31 +68,32 @@ inline std::string GetResponseLogMessage( Azure::Core::CaseInsensitiveSet const Azure::Core::Http::Policies::_detail::g_defaultAllowedHttpHeaders - = {"traceparent", - "tracestate", - "x-ms-request-id", - "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"}; + = { + "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", + "traceparent", + "tracestate", + "Transfer-Encoding", + "User-Agent" + "x-ms-client-request-id", + "x-ms-request-id", + "x-ms-return-client-request-id", +}; std::unique_ptr LogPolicy::Send( Request& request, diff --git a/sdk/core/azure-core/src/http/request_activity_policy.cpp b/sdk/core/azure-core/src/http/request_activity_policy.cpp index bf8f0f360a..556830a2e4 100644 --- a/sdk/core/azure-core/src/http/request_activity_policy.cpp +++ b/sdk/core/azure-core/src/http/request_activity_policy.cpp @@ -57,19 +57,21 @@ std::unique_ptr RequestActivityPolicy::Send( } } - // Propogate information from the scope to the HTTP headers. + // Propagate information from the scope to the HTTP headers. // // This will add the "traceparent" header and any other OpenTelemetry related headers. scope.PropagateToHttpHeaders(request); try { + // Send the request on to the service. auto response = nextPolicy.Send(request, contextAndSpan.first); + // And register the headers we received from the service. scope.AddAttribute( TracingAttributes::HttpStatusCode.ToString(), std::to_string(static_cast(response->GetStatusCode()))); - auto& responseHeaders = response->GetHeaders(); + auto const& responseHeaders = response->GetHeaders(); auto serviceRequestId = responseHeaders.find("x-ms-request-id"); if (serviceRequestId != responseHeaders.end()) { From d3f4964e6ad3f72069a4598101a327006ed0267e Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 13:10:19 -0700 Subject: [PATCH 17/28] clang-format and fixed clang warning --- .../src/private/attestation_client_private.hpp | 4 ++-- .../inc/azure/core/internal/input_sanitizer.hpp | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp index c0b1ae8ea0..ba8aa920bc 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp @@ -445,10 +445,10 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail /** * @brief Convert the internal attestation token to a public AttestationToken object. */ - operator Models::AttestationToken &() { return m_token; } + operator Models::AttestationToken&() { return m_token; } /** * @brief Convert the internal attestation token to a public AttestationToken object. */ - operator Models::AttestationToken const &() const { return m_token; } + operator Models::AttestationToken const&() const { return m_token; } }; }}}} // namespace Azure::Security::Attestation::_detail diff --git a/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp b/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp index c0a9bd89de..4e577e444f 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp @@ -9,14 +9,14 @@ namespace Azure { namespace Core { namespace _internal { class InputSanitizer final { /** - * @brief HTTP query parameter names that are allowed to be logged. + * @brief HTTP header names that are allowed to be logged. */ - std::set m_allowedHttpQueryParameters; + Azure::Core::CaseInsensitiveSet m_allowedHttpHeaders; /** - * @brief HTTP header names that are allowed to be logged. + * @brief HTTP query parameter names that are allowed to be logged. */ - Azure::Core::CaseInsensitiveSet m_allowedHttpHeaders; + std::set m_allowedHttpQueryParameters; // Manifest constant indicating a field was redacted. constexpr static const char* m_RedactedPlaceholder = "REDACTED"; From 799226f06f80645ee11eeab5267daab378bccf88 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 13:14:55 -0700 Subject: [PATCH 18/28] Added opentelemetry to cspell.json --- .vscode/cspell.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.vscode/cspell.json b/.vscode/cspell.json index 1799bc96c4..488cad2e31 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -92,6 +92,7 @@ "northcentralus", "NTSTATUS", "okhttp", + "opentelemetry", "otel", "PBYTE", "pdbs", From d1a3419592180b406cadf9df9f6e3e992692b57d Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 13:22:12 -0700 Subject: [PATCH 19/28] cspell --- sdk/attestation/azure-security-attestation/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/attestation/azure-security-attestation/README.md b/sdk/attestation/azure-security-attestation/README.md index 01b6750642..b191f1a48b 100644 --- a/sdk/attestation/azure-security-attestation/README.md +++ b/sdk/attestation/azure-security-attestation/README.md @@ -1,3 +1,7 @@ +--- +# cspell:words opentelemetry +--- + # Azure Attestation Package client library for C++ Microsoft Azure Attestation is a unified solution for remotely verifying the trustworthiness of a platform and integrity of the binaries running inside it. The service supports attestation of the platforms backed by Trusted Platform Modules (TPMs) alongside the ability to attest to the state of Trusted Execution Environments (TEEs) such as Intel(tm) Software Guard Extensions (SGX) enclaves and Virtualization-based Security (VBS) enclaves. From 696834a3d0e0be032175e83831eb3d3304676504 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 13:23:40 -0700 Subject: [PATCH 20/28] detail->details --- sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp index 673caec6af..e3968e0837 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/src/opentelemetry.cpp @@ -200,7 +200,7 @@ namespace Azure { namespace Core { namespace Tracing { namespace OpenTelemetry { /** * @brief Text map propagator used to read or write properties from an HTTP request. * - * @detail OpenTelemetry defines a `TextMapCarrier` class as a class which allows reading and + * @details OpenTelemetry defines a `TextMapCarrier` class as a class which allows reading and * writing to a map of text elements. The OpenTelemetry * [HttpTraceContext](https://opentelemetry-cpp.readthedocs.io/en/latest/otel_docs/classopentelemetry_1_1trace_1_1propagation_1_1HttpTraceContext.html) * uses a TextMapCarrier to propogate the required HTTP headers from an OpenTelemetry context From 3dc5c2829035850832a98215063dbe2389a73f31 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 13:55:18 -0700 Subject: [PATCH 21/28] Removed constexpr from redacted constant --- .../azure-core/inc/azure/core/internal/input_sanitizer.hpp | 2 +- sdk/core/azure-core/src/private/input_sanitizer.cpp | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp b/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp index 4e577e444f..2322f341a5 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp @@ -19,7 +19,7 @@ namespace Azure { namespace Core { namespace _internal { std::set m_allowedHttpQueryParameters; // Manifest constant indicating a field was redacted. - constexpr static const char* m_RedactedPlaceholder = "REDACTED"; + static const char* m_RedactedPlaceholder; public: InputSanitizer() = default; diff --git a/sdk/core/azure-core/src/private/input_sanitizer.cpp b/sdk/core/azure-core/src/private/input_sanitizer.cpp index 1adf130184..be2494e059 100644 --- a/sdk/core/azure-core/src/private/input_sanitizer.cpp +++ b/sdk/core/azure-core/src/private/input_sanitizer.cpp @@ -6,6 +6,8 @@ namespace Azure { namespace Core { namespace _internal { + const char* InputSanitizer::m_RedactedPlaceholder = "REDACTED"; + Azure::Core::Url InputSanitizer::SanitizeUrl(Azure::Core::Url const& url) const { std::ostringstream ss; @@ -83,4 +85,5 @@ namespace Azure { namespace Core { namespace _internal { } return m_RedactedPlaceholder; } + }}} // namespace Azure::Core::_internal From 2201f652366a3678b2300100732c13359c23343e Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 14:00:56 -0700 Subject: [PATCH 22/28] CSpell --- doc/DistributedTracing.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/DistributedTracing.md b/doc/DistributedTracing.md index 533d8826d9..ecf6e3be3f 100644 --- a/doc/DistributedTracing.md +++ b/doc/DistributedTracing.md @@ -1,3 +1,6 @@ +--- +# cspell:words openetelemetry +--- # Distributed Tracing in the C++ SDK Azure has adopted [W3C Distributed Tracing](https://www.w3.org/TR/trace-context/) as a paradigm for correlating From 9c161fdf6496bbb3b3a70eb625ce97156c7c46cd Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 14:21:53 -0700 Subject: [PATCH 23/28] clang-format --- .../src/private/attestation_client_private.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp index ba8aa920bc..c0b1ae8ea0 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp @@ -445,10 +445,10 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail /** * @brief Convert the internal attestation token to a public AttestationToken object. */ - operator Models::AttestationToken&() { return m_token; } + operator Models::AttestationToken &() { return m_token; } /** * @brief Convert the internal attestation token to a public AttestationToken object. */ - operator Models::AttestationToken const&() const { return m_token; } + operator Models::AttestationToken const &() const { return m_token; } }; }}}} // namespace Azure::Security::Attestation::_detail From ddedc06416667bf9e4cbd6296335195ef9739aa7 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 14:37:19 -0700 Subject: [PATCH 24/28] clang-format --- .../src/private/attestation_client_private.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp index c0b1ae8ea0..bfc68430b3 100644 --- a/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp +++ b/sdk/attestation/azure-security-attestation/src/private/attestation_client_private.hpp @@ -445,7 +445,7 @@ namespace Azure { namespace Security { namespace Attestation { namespace _detail /** * @brief Convert the internal attestation token to a public AttestationToken object. */ - operator Models::AttestationToken &() { return m_token; } + operator Models::AttestationToken&() { return m_token; } /** * @brief Convert the internal attestation token to a public AttestationToken object. */ From 4565386bc3bfee527519a515ebd9798b20693154 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 15:24:34 -0700 Subject: [PATCH 25/28] Static tests need MSVC_USE_STATIC_CRT --- .../templates/stages/platform-matrix-live.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/eng/pipelines/templates/stages/platform-matrix-live.json b/eng/pipelines/templates/stages/platform-matrix-live.json index 07787623a9..cfe43a4e67 100644 --- a/eng/pipelines/templates/stages/platform-matrix-live.json +++ b/eng/pipelines/templates/stages/platform-matrix-live.json @@ -73,7 +73,7 @@ "Win_x86_with_unit_test_winHttp": { "VcpkgInstall": "openssl", "CMAKE_GENERATOR_PLATFORM": "Win32", - "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON ", + "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DMSVC_USE_STATIC_CRT=ON ", "VCPKG_DEFAULT_TRIPLET": "x86-windows-static", "WindowsCtestConfig": "-C Release", "BuildArgs": "-v --parallel 8 --config Release" @@ -81,7 +81,7 @@ "Win_x86_no_rtti_whit_unit_test": { "VcpkgInstall": "libxml2 openssl", "CMAKE_GENERATOR_PLATFORM": "Win32", - "CmakeArgs": " -DBUILD_RTTI=OFF -DBUILD_TESTING=ON -DBUILD_SAMPLES=ON", + "CmakeArgs": " -DBUILD_RTTI=OFF -DBUILD_TESTING=ON -DBUILD_SAMPLES=ON -DMSVC_USE_STATIC_CRT=ON", "VCPKG_DEFAULT_TRIPLET": "x86-windows-static", "WindowsCtestConfig": "-C Release", "BuildArgs": "-v --parallel 8 --config Release" @@ -89,13 +89,13 @@ "Win_x86_with_unit_test_libcurl": { "CMAKE_GENERATOR_PLATFORM": "Win32", "VCPKG_DEFAULT_TRIPLET": "x86-windows-static", - "CmakeArgs": " -DBUILD_TRANSPORT_CURL=ON -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON ", + "CmakeArgs": " -DBUILD_TRANSPORT_CURL=ON -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DMSVC_USE_STATIC_CRT=ON ", "BuildArgs": "-v --parallel 8" }, "Win_x64_with_unit_test_winHttp": { "VcpkgInstall": "openssl", "CMAKE_GENERATOR_PLATFORM": "x64", - "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON ", + "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DMSVC_USE_STATIC_CRT=ON ", "BuildArgs": "-v --parallel 8 --config Release", "AZURE_CORE_ENABLE_JSON_TESTS": 1, "VCPKG_DEFAULT_TRIPLET": "x64-windows-static", @@ -105,7 +105,7 @@ "VcpkgInstall": "openssl", "VCPKG_DEFAULT_TRIPLET": "x64-windows-static", "CMAKE_GENERATOR_PLATFORM": "x64", - "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DBUILD_SAMPLES=ON ", + "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DBUILD_SAMPLES=ON -DMSVC_USE_STATIC_CRT=ON ", "BuildArgs": "-v --parallel 8 --config Release", "AZURE_CORE_ENABLE_JSON_TESTS": 1, "RunSamples": 1, @@ -122,7 +122,7 @@ "VcpkgInstall": "curl[winssl] openssl", "VCPKG_DEFAULT_TRIPLET": "x64-windows-static", "CMAKE_GENERATOR_PLATFORM": "x64", - "CmakeArgs": " -DBUILD_TRANSPORT_CURL=ON -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DBUILD_SAMPLES=ON ", + "CmakeArgs": " -DBUILD_TRANSPORT_CURL=ON -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DBUILD_SAMPLES=ON -DMSVC_USE_STATIC_CRT=ON ", "BuildArgs": "-v --parallel 8 --config Release", "RunSamples": 1, "WindowsCtestConfig": "-C Release" From aa395e45102615f8c7e4d9f1bf5e3a9941a3cca0 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 16:12:39 -0700 Subject: [PATCH 26/28] Missed setting static CRT in one place --- eng/pipelines/templates/stages/platform-matrix-live.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/pipelines/templates/stages/platform-matrix-live.json b/eng/pipelines/templates/stages/platform-matrix-live.json index cfe43a4e67..3338ff0de5 100644 --- a/eng/pipelines/templates/stages/platform-matrix-live.json +++ b/eng/pipelines/templates/stages/platform-matrix-live.json @@ -114,7 +114,7 @@ "Win_x64_with_unit_test_libcurl": { "VCPKG_DEFAULT_TRIPLET": "x64-windows-static", "CMAKE_GENERATOR_PLATFORM": "x64", - "CmakeArgs": " -DBUILD_TRANSPORT_CURL=ON -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON ", + "CmakeArgs": " -DBUILD_TRANSPORT_CURL=ON -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DMSVC_USE_STATIC_CRT=ON ", "BuildArgs": "-v --parallel 8 --config Release", "WindowsCtestConfig": "-C Release" }, From c7d5226886d422a5e1381b20d8a5eaf0cdbfb226 Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 16:54:21 -0700 Subject: [PATCH 27/28] Don't run otel tests and json tests in the same test stage --- .../stages/platform-matrix-live.json | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/eng/pipelines/templates/stages/platform-matrix-live.json b/eng/pipelines/templates/stages/platform-matrix-live.json index 3338ff0de5..151c57a2ae 100644 --- a/eng/pipelines/templates/stages/platform-matrix-live.json +++ b/eng/pipelines/templates/stages/platform-matrix-live.json @@ -78,7 +78,7 @@ "WindowsCtestConfig": "-C Release", "BuildArgs": "-v --parallel 8 --config Release" }, - "Win_x86_no_rtti_whit_unit_test": { + "Win_x86_no_rtti_with_unit_test": { "VcpkgInstall": "libxml2 openssl", "CMAKE_GENERATOR_PLATFORM": "Win32", "CmakeArgs": " -DBUILD_RTTI=OFF -DBUILD_TESTING=ON -DBUILD_SAMPLES=ON -DMSVC_USE_STATIC_CRT=ON", @@ -92,12 +92,30 @@ "CmakeArgs": " -DBUILD_TRANSPORT_CURL=ON -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DMSVC_USE_STATIC_CRT=ON ", "BuildArgs": "-v --parallel 8" }, + "Win_x64_with_json_unit_test_winHttp": { + "VcpkgInstall": "openssl", + "CMAKE_GENERATOR_PLATFORM": "x64", + "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DDISABLE_AZURE_CORE_OPENTELEMETRY=ON ", + "BuildArgs": "-v --parallel 8 --config Release", + "AZURE_CORE_ENABLE_JSON_TESTS": 1, + "VCPKG_DEFAULT_TRIPLET": "x64-windows-static", + "WindowsCtestConfig": "-C Release" + }, + "Win_x64_with_json_unit_samples_winHttp": { + "VcpkgInstall": "openssl", + "VCPKG_DEFAULT_TRIPLET": "x64-windows-static", + "CMAKE_GENERATOR_PLATFORM": "x64", + "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DBUILD_SAMPLES=ON -DDISABLE_AZURE_CORE_OPENTELEMETRY=ON ", + "BuildArgs": "-v --parallel 8 --config Release", + "AZURE_CORE_ENABLE_JSON_TESTS": 1, + "RunSamples": 1, + "WindowsCtestConfig": "-C Release" + }, "Win_x64_with_unit_test_winHttp": { "VcpkgInstall": "openssl", "CMAKE_GENERATOR_PLATFORM": "x64", "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DMSVC_USE_STATIC_CRT=ON ", "BuildArgs": "-v --parallel 8 --config Release", - "AZURE_CORE_ENABLE_JSON_TESTS": 1, "VCPKG_DEFAULT_TRIPLET": "x64-windows-static", "WindowsCtestConfig": "-C Release" }, @@ -107,7 +125,6 @@ "CMAKE_GENERATOR_PLATFORM": "x64", "CmakeArgs": " -DBUILD_TESTING=ON -DRUN_LONG_UNIT_TESTS=ON -DBUILD_PERFORMANCE_TESTS=ON -DBUILD_SAMPLES=ON -DMSVC_USE_STATIC_CRT=ON ", "BuildArgs": "-v --parallel 8 --config Release", - "AZURE_CORE_ENABLE_JSON_TESTS": 1, "RunSamples": 1, "WindowsCtestConfig": "-C Release" }, From 739903f77d80456fe61fe5ffed579ea09e6109cd Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 27 May 2022 17:28:56 -0700 Subject: [PATCH 28/28] Moved opentelemetry code coverage under core --- sdk/core/azure-core-tracing-opentelemetry/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core-tracing-opentelemetry/CMakeLists.txt b/sdk/core/azure-core-tracing-opentelemetry/CMakeLists.txt index e0dfb353de..4f81df1937 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/CMakeLists.txt +++ b/sdk/core/azure-core-tracing-opentelemetry/CMakeLists.txt @@ -56,7 +56,7 @@ target_include_directories( add_library(Azure::azure-core-tracing-opentelemetry ALIAS azure-core-tracing-opentelemetry) # coverage. Has no effect if BUILD_CODE_COVERAGE is OFF -create_code_coverage(core-tracing-opentelemetry azure-core-tracing-opentelemetry azure-core-tracing-opentelemetry-test "tests?/*;samples?/*") +create_code_coverage(core azure-core-tracing-opentelemetry azure-core-tracing-opentelemetry-test "tests?/*;samples?/*") target_link_libraries(azure-core-tracing-opentelemetry INTERFACE Threads::Threads)