Improve documentation and naming based on review comments

Author: Muriel Picone Farías

instrumentation/grape/README.md

@@ -2,7 +2,7 @@
 
 The Grape instrumentation is a community-maintained instrumentation for [Grape](https://github.com/ruby-grape/grape), a REST-like API framework for Ruby.
 
-It relies mostly on the Grape built-in support for `ActiveSupport::Notifications` (more info [here](https://github.com/ruby-grape/grape#active-support-instrumentation)).
+It relies on the Grape built-in support for `ActiveSupport::Notifications` (more info [here](https://github.com/ruby-grape/grape#active-support-instrumentation)).
 
 It currently supports the following events:
 
@@ -31,7 +31,7 @@ OpenTelemetry::SDK.configure do |c|
 end
 ```
 
-Since Grape is "designed to run on Rack or complement existing web application frameworks such as Rails and Sinatra", we recommend using it along with the Rack, Rails and/or Sinatra instrumentations.
+Since Grape is "designed to run on Rack or complement existing web application frameworks such as Rails and Sinatra", it is recommended to use this instrumentation along with the Rack, Rails and/or Sinatra instrumentations.
 
 Alternatively, you can also call `use_all` to install all the available instrumentation.
 
@@ -47,7 +47,7 @@ end
 
 Indicate if any events should not produce spans.
 
-- Accepted values: `:endpoint_render`, `:endpoint_run_filters`.
+- Accepted values: `:endpoint_render`, `:endpoint_run_filters`, `:format_response`.
 - Defaults to `[]` (no ignored events).
 
 Example:
@@ -58,7 +58,7 @@ OpenTelemetry::SDK.configure do |c|
 end
 ```
 
-Note that the `endpoint_run` event cannot be disabled since it is the parent event. If you need to disable the instrumentation, set `:enabled` to `false`:
+Note that the `endpoint_run` event cannot be ignored. If you need to disable the instrumentation, set `:enabled` to `false`:
 
 ```ruby
 OpenTelemetry::SDK.configure do |c|

instrumentation/grape/lib/opentelemetry/instrumentation/grape/custom_subscribers/endpoint_run.rb

@@ -7,8 +7,10 @@
 module OpenTelemetry
   module Instrumentation
     module Grape
-      # Contains all custom subscriber classes that implement the ActiveSupport::Subscriber interface
-      # Custom subscribers are needed to create a span at the start of an event, for example.
+      # Contains custom subscribers that implement the ActiveSupport::Notifications::Fanout::Subscribers::Evented
+      # interface. Custom subscribers are needed to create a span at the start of an event, for example.
+      #
+      # Reference: https://github.com/rails/rails/blob/05cb63abdaf6101e6c8fb43119e2c0d08e543c28/activesupport/lib/active_support/notifications/fanout.rb#L320-L322
       module CustomSubscribers
         # Implements the ActiveSupport::Subscriber interface to instrument the start and finish of the endpoint_run event
         class EndpointRun

instrumentation/grape/lib/opentelemetry/instrumentation/grape/event_handler.rb

@@ -27,7 +27,7 @@ def endpoint_run_finish(_name, _id, payload)
             return unless span && token
 
             if payload[:exception_object]
-              handle_error(span, payload[:exception_object])
+              handle_payload_exception(span, payload[:exception_object])
             else
               span.set_attribute(OpenTelemetry::SemanticConventions::Trace::HTTP_STATUS_CODE, payload[:endpoint].status)
             end
@@ -44,7 +44,7 @@ def endpoint_render(_name, start, _finish, _id, payload)
               'operation' => 'endpoint_render'
             }
             tracer.in_span(name, attributes: attributes, start_timestamp: start, kind: :server) do |span|
-              handle_error(span, payload[:exception_object]) if payload[:exception_object]
+              handle_payload_exception(span, payload[:exception_object]) if payload[:exception_object]
             end
           end
 
@@ -63,7 +63,7 @@ def endpoint_run_filters(_name, start, finish, _id, payload)
               'grape.filter.type' => type.to_s
             }
             tracer.in_span(name, attributes: attributes, start_timestamp: start, kind: :server) do |span|
-              handle_error(span, payload[:exception_object]) if payload[:exception_object]
+              handle_payload_exception(span, payload[:exception_object]) if payload[:exception_object]
             end
           end
 
@@ -77,7 +77,7 @@ def format_response(_name, start, _finish, _id, payload)
               'grape.formatter.type' => formatter_type(payload[:formatter])
             }
             tracer.in_span(name, attributes: attributes, start_timestamp: start, kind: :server) do |span|
-              handle_error(span, payload[:exception_object]) if payload[:exception_object]
+              handle_payload_exception(span, payload[:exception_object]) if payload[:exception_object]
             end
           end
 
@@ -105,7 +105,10 @@ def run_attributes(payload)
             }
           end
 
-          def handle_error(span, exception)
+          # ActiveSupport::Notifications will attach a `:exception_object` to the payload if there was
+          # an error raised during the execution of the &block associated to the Notification.
+          # This can be safely added to the span for tracing.
+          def handle_payload_exception(span, exception)
             span.record_exception(exception)
             span.status = OpenTelemetry::Trace::Status.error("Unhandled exception of type: #{exception.class}")
             return unless exception.respond_to?('status') && exception.status
@@ -114,17 +117,17 @@ def handle_error(span, exception)
           end
 
           def api_instance(endpoint)
-            endpoint.options[:for].base.to_s
+            endpoint.options[:for]&.base.to_s
           end
 
           def request_method(endpoint)
-            endpoint.options.fetch(:method).first
+            endpoint.options[:method]&.first
           end
 
           def path(endpoint)
             namespace = endpoint.routes.first.namespace
             version = endpoint.routes.first.options[:version] || ''
-            prefix = endpoint.routes.first.options[:prefix].to_s || ''
+            prefix = endpoint.routes.first.options[:prefix]&.to_s || ''
             parts = [prefix, version] + namespace.split('/') + endpoint.options[:path]
             parts.reject { |p| p.blank? || p.eql?('/') }.join('/').prepend('/')
           end

instrumentation/grape/lib/opentelemetry/instrumentation/grape/subscriber.rb

@@ -18,12 +18,18 @@ def subscribe
 
           private
 
+          # ActiveSupport::Notifications that can be subscribed to using the documented `.subscribe` interface.
+          #
+          # Reference: https://api.rubyonrails.org/classes/ActiveSupport/Notifications.html#method-c-subscribe
           SUBSCRIPTIONS = {
             endpoint_render: 'endpoint_render.grape',
             endpoint_run_filters: 'endpoint_run_filters.grape',
             format_response: 'format_response.grape'
           }.freeze
 
+          # ActiveSupport::Notifications to be subscribed to that implement the ActiveSupport::Subscriber interface.
+          #
+          # Reference: https://github.com/rails/rails/blob/05cb63abdaf6101e6c8fb43119e2c0d08e543c28/activesupport/lib/active_support/notifications/fanout.rb#L320-L322
           CUSTOM_SUBSCRIPTIONS = {
             endpoint_run: 'OpenTelemetry::Instrumentation::Grape::CustomSubscribers::EndpointRun'
           }.freeze