Adding missing root span attributes and a Directive to configure root…

… span name (operation name) (open-telemetry#428)

* Adding missing span attributes and OperationName Directive to name spans

* making changes in tests

* modifying tests 2

* modifying tests 3

* modify tests

* tests passed

* indentation fix

instrumentation/otel-webserver-module/README.md

@@ -180,6 +180,7 @@ Currently, Nginx Webserver module monitores some fixed set of modules, which get
 |*NginxModuleAttributes*           |                | OPTIONAL: Can be used to pass additionalccustom attributes to the span, nginx variables are also supported. All elements must be separated by a space, and different attribute key value pairs must be separated by a comma( even the comma needs to be separated by space). e.g. ```NginxModuleAttributes     Key1 Value1 , Key2 $request_uri;``` |
 |*NginxModuleIgnorePaths*           |               | OPTIONAL: Request URIs matching the Regex will not be monitored. Multiple space separated Regex can be provided( `'\'` symbol needs to be used carefully, Nginx treats `'\'` as a escape sequence, thus if the Regex sequence contains a `'\'`, it need to be replaced by `'\\'`, likewise if sequence contains `'\\'`, it need to be written as `'\\\\'` e.g. `.*\.html` -> `.*\\.html` ) e.g. ```NginxModuleIgnorePaths               .*\\.html /test_.*;```|
 |*NginxModulePropagatorType*                   | w3c                 | OPTIONAL: Specify the Propagator used by the instrumentation (W3C and B3 propagators available). e.g.```NginxModulePropagatorType                  b3;```|
+|*NginxModuleOperationName*                   |                 | OPTIONAL: Specify the operation name (span name) for any specific endpoint.  e.g.```NginxModuleOperationName                  My_Backend;```|
 
 
 

instrumentation/otel-webserver-module/include/core/api/Payload.h

@@ -46,6 +46,9 @@ class RequestPayload
     std::string target;
     std::string flavor;
     std::string client_ip;
+	std::string user_agent;
+	std::string operation_name;
+	long peer_port;
     long port = 80;
 
 public:
@@ -68,6 +71,10 @@ class RequestPayload
     void set_target(const char* aTarget) {target = aTarget; }
     void set_flavor(const char* aflavor) {flavor = aflavor; }
     void set_client_ip(const char* clientIp) {client_ip = clientIp; }
+	void set_user_agent(const char* userAgent) {user_agent = userAgent; }
+	void set_operation_name(const char* operationName) {operation_name = operationName; }
+
+	void set_peer_port(long aPeerPort) {peer_port = aPeerPort; }
     void set_port(long aPort) {port = aPort; }
 
 
@@ -83,6 +90,9 @@ class RequestPayload
     std::string& get_target() {return target; }
     std::string& get_flavor() {return flavor; }
     std::string& get_client_ip() {return client_ip; }
+	std::string& get_user_agent() {return user_agent; }
+	std::string& get_operation_name() {return operation_name; }
+	long get_peer_port() {return peer_port; }
     long get_port() {return port; }
     std::unordered_map<std::string, std::string>& get_request_headers() {
     	return request_headers;

instrumentation/otel-webserver-module/include/core/api/opentelemetry_ngx_api.h

@@ -45,12 +45,15 @@ typedef struct {
     const char* http_post_param;
     const char* request_method;
     const char* client_ip;
+    const char* user_agent;
+    const char* operation_name;
 
     http_headers* propagation_headers;
     http_headers* request_headers;
 
     int propagation_count;
     int request_headers_count;
+    int peer_port;
 }request_payload;
 
 typedef struct {

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

@@ -35,12 +35,13 @@ const std::string kAttrHTTPUrl                = "http.url";
 const std::string kAttrHTTPFlavor             = "http.flavor";
 const std::string kAttrHTTPClientIP           = "http.client_ip";
 const std::string kAttrHTTPStatusCode         = "http.status_code";
-const std::string kAttrNETHostPort            = "net.host.port";
+const std::string kAttrNETHostPort            = "net.server.port";
 const std::string kAttrRequestProtocol        = "request_protocol";
-const std::string kHTTPFlavor1_0                 = "1.0";
-const std::string kHTTPFlavor1_1                = "1.1";
+const std::string kHTTPFlavor1_0              = "1.0";
+const std::string kHTTPFlavor1_1              = "1.1";
 const std::string kOtelAttributes             = "otel.attribute.";
-
+const std::string kAttrHTTPUserAgent          = "http.user_agent";
+const std::string kAttrNETPeerPort            = "net.remote.port";
 
 constexpr int HTTP_ERROR_1XX = 100;
 constexpr int HTTP_ERROR_4XX = 400;

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

@@ -64,7 +64,7 @@ OTEL_SDK_STATUS_CODE RequestProcessingEngine::startRequest(
         return OTEL_STATUS(payload_reflector_is_null);
     }
 
-    std::string spanName = m_spanNamer->getSpanName(payload->get_uri());
+    std::string spanName = (payload->get_operation_name().empty() ? m_spanNamer->getSpanName(payload->get_uri()) : payload->get_operation_name());
     otel::core::sdkwrapper::OtelKeyValueMap keyValueMap;
     keyValueMap[kAttrRequestProtocol] = payload->get_request_protocol();
     keyValueMap[kAttrHTTPServerName] = payload->get_server_name();
@@ -75,6 +75,8 @@ OTEL_SDK_STATUS_CODE RequestProcessingEngine::startRequest(
     keyValueMap[kAttrHTTPTarget] =payload->get_target();
     keyValueMap[kAttrHTTPFlavor] = payload->get_flavor();
     keyValueMap[kAttrHTTPClientIP] = payload->get_client_ip();
+    keyValueMap[kAttrHTTPUserAgent] = payload->get_user_agent();
+    keyValueMap[kAttrNETPeerPort] = payload->get_peer_port();
 
     auto& request_headers = payload->get_request_headers();
     for (auto itr = request_headers.begin(); itr != request_headers.end(); itr++) {

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

@@ -48,6 +48,10 @@ void populatePayload(request_payload* req_payload, void* load)
     payload->set_http_get_parameter(req_payload->http_get_param);
     payload->set_http_request_method(req_payload->request_method);
     payload->set_client_ip(req_payload->client_ip);
+    payload->set_user_agent(req_payload->user_agent);
+    payload->set_peer_port(req_payload->peer_port);
+    payload->set_operation_name(req_payload->operation_name);
+
 
     for(int i=0; i<req_payload->propagation_count; i++){
 

instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.c

@@ -34,15 +34,15 @@ otel_ngx_module otel_monitored_modules[] = {
     {
         "ngx_http_realip_module",
         0,
-        {NGX_HTTP_POST_READ_PHASE, NGX_HTTP_PREACCESS_PHASE},
+        {NGX_HTTP_PREACCESS_PHASE},
         ngx_http_otel_realip_handler,
         0,
         2
     },
     {
         "ngx_http_rewrite_module",
         0,
-        {NGX_HTTP_SERVER_REWRITE_PHASE, NGX_HTTP_REWRITE_PHASE},
+        {NGX_HTTP_REWRITE_PHASE},
         ngx_http_otel_rewrite_handler,
         0,
         2
@@ -406,6 +406,13 @@ static ngx_command_t ngx_http_opentelemetry_commands[] = {
       offsetof(ngx_http_opentelemetry_loc_conf_t, nginxModulePropagatorType),
       NULL},
 
+    { ngx_string("NginxModuleOperationName"),
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
+      ngx_conf_set_str_slot,
+      NGX_HTTP_LOC_CONF_OFFSET,
+      offsetof(ngx_http_opentelemetry_loc_conf_t, nginxModuleOperationName),
+      NULL},
+
     /* command termination */
     ngx_null_command
 };
@@ -516,7 +523,7 @@ static char* ngx_http_opentelemetry_merge_loc_conf(ngx_conf_t *cf, void *parent,
     ngx_conf_merge_str_value(conf->nginxModuleRequestHeaders, prev->nginxModuleRequestHeaders, "");
     ngx_conf_merge_str_value(conf->nginxModuleResponseHeaders, prev->nginxModuleResponseHeaders, "");
     ngx_conf_merge_str_value(conf->nginxModulePropagatorType, prev->nginxModulePropagatorType, "w3c");
-
+    ngx_conf_merge_str_value(conf->nginxModuleOperationName, prev->nginxModuleOperationName, "");
 
     return NGX_CONF_OK;
 }
@@ -1049,7 +1056,7 @@ static void resolve_attributes_variables(ngx_http_request_t* r)
             } else {
                 ngx_str_t * ngx_str = (ngx_str_t *)(element);
                 ngx_str->data = value->data;
-                ngx_str->len = ngx_strlen(value);
+                ngx_str->len = value->len;
                 // Variable was found, `value` now contains the value.
             }
         }
@@ -1435,6 +1442,14 @@ static void startMonitoringRequest(ngx_http_request_t* r){
 }
 
 static ngx_int_t ngx_http_otel_rewrite_handler(ngx_http_request_t *r){
+
+    // This will be the first hanndler to be encountered,
+    // Here, Init and start the Request Processing by creating Trace, spans etc
+    if(!r->internal)
+    {
+        startMonitoringRequest(r);
+    }
+
     otel_startInteraction(r, "ngx_http_rewrite_module");
     ngx_int_t rvalue = h[NGX_HTTP_REWRITE_MODULE_INDEX](r);
     otel_stopInteraction(r, "ngx_http_rewrite_module", OTEL_SDK_NO_HANDLE);
@@ -1459,14 +1474,6 @@ static ngx_int_t ngx_http_otel_limit_req_handler(ngx_http_request_t *r){
 }
 
 static ngx_int_t ngx_http_otel_realip_handler(ngx_http_request_t *r){
-
-    // This will be the first hanndler to be encountered,
-    // Here, Init and start the Request Processing by creating Trace, spans etc
-    if(!r->internal)
-    {
-        startMonitoringRequest(r);
-    }
-
     otel_startInteraction(r, "ngx_http_realip_module");
     ngx_int_t rvalue = h[NGX_HTTP_REALIP_MODULE_INDEX](r);
     otel_stopInteraction(r, "ngx_http_realip_module", OTEL_SDK_NO_HANDLE);    
@@ -1797,6 +1804,24 @@ static void fillRequestPayload(request_payload* req_payload, ngx_http_request_t*
     temp_request_method[(r->method_name).len]='\0';
     req_payload->request_method = temp_request_method;
 
+    char *temp_user_agent = ngx_pcalloc(r->pool, r->headers_in.user_agent->value.len +1);
+    strcpy(temp_user_agent,(const char*)(r->headers_in.user_agent->value.data));
+    temp_user_agent[r->headers_in.user_agent->value.len]='\0';
+    req_payload->user_agent = temp_user_agent;
+
+    ngx_uint_t remote_port = 0;
+    if (r->connection != NULL) {
+        struct sockaddr *temp_sockaddress = r->connection->sockaddr;
+        if(temp_sockaddress->sa_family == 2) {
+                struct sockaddr_in *temp_soc = (struct sockaddr_in *) temp_sockaddress;
+                remote_port = ntohs(temp_soc->sin_port);
+        }
+        else if(temp_sockaddress->sa_family == 10 || temp_sockaddress->sa_family == 23){
+                struct sockaddr_in6 *temp_soc6 = (struct sockaddr_in6 *) temp_sockaddress;
+                remote_port = ntohs(temp_soc6->sin6_port);
+        }
+    }
+    req_payload->peer_port = remote_port;
     // flavor has to be scraped from protocol in future
     req_payload->flavor = temp_http_protocol;
 
@@ -1833,6 +1858,8 @@ static void fillRequestPayload(request_payload* req_payload, ngx_http_request_t*
     ngx_http_opentelemetry_loc_conf_t *conf =
       ngx_http_get_module_loc_conf(r, ngx_http_opentelemetry_module);
 
+    req_payload->operation_name = (const char*)conf->nginxModuleOperationName.data;
+
     part = &r->headers_in.headers.part;
     header = (ngx_table_elt_t*)part->elts;
     nelts = part->nelts;
@@ -1896,7 +1923,12 @@ static void fillResponsePayload(response_payload* res_payload, ngx_http_request_
 
         resolve_attributes_variables(r);
         for (ngx_uint_t j = 0, isKey = 1, isValue = 0; j < conf->nginxModuleAttributes->nelts; j++) {
-            const char* data = (const char*)(((ngx_str_t *)(conf->nginxModuleAttributes->elts))[j]).data;
+
+            ngx_str_t data_obj = ((ngx_str_t *)(conf->nginxModuleAttributes->elts))[j];
+            char* data = ngx_pcalloc(r->pool, data_obj.len +1);
+            ngx_memcpy(data, (const char*)(data_obj.data) , data_obj.len);
+            data[data_obj.len] = '\0';
+
             if(strcmp(data, ",") == 0){
                 otel_attributes_idx++;
                 continue;

instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.h

@@ -107,6 +107,7 @@ typedef struct {
     ngx_array_t  *nginxModuleAttributes;
     ngx_array_t  *nginxModuleIgnorePaths;
     ngx_str_t nginxModulePropagatorType;
+    ngx_str_t nginxModuleOperationName;
 
 } ngx_http_opentelemetry_loc_conf_t;
 

instrumentation/otel-webserver-module/test/unit/RequestProcessingEngine_test.cpp

@@ -82,22 +82,22 @@ MATCHER_P(HasMapVal, value, "") {
     		if(nostd::get<int64_t>(argkeyValue) != nostd::get<int64_t>(keyValue)){
     			valueMatches = false;
     		}
-    }
-    else if (nostd::holds_alternative<nostd::string_view>(keyValue)){
-    		
-    		if(nostd::get<nostd::string_view>(argkeyValue) != nostd::get<nostd::string_view>(keyValue)){
-    			  valueMatches = false;	
-    		}	
-    }
-    else if (nostd::holds_alternative<int32_t>(keyValue)){
-    		if(nostd::get<int32_t>(argkeyValue) != nostd::get<int32_t>(keyValue))
-    			valueMatches = false;
-    }
-    else if (nostd::holds_alternative<bool>(keyValue))
-    {
-    		if(nostd::get<bool>(argkeyValue) != nostd::get<bool>(keyValue))
-    			valueMatches = false;
-    }
+		}
+		else if (nostd::holds_alternative<nostd::string_view>(keyValue)){
+				
+				if(nostd::get<nostd::string_view>(argkeyValue) != nostd::get<nostd::string_view>(keyValue)){
+					valueMatches = false;	
+				}	
+		}
+		else if (nostd::holds_alternative<int32_t>(keyValue)){
+				if(nostd::get<int32_t>(argkeyValue) != nostd::get<int32_t>(keyValue))
+					valueMatches = false;
+		}
+		else if (nostd::holds_alternative<bool>(keyValue))
+		{
+				if(nostd::get<bool>(argkeyValue) != nostd::get<bool>(keyValue))
+					valueMatches = false;
+		}
 	}
 
 	return valueMatches;
@@ -137,24 +137,28 @@ TEST(TestRequestProcessingEngine, StartRequest)
 	payload.set_uri("dummy_span");
 	payload.set_server_name("localhost");
 	payload.set_host("host");
-  payload.set_http_request_method("GET");
-  payload.set_scheme("http");
-  payload.set_target("target");
-  payload.set_flavor("1.1");
-  payload.set_client_ip("clientip");
-  payload.set_port(80);
+	payload.set_http_request_method("GET");
+	payload.set_scheme("http");
+	payload.set_target("target");
+	payload.set_flavor("1.1");
+	payload.set_client_ip("clientip");
+	payload.set_port(80);
+	payload.set_user_agent("useragent");
+	payload.set_peer_port(12345);
 
 
 	otel::core::sdkwrapper::OtelKeyValueMap keyValueMap;
-  keyValueMap[kAttrRequestProtocol] = (opentelemetry::nostd::string_view)"GET";
-  keyValueMap[kAttrHTTPServerName] = (opentelemetry::nostd::string_view)"localhost";
-  keyValueMap[kAttrHTTPMethod] = (opentelemetry::nostd::string_view)"GET";
-  keyValueMap[kAttrNetHostName] =(opentelemetry::nostd::string_view)"host";
-  keyValueMap[kAttrNETHostPort] = (long)80;
-  keyValueMap[kAttrHTTPScheme] = (opentelemetry::nostd::string_view)"http";
-  keyValueMap[kAttrHTTPTarget] = (opentelemetry::nostd::string_view)"target";
-  keyValueMap[kAttrHTTPFlavor] = (opentelemetry::nostd::string_view)"1.1";
-  keyValueMap[kAttrHTTPClientIP] = (opentelemetry::nostd::string_view)"clientip";
+	keyValueMap[kAttrRequestProtocol] = (opentelemetry::nostd::string_view)"GET";
+	keyValueMap[kAttrHTTPServerName] = (opentelemetry::nostd::string_view)"localhost";
+	keyValueMap[kAttrHTTPMethod] = (opentelemetry::nostd::string_view)"GET";
+	keyValueMap[kAttrNetHostName] =(opentelemetry::nostd::string_view)"host";
+	keyValueMap[kAttrNETHostPort] = (long)80;
+	keyValueMap[kAttrHTTPScheme] = (opentelemetry::nostd::string_view)"http";
+	keyValueMap[kAttrHTTPTarget] = (opentelemetry::nostd::string_view)"target";
+	keyValueMap[kAttrHTTPFlavor] = (opentelemetry::nostd::string_view)"1.1";
+	keyValueMap[kAttrHTTPClientIP] = (opentelemetry::nostd::string_view)"clientip";
+	keyValueMap[kAttrNETPeerPort] = (long)12345;
+	keyValueMap[kAttrHTTPUserAgent] = (opentelemetry::nostd::string_view)"useragent";
 	std::shared_ptr<otel::core::sdkwrapper::IScopedSpan> span;
 	span.reset(new MockScopedSpan);
 
@@ -163,7 +167,7 @@ TEST(TestRequestProcessingEngine, StartRequest)
 		otel::core::sdkwrapper::SpanKind::SERVER,
 		HasMapVal(keyValueMap),
 		payload.get_http_headers())).
-	WillOnce(Return(span));
+		WillOnce(Return(span));
 
 	int* dummy = new int(2);
 	void* reqHandle = dummy;
@@ -176,7 +180,7 @@ TEST(TestRequestProcessingEngine, StartRequest)
 	EXPECT_EQ(reqContext->getContextName(), "ws_context");
 	EXPECT_FALSE(reqContext->hasActiveInteraction());
 
-  delete(dummy);
+  	delete(dummy);
 }