Set span status according to Semantic Conventions of Http (#164) (#169)

* Set span status according to http semantics

* Set span status changes part-2

* Set span status according to http semantics spaces

* Review comments addressed for span status

* Moved error status from Apache hooks to Request processing engine

Co-authored-by: Narasimha <ngonapa@cisco.com>

instrumentation/otel-webserver-module/include/core/sdkwrapper/IScopedSpan.h

@@ -17,10 +17,12 @@
 #ifndef __ISCOPEDSPAN_H
 #define __ISCOPEDSPAN_H
 
+#include "sdkwrapper/SdkEnums.h"
 #include <opentelemetry/common/attribute_value.h>
 #include <unordered_map>
 #include <chrono>
 
+
 namespace appd {
 namespace core {
 namespace sdkwrapper {
@@ -29,11 +31,6 @@ using SpanAttributeValue = opentelemetry::common::AttributeValue;
 using OtelKeyValueMap = std::unordered_map<std::string, SpanAttributeValue>;
 using namespace opentelemetry;
 
-enum class StatusCode {
-  Ok,
-  Error
-};
-
 class IScopedSpan {
 public:
 	virtual ~IScopedSpan() = default;
@@ -45,9 +42,11 @@ class IScopedSpan {
         const OtelKeyValueMap& attributes) = 0;
 
 	virtual void AddAttribute(const std::string& key,
-		const SpanAttributeValue& value) = 0;
+	const SpanAttributeValue& value) = 0;
+
+        virtual void SetStatus(const StatusCode status, const std::string& desc = "") = 0;
 
-  virtual void SetStatus(const StatusCode status, const std::string& desc = "") = 0;
+        virtual SpanKind GetSpanKind() = 0;
 };
 
 } //sdkwrapper

instrumentation/otel-webserver-module/include/core/sdkwrapper/ISdkWrapper.h

@@ -19,19 +19,14 @@
 
 #include <unordered_map>
 #include <memory>
+#include "sdkwrapper/SdkEnums.h"
 #include "sdkwrapper/IScopedSpan.h"
 #include "api/TenantConfig.h"
 
 namespace appd {
 namespace core {
 namespace sdkwrapper {
 
-enum class SpanKind {
-	INTERNAL,
-  	SERVER,
-  	CLIENT
-};
-
 class ISdkWrapper {
 public:
 	virtual ~ISdkWrapper() = default;

instrumentation/otel-webserver-module/include/core/sdkwrapper/ScopedSpan.h

@@ -46,12 +46,15 @@ class ScopedSpan : public IScopedSpan {
 	void AddAttribute(const std::string& key,
         const SpanAttributeValue& value) override;
 
-  void SetStatus(const StatusCode status, const std::string& desc) override;
+    void SetStatus(const StatusCode status, const std::string& desc) override;
+
+    SpanKind GetSpanKind();
 
 private:
 	opentelemetry::nostd::shared_ptr<trace::Span> mSpan;
 	std::unique_ptr<trace::Scope> mScope;
-  const AgentLogger& mLogger;
+    const AgentLogger& mLogger;
+    trace::SpanKind mSpanKind;
 };
 
 } //sdkwrapper

instrumentation/otel-webserver-module/include/core/sdkwrapper/SdkConstants.h

@@ -24,6 +24,11 @@ const std::string kServiceNamespace = "service.namespace";
 const std::string kServiceInstanceId = "service.instance.id";
 const std::string kOtelLibraryName = "telemetry.sdk.language";
 const std::string kOtelLibraryVersion = "telemetry.sdk.version";
+const std::string kHttpErrorCode = "HTTP ERROR CODE:";
+constexpr int HTTP_ERROR_1XX = 100;
+constexpr int HTTP_ERROR_4XX = 400;
+constexpr int HTTP_ERROR_5XX = 500;
+
 } // sdkwrapper
 } // core
 } // appd

instrumentation/otel-webserver-module/include/core/sdkwrapper/SdkEnums.h

@@ -0,0 +1,37 @@
+/*
+* Copyright 2021 AppDynamics LLC. 
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+*
+*      http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+
+#pragma once
+
+namespace appd {
+namespace core {
+namespace sdkwrapper {
+
+enum class StatusCode {
+  Ok,
+  Error,
+  Unset
+};
+
+enum class SpanKind {
+  INTERNAL,
+  SERVER,
+  CLIENT
+};
+
+} // sdkwrapper
+} // core
+} // appd

instrumentation/otel-webserver-module/include/core/sdkwrapper/ServerSpan.h

@@ -51,6 +51,8 @@ class ServerSpan : public IScopedSpan {
 
   void SetStatus(const StatusCode status, const std::string& desc) override;
 
+  SpanKind GetSpanKind();
+
 private:
 	std::unique_ptr<ScopedSpan> mScopedSpan;
     nostd::unique_ptr<context::Token> mToken;

instrumentation/otel-webserver-module/src/apache/ApacheHooks.cpp

@@ -634,7 +634,7 @@ int ApacheHooks::appd_hook_log_transaction_end(request_rec* r)
     if (appd_requestHasErrors(r))
     {
         std::ostringstream oss;
-        oss << "HTTP ERROR CODE:" << r->status;
+        oss << r->status;
         res = wsAgent.endRequest(reqHandle, oss.str().c_str());
     }
     else

instrumentation/otel-webserver-module/src/core/api/RequestProcessingEngine.cpp

@@ -19,17 +19,22 @@
 #include "api/ApiUtils.h"
 #include "api/Payload.h"
 #include "sdkwrapper/SdkWrapper.h"
+#include "sdkwrapper/IScopedSpan.h"
+#include "sdkwrapper/SdkConstants.h"
 #include <sys/types.h>
 #include <unistd.h>
 #include <boost/filesystem.hpp>
 #include <boost/thread/locks.hpp>
 #include <log4cxx/logger.h>
 #include <unordered_map>
+#include <sstream>
 
 
 namespace appd {
 namespace core {
 
+using namespace sdkwrapper;
+
 RequestProcessingEngine::RequestProcessingEngine()
     : mLogger(getLogger(std::string(LogContext::AGENT) + ".RequestProcessingEngine"))
 {
@@ -99,10 +104,33 @@ APPD_SDK_STATUS_CODE RequestProcessingEngine::endRequest(
 
     // check for error and set attribute in the scopedSpan.
     if (error) {
-        rootSpan->SetStatus(sdkwrapper::StatusCode::Error, error);
-        LOG4CXX_TRACE(mLogger, "Setting status as error[" << error <<"] on root Span");
+        std::string errorStatus;
+        std::stringstream strValue;
+        unsigned int errorValue;
+
+        strValue << error;
+        strValue >> errorValue;
+
+        strValue << kHttpErrorCode + error; // This is status message eg: HTTP ERROR CODE:403
+        strValue >> errorStatus;
+
+        if (errorValue >= HTTP_ERROR_1XX &&   errorValue < HTTP_ERROR_4XX ) {
+            rootSpan->SetStatus(StatusCode::Unset);
+        }
+        else if (errorValue >= HTTP_ERROR_4XX &&   errorValue < HTTP_ERROR_5XX ) {
+            if (rootSpan->GetSpanKind() == SpanKind::SERVER)
+                rootSpan->SetStatus(StatusCode::Unset);
+            else
+                rootSpan->SetStatus(StatusCode::Error, errorStatus);
+
+        } else {
+            rootSpan->SetStatus(StatusCode::Error, errorStatus);
+        }
+
+        LOG4CXX_TRACE(mLogger, "Setting status as error[" << errorStatus <<"] on root Span");
+
     } else {
-        rootSpan->SetStatus(sdkwrapper::StatusCode::Ok);
+        rootSpan->SetStatus(StatusCode::Ok);
     }
 
     LOG4CXX_TRACE(mLogger, "Ending root span with id: " << rootSpan.get());
@@ -137,7 +165,7 @@ APPD_SDK_STATUS_CODE RequestProcessingEngine::startInteraction(
     // TODO : confirm and update name later
     std::string spanName = payload->moduleName + "_" + payload->phaseName;
     keyValueMap["interactionType"] = "EXIT_CALL";
-    auto interactionSpan = m_sdkWrapper->CreateSpan(spanName, sdkwrapper::SpanKind::CLIENT, keyValueMap);
+    auto interactionSpan = m_sdkWrapper->CreateSpan(spanName, SpanKind::CLIENT, keyValueMap);
     LOG4CXX_TRACE(mLogger, "Client Span started with SpanName: " << spanName
         << " Span Id: " << interactionSpan.get());
     m_sdkWrapper->PopulatePropagationHeaders(propagationHeaders);
@@ -177,11 +205,22 @@ APPD_SDK_API APPD_SDK_STATUS_CODE RequestProcessingEngine::endInteraction(
     // If errorCode is 0 or errMsg is empty, there is no error.
     bool isError = payload->errorCode != 0 && !payload->errorMsg.empty();
     if (isError) {
-        interactionSpan->SetStatus(sdkwrapper::StatusCode::Error, payload->errorMsg);
+        if (payload->errorCode >= HTTP_ERROR_1XX &&   payload->errorCode < HTTP_ERROR_4XX ) {
+            interactionSpan->SetStatus(StatusCode::Unset);
+        }
+        else if (payload->errorCode >= HTTP_ERROR_4XX &&   payload->errorCode < HTTP_ERROR_5XX ) {
+            if (interactionSpan->GetSpanKind() == SpanKind::SERVER)
+                interactionSpan->SetStatus(StatusCode::Unset);
+            else
+                interactionSpan->SetStatus(StatusCode::Error, payload->errorMsg);
+
+        } else {
+            interactionSpan->SetStatus(StatusCode::Error, payload->errorMsg);
+        }
         interactionSpan->AddAttribute("error_code", payload->errorCode);
         LOG4CXX_TRACE(mLogger, "Span updated with error Code: " << payload->errorCode);
     } else {
-        interactionSpan->SetStatus(sdkwrapper::StatusCode::Ok);
+        interactionSpan->SetStatus(StatusCode::Ok);
     }
 
     if (!payload->backendName.empty()) {

instrumentation/otel-webserver-module/src/core/sdkwrapper/ScopedSpan.cpp

@@ -32,6 +32,7 @@ ScopedSpan::ScopedSpan(
 	options.kind = kind;
 	mSpan = sdkHelperFactory->GetTracer()->StartSpan(name, attributes, options);
 	mScope.reset(new trace::Scope(mSpan));
+	mSpanKind = kind;
 }
 
 void ScopedSpan::End()
@@ -68,6 +69,16 @@ void ScopedSpan::SetStatus(const StatusCode status, const std::string& desc)
   mSpan->SetStatus(otelStatus, desc);
 }
 
+SpanKind ScopedSpan::GetSpanKind()
+{
+	if (mSpanKind == trace::SpanKind::kServer)
+		return SpanKind::SERVER;
+	else if (mSpanKind == trace::SpanKind::kClient)
+		return SpanKind::CLIENT;
+	else
+		return SpanKind::INTERNAL;
+}
+
 } //sdkwrapper
 } //core
 } //appd

instrumentation/otel-webserver-module/src/core/sdkwrapper/ServerSpan.cpp

@@ -68,6 +68,12 @@ void ServerSpan::SetStatus(const StatusCode status, const std::string& desc)
   mScopedSpan->SetStatus(status, desc);
 }
 
+SpanKind ServerSpan::GetSpanKind()
+{
+  return SpanKind::SERVER;
+}
+
+
 } //sdkwrapper
 } //core
 } //appd