add additional attributes for OTel compatibility

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## dev
 
-Version <dev> adds instrumentation for Async::HTTP, Ethon, and HTTPX, gleans Docker container IDs from cgroups v2-based containers, records additional synthetics attributes, fixes an issue with Rails 7.1 that could cause duplicate log records to be sent to New Relic, and fixes a deprecation warning for the Sidekiq error handler.
+Version <dev> adds instrumentation for Async::HTTP, Ethon, and HTTPX, adds the ability to ignore specific routes with Roda, gleans Docker container IDs from cgroups v2-based containers, records additional synthetics attributes, fixes an issue with Rails 7.1 that could cause duplicate log records to be sent to New Relic, fixes a deprecation warning for the Sidekiq error handler, and adds additional attributes for OpenTelemetry compatibility.
 
 - **Feature: Add instrumentation for Async::HTTP**
 
@@ -32,7 +32,7 @@ Version <dev> adds instrumentation for Async::HTTP, Ethon, and HTTPX, gleans Doc
 
   For compatibility with Ruby 3.4 and to silence compatibility warnings present in Ruby 3.3, declare a dependency on the `base64` gem. The New Relic Ruby agent uses the native Ruby `base64` gem for Base 64 encoding/decoding. The agent is joined by Ruby on Rails ([rails/rails@3e52adf](https://github.com/rails/rails/commit/3e52adf28e90af490f7e3bdc4bcc85618a4e0867)) and others in making this change in preparation for Ruby 3.3/3.4. [PR#2238](https://github.com/newrelic/newrelic-ruby-agent/pull/2238)
 
--**Feature: Add Roda support for the newrelic_ignore\* family of methods**
+- **Feature: Add Roda support for the newrelic_ignore\* family of methods**
 
   The agent can now selectively disable instrumentation for particular requests within Roda applications. Supported methods include:
   - `newrelic_ignore`: ignore a given route.
@@ -41,6 +41,22 @@ Version <dev> adds instrumentation for Async::HTTP, Ethon, and HTTPX, gleans Doc
 
   For more information, see [Roda Instrumentation](https://docs.newrelic.com/docs/apm/agents/ruby-agent/instrumented-gems/roda-instrumentation/). [PR#2267](https://github.com/newrelic/newrelic-ruby-agent/pull/2267)
 
+- **Feature: Add additional span attributes for OpenTelemetry compatibility**
+
+  For improved compatibility with OpenTelemetry's semantic conventions, the agent's datastore (for databases) and external request (for HTTP clients) segments have been updated with additional attributes.
+
+  Datastore segments now offer 3 additional attributes:
+  - `db.system`: The database system. For Ruby we use the database adapter name here.
+  - `server.address`: The database host.
+  - `server.port`: The database port.
+
+  External request segments now offer 3 additional attributes:
+  - `http.request.method`: The HTTP method (ex: 'GET')
+  - `server.address`: The target host.
+  - `server.port`: The target port.
+
+  For maximum backwards compatibility, no existing attributes have been renamed or removed. [PR#2283](https://github.com/newrelic/newrelic-ruby-agent/pull/2283)
+
 - **Bugfix: Stop sending duplicate log events for Rails 7.1 users**
 
   Rails 7.1 introduced the public API [`ActiveSupport::BroadcastLogger`](https://api.rubyonrails.org/classes/ActiveSupport/BroadcastLogger.html). This logger replaces a private API, `ActiveSupport::Logger.broadcast`. In Rails versions below 7.1, the agent uses the `broadcast` method to stop duplicate logs from being recoded by broadcasted loggers. Now, we've updated the code to provide a similar duplication fix for the `ActiveSupport::BroadcastLogger` class. [PR#2252](https://github.com/newrelic/newrelic-ruby-agent/pull/2252)

lib/new_relic/agent/span_event_primitive.rb

@@ -29,12 +29,16 @@ module SpanEventPrimitive
       CATEGORY_KEY = 'category'
       HTTP_URL_KEY = 'http.url'
       HTTP_METHOD_KEY = 'http.method'
+      HTTP_REQUEST_METHOD_KEY = 'http.request.method'
       HTTP_STATUS_CODE_KEY = 'http.statusCode'
       COMPONENT_KEY = 'component'
       DB_INSTANCE_KEY = 'db.instance'
       DB_STATEMENT_KEY = 'db.statement'
+      DB_SYSTEM_KEY = 'db.system'
       PEER_ADDRESS_KEY = 'peer.address'
       PEER_HOSTNAME_KEY = 'peer.hostname'
+      SERVER_ADDRESS_KEY = 'server.address'
+      SERVER_PORT_KEY = 'server.port'
       SPAN_KIND_KEY = 'span.kind'
       ENTRY_POINT_KEY = 'nr.entryPoint'
       TRUSTED_PARENT_KEY = 'trustedParentId'
@@ -69,10 +73,12 @@ def for_external_request_segment(segment)
 
         intrinsics[COMPONENT_KEY] = segment.library
         intrinsics[HTTP_METHOD_KEY] = segment.procedure
+        intrinsics[HTTP_REQUEST_METHOD_KEY] = segment.procedure
         intrinsics[HTTP_STATUS_CODE_KEY] = segment.http_status_code if segment.http_status_code
         intrinsics[CATEGORY_KEY] = HTTP_CATEGORY
         intrinsics[SPAN_KIND_KEY] = CLIENT
-
+        intrinsics[SERVER_ADDRESS_KEY] = segment.uri.host
+        intrinsics[SERVER_PORT_KEY] = segment.uri.port
         agent_attributes = error_attributes(segment) || {}
 
         if allowed?(HTTP_URL_KEY)
@@ -86,7 +92,7 @@ def for_external_request_segment(segment)
         [intrinsics, custom_attributes(segment), agent_attributes]
       end
 
-      def for_datastore_segment(segment)
+      def for_datastore_segment(segment) # rubocop:disable Metrics/AbcSize
         intrinsics = intrinsics_for(segment)
 
         intrinsics[COMPONENT_KEY] = segment.product
@@ -101,9 +107,15 @@ def for_datastore_segment(segment)
         if segment.host && segment.port_path_or_id && allowed?(PEER_ADDRESS_KEY)
           agent_attributes[PEER_ADDRESS_KEY] = truncate("#{segment.host}:#{segment.port_path_or_id}")
         end
-        if segment.host && allowed?(PEER_HOSTNAME_KEY)
-          agent_attributes[PEER_HOSTNAME_KEY] = truncate(segment.host)
+        if segment.host
+          [PEER_HOSTNAME_KEY, SERVER_ADDRESS_KEY].each do |key|
+            agent_attributes[key] = truncate(segment.host) if allowed?(key)
+          end
+        end
+        if segment.port_path_or_id&.match?(/^\d+$/) && allowed?(SERVER_PORT_KEY)
+          agent_attributes[SERVER_PORT_KEY] = segment.port_path_or_id
         end
+        agent_attributes[DB_SYSTEM_KEY] = segment.product if allowed?(DB_SYSTEM_KEY)
 
         if segment.sql_statement && allowed?(DB_STATEMENT_KEY)
           agent_attributes[DB_STATEMENT_KEY] = truncate(segment.sql_statement.safe_sql, 2000)

test/new_relic/agent/span_event_primitive_test.rb

@@ -2,6 +2,7 @@
 # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
 # frozen_string_literal: true
 
+require 'uri'
 require_relative '../../test_helper'
 
 module NewRelic
@@ -325,18 +326,24 @@ def test_agent_attributes_are_not_included_for_external_segments_by_default
                 segment = MiniTest::Mock.new
                 segment.expect(:library, :library)
                 segment.expect(:procedure, :procedure)
+                segment.expect(:procedure, :procedure) # 2 calls
                 segment.expect(:http_status_code, :http_status_code)
                 segment.expect(:http_status_code, :http_status_code) # 2 calls
-                segment.expect(:uri, :uri)
+                segment.expect(:uri, uri_for_testing)
+                segment.expect(:uri, uri_for_testing)
+                segment.expect(:uri, uri_for_testing) # 3 calls
                 segment.expect(:record_agent_attributes?, false)
                 result = SpanEventPrimitive.for_external_request_segment(segment)
                 expected_intrinsics = {'component' => :library,
                                        'http.method' => :procedure,
+                                       'http.request.method' => :procedure,
                                        'http.statusCode' => :http_status_code,
                                        'category' => 'http',
-                                       'span.kind' => 'client'}
+                                       'span.kind' => 'client',
+                                       'server.address' => 'newrelic.com',
+                                       'server.port' => 443}
                 expected_custom_attrs = {}
-                expected_agent_attrs = {'http.url' => 'uri'}
+                expected_agent_attrs = {'http.url' => uri_for_testing.to_s}
 
                 assert_equal [expected_intrinsics, expected_custom_attrs, expected_agent_attrs], result
               end
@@ -353,25 +360,37 @@ def test_agent_attributes_are_included_for_external_segments_when_enabled_on_a_p
                   segment = MiniTest::Mock.new
                   segment.expect(:library, :library)
                   segment.expect(:procedure, :procedure)
+                  segment.expect(:procedure, :procedure) # 2 calls
                   segment.expect(:http_status_code, :http_status_code)
                   segment.expect(:http_status_code, :http_status_code) # 2 calls
-                  segment.expect(:uri, :uri)
+                  segment.expect(:uri, uri_for_testing)
+                  segment.expect(:uri, uri_for_testing)
+                  segment.expect(:uri, uri_for_testing) # 3 calls
                   segment.expect(:record_agent_attributes?, true)
                   result = SpanEventPrimitive.for_external_request_segment(segment)
                   expected_intrinsics = {'component' => :library,
                                          'http.method' => :procedure,
+                                         'http.request.method' => :procedure,
                                          'http.statusCode' => :http_status_code,
                                          'category' => 'http',
-                                         'span.kind' => 'client'}
+                                         'span.kind' => 'client',
+                                         'server.address' => 'newrelic.com',
+                                         'server.port' => 443}
                   expected_custom_attrs = {}
-                  expected_agent_attrs = {'http.url' => 'uri'}.merge(existing_agent_attributes)
+                  expected_agent_attrs = {'http.url' => uri_for_testing.to_s}.merge(existing_agent_attributes)
 
                   assert_equal [expected_intrinsics, expected_custom_attrs, expected_agent_attrs], result
                 end
               end
             end
           end
         end
+
+        private
+
+        def uri_for_testing
+          URI.parse('https://newrelic.com')
+        end
       end
     end
   end

test/new_relic/agent/transaction/datastore_segment_test.rb

@@ -289,6 +289,9 @@ def test_sampled_segment_records_span_event
           assert_equal 'calzone_zone', agent_attributes.fetch('db.instance')
           assert_equal 'rachel.foo:1337807', agent_attributes.fetch('peer.address')
           assert_equal 'rachel.foo', agent_attributes.fetch('peer.hostname')
+          assert_equal 'rachel.foo', agent_attributes.fetch('server.address')
+          assert_equal '1337807', agent_attributes.fetch('server.port')
+          assert_equal 'SQLite', agent_attributes.fetch('db.system')
           assert_equal sql_statement, agent_attributes.fetch('db.statement')
         end
 

test/new_relic/agent/transaction/external_request_segment_test.rb

@@ -841,6 +841,9 @@ def test_sampled_external_records_span_event
           assert_equal expected_name, external_intrinsics.fetch('name')
           assert_equal segment.library, external_intrinsics.fetch('component')
           assert_equal segment.procedure, external_intrinsics.fetch('http.method')
+          assert_equal segment.procedure, external_intrinsics.fetch('http.request.method')
+          assert_equal 'remotehost.com', external_intrinsics.fetch('server.address')
+          assert_equal 80, external_intrinsics.fetch('server.port')
           assert_equal 'http', external_intrinsics.fetch('category')
           assert_equal segment.uri.to_s, external_agent_attributes.fetch('http.url')
         end