Skip to content

Commit

Permalink
OpenTelemetry API Review Feedback (#3687)
Browse files Browse the repository at this point in the history 
* OpenTelemetry API Review Feedback
  • Loading branch information
@LarryOsterman
LarryOsterman authored Jun 2, 2022
1 parent 0fd0267 commit ebe084b
Show file tree
Hide file tree
Showing 6 changed files with 208 additions and 113 deletions.
50 changes: 49 additions & 1 deletion sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,54 @@ TEST_F(OpenTelemetryServiceTests, CreateWithImplicitProvider)
Azure::Core::Context::ApplicationContext.SetTracerProvider(nullptr);
}

TEST_F(OpenTelemetryServiceTests, CreateSpanWithOptions)
{
{
auto tracerProvider(CreateOpenTelemetryProvider());
auto provider(std::make_shared<Azure::Core::Tracing::OpenTelemetry::OpenTelemetryProvider>(
tracerProvider));

Azure::Core::Context::ApplicationContext.SetTracerProvider(provider);

{
Azure::Core::_internal::ClientOptions clientOptions;
clientOptions.Telemetry.ApplicationId = "MyApplication";

Azure::Core::Tracing::_internal::DiagnosticTracingFactory serviceTrace(
clientOptions, "my-service", "1.0beta-2");

Azure::Core::Context clientContext;
Azure::Core::Tracing::_internal::CreateSpanOptions createOptions;
createOptions.Kind = Azure::Core::Tracing::_internal::SpanKind::Internal;
createOptions.Attributes = serviceTrace.CreateAttributeSet();
createOptions.Attributes->AddAttribute("TestAttribute", 3);
auto contextAndSpan = serviceTrace.CreateSpan("My API", createOptions, clientContext);
EXPECT_FALSE(contextAndSpan.first.IsCancelled());
}

// Now let's verify what was logged via OpenTelemetry.
auto spans = m_spanData->GetSpans();
EXPECT_EQ(1ul, spans.size());

VerifySpan(spans[0], R"(
{
"name": "My API",
"kind": "internal",
"attributes": {
"az.namespace": "my-service",
"TestAttribute": 3
},
"library": {
"name": "my-service",
"version": "1.0beta-2"
}
})");
}

// Clear the global tracer provider set earlier in the test.
Azure::Core::Context::ApplicationContext.SetTracerProvider(nullptr);
}

TEST_F(OpenTelemetryServiceTests, NestSpans)
{
{
Expand Down Expand Up @@ -635,7 +683,7 @@ TEST_F(OpenTelemetryServiceTests, ServiceApiImplementation)

VerifySpan(spans[0], R"(
{
"name": "HTTP GET #0",
"name": "HTTP GET",
"kind": "client",
"statusCode": "unset",
"attributes": {
Expand Down
14 changes: 6 additions & 8 deletions sdk/core/azure-core/inc/azure/core/internal/tracing/service_tracing.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,6 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal {
// using TracingContext = std::pair<std::shared_ptr<Span>, std::shared_ptr<Tracer>>;
using TracingContext = std::shared_ptr<Span>;

static DiagnosticTracingFactory* DiagnosticFactoryFromContext(
Azure::Core::Context const& context);

static Azure::Nullable<TracingContext> TracingContextFromContext(
Azure::Core::Context const& context);

public:
DiagnosticTracingFactory(
Azure::Core::_internal::ClientOptions const& options,
Expand All @@ -200,6 +194,7 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal {
}

DiagnosticTracingFactory() = default;
DiagnosticTracingFactory(DiagnosticTracingFactory const&) = default;

/** @brief A ContextAndSpan provides an updated Context object and a new span object
* which can be used to add events and attributes to the span.
Expand All @@ -211,12 +206,15 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal {
Azure::Core::Tracing::_internal::SpanKind const& spanKind,
Azure::Core::Context const& clientContext);

static ContextAndSpan CreateSpanFromContext(
ContextAndSpan CreateSpan(
std::string const& spanName,
Azure::Core::Tracing::_internal::SpanKind const& spanKind,
Azure::Core::Tracing::_internal::CreateSpanOptions& spanOptions,
Azure::Core::Context const& clientContext);

std::unique_ptr<Azure::Core::Tracing::_internal::AttributeSet> CreateAttributeSet();

static std::unique_ptr<DiagnosticTracingFactory> DiagnosticFactoryFromContext(
Azure::Core::Context const& context);
};

/**
Expand Down
98 changes: 54 additions & 44 deletions sdk/core/azure-core/src/http/request_activity_policy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,68 +23,78 @@ std::unique_ptr<RawResponse> RequestActivityPolicy::Send(
NextHttpPolicy nextPolicy,
Context const& context) const
{
// Create a tracing span over the HTTP request.
std::stringstream ss;
// 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)
// Find a tracing factory from our context. Note that the factory value is owned by the
// context chain so we can manage a raw pointer to the factory.
auto tracingFactory = DiagnosticTracingFactory::DiagnosticFactoryFromContext(context);
if (tracingFactory)
{
// 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);
auto scope = std::move(contextAndSpan.second);
// Create a tracing span over the HTTP request.
std::stringstream ss;
ss << "HTTP " << request.GetMethod().ToString();

scope.AddAttribute(TracingAttributes::HttpMethod.ToString(), request.GetMethod().ToString());
scope.AddAttribute("http.url", m_inputSanitizer.SanitizeUrl(request.GetUrl()).GetAbsoluteUrl());
{
CreateSpanOptions createOptions;
createOptions.Kind = SpanKind::Client;
createOptions.Attributes = tracingFactory->CreateAttributeSet();
// Note that the AttributeSet takes a *reference* to the values passed into the AttributeSet.
// This means that all the values passed into the AttributeSet MUST be stabilized across the
// lifetime of the AttributeSet.
std::string httpMethod = request.GetMethod().ToString();
createOptions.Attributes->AddAttribute(TracingAttributes::HttpMethod.ToString(), httpMethod);

std::string sanitizedUrl = m_inputSanitizer.SanitizeUrl(request.GetUrl()).GetAbsoluteUrl();
createOptions.Attributes->AddAttribute("http.url", sanitizedUrl);
Azure::Nullable<std::string> requestId = request.GetHeader("x-ms-client-request-id");
if (requestId.HasValue())
{
scope.AddAttribute(TracingAttributes::RequestId.ToString(), requestId.Value());
createOptions.Attributes->AddAttribute(
TracingAttributes::RequestId.ToString(), requestId.Value());
}
}

{
auto userAgent = request.GetHeader("User-Agent");
if (userAgent.HasValue())
{
scope.AddAttribute(TracingAttributes::HttpUserAgent.ToString(), userAgent.Value());
createOptions.Attributes->AddAttribute(
TracingAttributes::HttpUserAgent.ToString(), userAgent.Value());
}
}

// Propagate information from the scope to the HTTP headers.
//
// This will add the "traceparent" header and any other OpenTelemetry related headers.
scope.PropagateToHttpHeaders(request);
auto contextAndSpan = tracingFactory->CreateSpan(ss.str(), createOptions, context);
auto scope = std::move(contextAndSpan.second);

try
{
// Send the request on to the service.
auto response = nextPolicy.Send(request, contextAndSpan.first);
// Propagate information from the scope to the HTTP headers.
//
// This will add the "traceparent" header and any other OpenTelemetry related headers.
scope.PropagateToHttpHeaders(request);

// And register the headers we received from the service.
scope.AddAttribute(
TracingAttributes::HttpStatusCode.ToString(),
std::to_string(static_cast<int>(response->GetStatusCode())));
auto const& responseHeaders = response->GetHeaders();
auto serviceRequestId = responseHeaders.find("x-ms-request-id");
if (serviceRequestId != responseHeaders.end())
try
{
scope.AddAttribute(TracingAttributes::ServiceRequestId.ToString(), serviceRequestId->second);
// 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<int>(response->GetStatusCode())));
auto const& responseHeaders = response->GetHeaders();
auto serviceRequestId = responseHeaders.find("x-ms-request-id");
if (serviceRequestId != responseHeaders.end())
{
scope.AddAttribute(
TracingAttributes::ServiceRequestId.ToString(), serviceRequestId->second);
}

return response;
}
catch (const TransportException& e)
{
scope.AddEvent(e);
scope.SetStatus(SpanStatus::Error);

return response;
// Rethrow the exception.
throw;
}
}
catch (const TransportException& e)
else
{
scope.AddEvent(e);

// Rethrow the exception.
throw;
return nextPolicy.Send(request, context);
}
}
70 changes: 32 additions & 38 deletions sdk/core/azure-core/src/tracing/tracing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,26 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal {
Azure::Core::Tracing::_internal::SpanKind const& spanKind,
Azure::Core::Context const& context)
{
CreateSpanOptions createOptions;
if (m_serviceTracer)
{
Azure::Core::Context contextToUse = context;
CreateSpanOptions createOptions;

createOptions.Kind = spanKind;
createOptions.Attributes = m_serviceTracer->CreateAttributeSet();
return CreateSpan(methodName, createOptions, context);
}
else
{
return std::make_pair(context, ServiceSpan{});
}
}

DiagnosticTracingFactory::ContextAndSpan DiagnosticTracingFactory::CreateSpan(
std::string const& methodName,
Azure::Core::Tracing::_internal::CreateSpanOptions& createOptions,
Azure::Core::Context const& context)
{
if (m_serviceTracer)
{
Azure::Core::Context contextToUse = context;
Expand All @@ -50,12 +69,14 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal {
// span if there is no parent span in the context
createOptions.ParentSpan = nullptr;
}
createOptions.Attributes = m_serviceTracer->CreateAttributeSet();

if (!createOptions.Attributes)
{
createOptions.Attributes = m_serviceTracer->CreateAttributeSet();
}
createOptions.Attributes->AddAttribute(
TracingAttributes::AzNamespace.ToString(), m_serviceName);

createOptions.Kind = spanKind;

std::shared_ptr<Span> newSpan(m_serviceTracer->CreateSpan(methodName, createOptions));
TracingContext tracingContext = newSpan;
Azure::Core::Context newContext = contextToUse.WithValue(ContextSpanKey, tracingContext);
Expand All @@ -68,44 +89,14 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal {
return std::make_pair(context, ServiceSpan{});
}
}
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, spanKind, context);
}
else
{
return std::make_pair(context, ServiceSpan{});
}
}

Azure::Nullable<DiagnosticTracingFactory::TracingContext>
DiagnosticTracingFactory::TracingContextFromContext(Azure::Core::Context const& context)
{
TracingContext traceContext;
if (context.TryGetValue(ContextSpanKey, traceContext))
{
return traceContext;
}
else
{
return Azure::Nullable<TracingContext>{};
}
}

DiagnosticTracingFactory* DiagnosticTracingFactory::DiagnosticFactoryFromContext(
std::unique_ptr<DiagnosticTracingFactory> DiagnosticTracingFactory::DiagnosticFactoryFromContext(
Azure::Core::Context const& context)
{
DiagnosticTracingFactory* factory;
if (context.TryGetValue(TracingFactoryContextKey, factory))
{
return factory;
return std::make_unique<DiagnosticTracingFactory>(*factory);
}
else
{
Expand All @@ -116,10 +107,13 @@ namespace Azure { namespace Core { namespace Tracing { namespace _internal {
std::unique_ptr<Azure::Core::Tracing::_internal::AttributeSet>
DiagnosticTracingFactory::CreateAttributeSet()
{
return m_serviceTracer->CreateAttributeSet();
if (m_serviceTracer)
{
return m_serviceTracer->CreateAttributeSet();
}
return nullptr;
}

Azure::Core::Context::Key DiagnosticTracingFactory::ContextSpanKey;
Azure::Core::Context::Key DiagnosticTracingFactory::TracingFactoryContextKey;

}}}} // namespace Azure::Core::Tracing::_internal
Loading

0 comments on commit ebe084b

Please sign in to comment.