Support ActiveStorage spans in tracing events

CHANGELOG.md

@@ -18,6 +18,7 @@
 
   **Please note that this is an experimental feature and could be further enhanced/removed before the final release.**
 
+- Support `ActiveStorage` spans in tracing events [#1588](https://github.com/getsentry/sentry-ruby/pull/1588)
 
 ### Miscellaneous
 

sentry-rails/examples/rails-6.0/Gemfile

@@ -23,6 +23,8 @@ gem 'jbuilder', '~> 2.7'
 # Use Active Model has_secure_password
 # gem 'bcrypt', '~> 3.1.7'
 
+gem 'image_processing', '~> 1.2'
+
 gem 'sentry-ruby', path: "../../../sentry-ruby"
 gem 'sentry-sidekiq', path: "../../../sentry-sidekiq"
 gem 'sentry-resque', path: "../../../sentry-resque"

sentry-rails/examples/rails-6.0/app/controllers/posts_controller.rb

@@ -10,6 +10,12 @@ def index
   # GET /posts/1
   # GET /posts/1.json
   def show
+    @post.cover.attach(
+      io: File.open(File.join(Rails.root, 'public', 'favicon.ico')),
+      filename: 'favicon.ico',
+      identify: false
+    )
+    @post
   end
 
   # GET /posts/new
@@ -69,6 +75,6 @@ def set_post
 
     # Only allow a list of trusted parameters through.
     def post_params
-      params.require(:post).permit(:title, :content)
+      params.require(:post).permit(:title, :content, :cover)
     end
 end

sentry-rails/examples/rails-6.0/app/models/post.rb

@@ -1,5 +1,3 @@
 class Post < ApplicationRecord
-  before_save do
-    raise "foo"
-  end
+  has_one_attached :cover
 end

sentry-rails/examples/rails-6.0/app/views/posts/_form.html.erb

@@ -21,6 +21,11 @@
     <%= form.text_area :content %>
   </div>
 
+  <div class="field">
+    <%= form.label :cover %>
+    <%= form.file_field :cover %>
+  </div>
+
   <div class="actions">
     <%= form.submit %>
   </div>

sentry-rails/examples/rails-6.0/app/views/posts/show.html.erb

@@ -10,5 +10,10 @@
   <%= @post.content %>
 </p>
 
+<p>
+  <strong>Cover:</strong>
+  <%= image_tag @post.cover.variant(resize_to_limit: [100, 100]) %>
+</p>
+
 <%= link_to 'Edit', edit_post_path(@post) %> |
 <%= link_to 'Back', posts_path %>

sentry-rails/examples/rails-6.0/db/migrate/20211002044752_create_active_storage_tables.active_storage.rb

@@ -0,0 +1,36 @@
+# This migration comes from active_storage (originally 20170806125915)
+class CreateActiveStorageTables < ActiveRecord::Migration[5.2]
+  def change
+    create_table :active_storage_blobs do |t|
+      t.string   :key,          null: false
+      t.string   :filename,     null: false
+      t.string   :content_type
+      t.text     :metadata
+      t.string   :service_name, null: false
+      t.bigint   :byte_size,    null: false
+      t.string   :checksum,     null: false
+      t.datetime :created_at,   null: false
+
+      t.index [ :key ], unique: true
+    end
+
+    create_table :active_storage_attachments do |t|
+      t.string     :name,     null: false
+      t.references :record,   null: false, polymorphic: true, index: false
+      t.references :blob,     null: false
+
+      t.datetime :created_at, null: false
+
+      t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true
+      t.foreign_key :active_storage_blobs, column: :blob_id
+    end
+
+    create_table :active_storage_variant_records do |t|
+      t.belongs_to :blob, null: false, index: false
+      t.string :variation_digest, null: false
+
+      t.index %i[ blob_id variation_digest ], name: "index_active_storage_variant_records_uniqueness", unique: true
+      t.foreign_key :active_storage_blobs, column: :blob_id
+    end
+  end
+end

sentry-rails/examples/rails-6.0/db/schema.rb

@@ -2,15 +2,43 @@
 # of editing this file, please use the migrations feature of Active Record to
 # incrementally modify your database, and then regenerate this schema definition.
 #
-# This file is the source Rails uses to define your schema when running `rails
-# db:schema:load`. When creating a new database, `rails db:schema:load` tends to
+# This file is the source Rails uses to define your schema when running `bin/rails
+# db:schema:load`. When creating a new database, `bin/rails db:schema:load` tends to
 # be faster and is potentially less error prone than running all of your
 # migrations from scratch. Old migrations may fail to apply correctly if those
 # migrations use external dependencies or application code.
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_11_20_074001) do
+ActiveRecord::Schema.define(version: 2021_10_02_044752) do
+
+  create_table "active_storage_attachments", force: :cascade do |t|
+    t.string "name", null: false
+    t.string "record_type", null: false
+    t.integer "record_id", null: false
+    t.integer "blob_id", null: false
+    t.datetime "created_at", null: false
+    t.index ["blob_id"], name: "index_active_storage_attachments_on_blob_id"
+    t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true
+  end
+
+  create_table "active_storage_blobs", force: :cascade do |t|
+    t.string "key", null: false
+    t.string "filename", null: false
+    t.string "content_type"
+    t.text "metadata"
+    t.string "service_name", null: false
+    t.bigint "byte_size", null: false
+    t.string "checksum", null: false
+    t.datetime "created_at", null: false
+    t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
+  end
+
+  create_table "active_storage_variant_records", force: :cascade do |t|
+    t.integer "blob_id", null: false
+    t.string "variation_digest", null: false
+    t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true
+  end
 
   create_table "posts", force: :cascade do |t|
     t.string "title"
@@ -19,4 +47,6 @@
     t.datetime "updated_at", precision: 6, null: false
   end
 
+  add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
+  add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
 end

sentry-rails/lib/sentry/rails/configuration.rb

@@ -1,6 +1,7 @@
 require "sentry/rails/tracing/action_controller_subscriber"
 require "sentry/rails/tracing/action_view_subscriber"
 require "sentry/rails/tracing/active_record_subscriber"
+require "sentry/rails/tracing/active_storage_subscriber"
 
 module Sentry
   class Configuration
@@ -59,7 +60,8 @@ def initialize
         @tracing_subscribers = Set.new([
           Sentry::Rails::Tracing::ActionControllerSubscriber,
           Sentry::Rails::Tracing::ActionViewSubscriber,
-          Sentry::Rails::Tracing::ActiveRecordSubscriber
+          Sentry::Rails::Tracing::ActiveRecordSubscriber,
+          Sentry::Rails::Tracing::ActiveStorageSubscriber
         ])
       end
     end

sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb

@@ -9,7 +9,9 @@ def subscribe!
           end
 
           def unsubscribe!
-            ActiveSupport::Notifications.unsubscribe(*self::EVENT_NAMES)
+            self::EVENT_NAMES.each do |name|
+              ActiveSupport::Notifications.unsubscribe(name)
+            end
           end
 
           if ::Rails.version.to_i == 5

sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb

@@ -0,0 +1,34 @@
+require "sentry/rails/tracing/abstract_subscriber"
+
+module Sentry
+  module Rails
+    module Tracing
+      class ActiveStorageSubscriber < AbstractSubscriber
+        EVENT_NAMES = %w(
+          service_upload.active_storage
+          service_download.active_storage
+          service_streaming_download.active_storage
+          service_download_chunk.active_storage
+          service_delete.active_storage
+          service_delete_prefixed.active_storage
+          service_exist.active_storage
+          service_url.active_storage
+          service_mirror.active_storage
+          service_update_metadata.active_storage
+          preview.active_storage
+          analyze.active_storage
+        ).freeze
+
+        def self.subscribe!
+          subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload|
+            record_on_current_span(op: event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:service], duration: duration) do |span|
+              payload.each do |key, value|
+                span.set_data(key, value) unless key == START_TIMESTAMP_NAME
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

sentry-rails/spec/sentry/rails/configuration_spec.rb

@@ -23,12 +23,12 @@
     class MySubscriber; end
 
     it "returns the default subscribers" do
-      expect(subject.tracing_subscribers.size).to eq(3)
+      expect(subject.tracing_subscribers.size).to eq(4)
     end
 
     it "is customizable" do
       subject.tracing_subscribers << MySubscriber
-      expect(subject.tracing_subscribers.size).to eq(4)
+      expect(subject.tracing_subscribers.size).to eq(5)
     end
 
     it "is replaceable" do

sentry-rails/spec/sentry/rails/tracing/active_storage_subscriber_spec.rb

@@ -0,0 +1,49 @@
+require "spec_helper"
+
+RSpec.describe Sentry::Rails::Tracing::ActiveStorageSubscriber, :subscriber, type: :request, skip: Rails.version.to_f <= 5.2 do
+  let(:transport) do
+    Sentry.get_current_client.transport
+  end
+
+  context "when transaction is sampled" do
+    before do
+      make_basic_app do |config|
+        config.traces_sample_rate = 1.0
+        config.rails.tracing_subscribers = [described_class]
+      end
+    end
+
+    it "records the upload event" do
+      p = Post.create!
+      get "/posts/#{p.id}/attach"
+
+      expect(response).to have_http_status(:ok)
+      expect(transport.events.count).to eq(1)
+
+      transaction = transport.events.first.to_hash
+      expect(transaction[:type]).to eq("transaction")
+      expect(transaction[:spans].count).to eq(1)
+
+      span = transaction[:spans][0]
+      expect(span[:op]).to eq("service_upload.active_storage")
+      expect(span[:description]).to eq("Disk")
+      expect(span.dig(:data, :key)).to eq(p.cover.key)
+      expect(span[:trace_id]).to eq(transaction.dig(:contexts, :trace, :trace_id))
+    end
+  end
+
+  context "when transaction is not sampled" do
+    before do
+      make_basic_app
+    end
+
+    it "doesn't record spans" do
+      p = Post.create!
+      get "/posts/#{p.id}/attach"
+
+      expect(response).to have_http_status(:ok)
+
+      expect(transport.events.count).to eq(0)
+    end
+  end
+end