From 837577a4c750c69d17d8a8fd48fa5ffb6d5669f2 Mon Sep 17 00:00:00 2001
From: Peter Solnica <peter@solnica.online>
Date: Thu, 8 Aug 2024 14:51:24 +0200
Subject: [PATCH] File/attachment uploads (#2357)

---
 CHANGELOG.md                              |  7 ++++
 sentry-ruby/lib/sentry-ruby.rb            |  7 ++++
 sentry-ruby/lib/sentry/attachment.rb      | 42 +++++++++++++++++++
 sentry-ruby/lib/sentry/event.rb           |  4 ++
 sentry-ruby/lib/sentry/scope.rb           | 14 +++++++
 sentry-ruby/lib/sentry/transport.rb       |  6 +++
 sentry-ruby/sentry-ruby.gemspec           |  2 +-
 sentry-ruby/spec/fixtures/attachment.txt  |  1 +
 sentry-ruby/spec/sentry/scope_spec.rb     | 24 +++++++++++
 sentry-ruby/spec/sentry/transport_spec.rb | 18 ++++++++
 sentry-ruby/spec/sentry_spec.rb           | 51 +++++++++++++++++++++++
 sentry-ruby/spec/spec_helper.rb           |  8 ++++
 12 files changed, 183 insertions(+), 1 deletion(-)
 create mode 100644 sentry-ruby/lib/sentry/attachment.rb
 create mode 100644 sentry-ruby/spec/fixtures/attachment.txt

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 087f72996..cf7041e5c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,13 @@
 - Support for tracing Faraday requests ([#2345](https://github.com/getsentry/sentry-ruby/pull/2345))
   - Closes [#1795](https://github.com/getsentry/sentry-ruby/issues/1795)
   - Please note that the Faraday instrumentation has some limitations in case of async requests: https://github.com/lostisland/faraday/issues/1381
+- Support for attachments ([#2357](https://github.com/getsentry/sentry-ruby/pull/2357))
+  Usage:
+
+  ```ruby
+  Sentry.add_attachment(path: '/foo/bar.txt')
+  Sentry.add_attachment(filename: 'payload.json', bytes: '{"value": 42}'))
+  ```
 - Transaction data are now included in the context ([#2365](https://github.com/getsentry/sentry-ruby/pull/2365))
   - Closes [#2364](https://github.com/getsentry/sentry-ruby/issues/2363)
 
diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb
index dd59d6e67..358a7c329 100644
--- a/sentry-ruby/lib/sentry-ruby.rb
+++ b/sentry-ruby/lib/sentry-ruby.rb
@@ -211,6 +211,13 @@ def set_context(*args)
       get_current_scope.set_context(*args)
     end
 
+    # @!method add_attachment
+    #   @!macro add_attachment
+    def add_attachment(**opts)
+      return unless initialized?
+      get_current_scope.add_attachment(**opts)
+    end
+
     ##### Main APIs #####
 
     # Initializes the SDK with given configuration.
diff --git a/sentry-ruby/lib/sentry/attachment.rb b/sentry-ruby/lib/sentry/attachment.rb
new file mode 100644
index 000000000..f40971599
--- /dev/null
+++ b/sentry-ruby/lib/sentry/attachment.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module Sentry
+  class Attachment
+    PathNotFoundError = Class.new(StandardError)
+
+    attr_reader :bytes, :filename, :path, :content_type
+
+    def initialize(bytes: nil, filename: nil, content_type: nil, path: nil)
+      @bytes = bytes
+      @filename = infer_filename(filename, path)
+      @path = path
+      @content_type = content_type
+    end
+
+    def to_envelope_headers
+      { type: 'attachment', filename: filename, content_type: content_type, length: payload.bytesize }
+    end
+
+    def payload
+      @payload ||= if bytes
+        bytes
+      else
+        File.binread(path)
+      end
+    rescue Errno::ENOENT
+      raise PathNotFoundError, "Failed to read attachment file, file not found: #{path}"
+    end
+
+    private
+
+    def infer_filename(filename, path)
+      return filename if filename
+
+      if path
+        File.basename(path)
+      else
+        raise ArgumentError, "filename or path is required"
+      end
+    end
+  end
+end
diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb
index b3fb5b275..7fbd6172c 100644
--- a/sentry-ruby/lib/sentry/event.rb
+++ b/sentry-ruby/lib/sentry/event.rb
@@ -42,6 +42,9 @@ class Event
     # @return [Hash, nil]
     attr_accessor :dynamic_sampling_context
 
+    # @return [Array<Attachment>]
+    attr_accessor :attachments
+
     # @param configuration [Configuration]
     # @param integration_meta [Hash, nil]
     # @param message [String, nil]
@@ -57,6 +60,7 @@ def initialize(configuration:, integration_meta: nil, message: nil)
       @extra         = {}
       @contexts      = {}
       @tags          = {}
+      @attachments   = []
 
       @fingerprint = []
       @dynamic_sampling_context = nil
diff --git a/sentry-ruby/lib/sentry/scope.rb b/sentry-ruby/lib/sentry/scope.rb
index 591a00682..73e873f43 100644
--- a/sentry-ruby/lib/sentry/scope.rb
+++ b/sentry-ruby/lib/sentry/scope.rb
@@ -2,6 +2,7 @@
 
 require "sentry/breadcrumb_buffer"
 require "sentry/propagation_context"
+require "sentry/attachment"
 require "etc"
 
 module Sentry
@@ -22,6 +23,7 @@ class Scope
       :rack_env,
       :span,
       :session,
+      :attachments,
       :propagation_context
     ]
 
@@ -55,6 +57,7 @@ def apply_to_event(event, hint = nil)
         event.level = level
         event.breadcrumbs = breadcrumbs
         event.rack_env = rack_env if rack_env
+        event.attachments = attachments
       end
 
       if span
@@ -102,6 +105,7 @@ def dup
       copy.span = span.deep_dup
       copy.session = session.deep_dup
       copy.propagation_context = propagation_context.deep_dup
+      copy.attachments = attachments.dup
       copy
     end
 
@@ -119,6 +123,7 @@ def update_from_scope(scope)
       self.fingerprint = scope.fingerprint
       self.span = scope.span
       self.propagation_context = scope.propagation_context
+      self.attachments = scope.attachments
     end
 
     # Updates the scope's data from the given options.
@@ -128,6 +133,7 @@ def update_from_scope(scope)
     # @param user [Hash]
     # @param level [String, Symbol]
     # @param fingerprint [Array]
+    # @param attachments [Array<Attachment>]
     # @return [Array]
     def update_from_options(
       contexts: nil,
@@ -136,6 +142,7 @@ def update_from_options(
       user: nil,
       level: nil,
       fingerprint: nil,
+      attachments: nil,
       **options
     )
       self.contexts.merge!(contexts) if contexts
@@ -283,6 +290,12 @@ def generate_propagation_context(env = nil)
       @propagation_context = PropagationContext.new(self, env)
     end
 
+    # Add a new attachment to the scope.
+    def add_attachment(**opts)
+      attachments << (attachment = Attachment.new(**opts))
+      attachment
+    end
+
     protected
 
     # for duplicating scopes internally
@@ -303,6 +316,7 @@ def set_default_value
       @rack_env = {}
       @span = nil
       @session = nil
+      @attachments = []
       generate_propagation_context
       set_new_breadcrumb_buffer
     end
diff --git a/sentry-ruby/lib/sentry/transport.rb b/sentry-ruby/lib/sentry/transport.rb
index acef83de0..39a993fe5 100644
--- a/sentry-ruby/lib/sentry/transport.rb
+++ b/sentry-ruby/lib/sentry/transport.rb
@@ -145,6 +145,12 @@ def envelope_from_event(event)
         )
       end
 
+      if event.is_a?(Event) && event.attachments.any?
+        event.attachments.each do |attachment|
+          envelope.add_item(attachment.to_envelope_headers, attachment.payload)
+        end
+      end
+
       client_report_headers, client_report_payload = fetch_pending_client_report
       envelope.add_item(client_report_headers, client_report_payload) if client_report_headers
 
diff --git a/sentry-ruby/sentry-ruby.gemspec b/sentry-ruby/sentry-ruby.gemspec
index 31fa0b581..52cbcc180 100644
--- a/sentry-ruby/sentry-ruby.gemspec
+++ b/sentry-ruby/sentry-ruby.gemspec
@@ -26,6 +26,6 @@ Gem::Specification.new do |spec|
 
   spec.require_paths = ["lib"]
 
-  spec.add_dependency "concurrent-ruby", '~> 1.0', '>= 1.0.2'
+  spec.add_dependency "concurrent-ruby", "~> 1.0", ">= 1.0.2"
   spec.add_dependency "bigdecimal"
 end
diff --git a/sentry-ruby/spec/fixtures/attachment.txt b/sentry-ruby/spec/fixtures/attachment.txt
new file mode 100644
index 000000000..3b18e512d
--- /dev/null
+++ b/sentry-ruby/spec/fixtures/attachment.txt
@@ -0,0 +1 @@
+hello world
diff --git a/sentry-ruby/spec/sentry/scope_spec.rb b/sentry-ruby/spec/sentry/scope_spec.rb
index 651c995d1..5de4d8d6d 100644
--- a/sentry-ruby/spec/sentry/scope_spec.rb
+++ b/sentry-ruby/spec/sentry/scope_spec.rb
@@ -201,6 +201,7 @@
       scope.set_user({ id: 1 })
       scope.set_transaction_name("WelcomeController#index", source: :view)
       scope.set_fingerprint(["foo"])
+      scope.add_attachment(bytes: "file-data", filename: "test.txt")
       scope
     end
 
@@ -219,6 +220,10 @@
       expect(event.contexts).to include(:trace)
       expect(event.contexts[:os].keys).to match_array([:name, :version, :build, :kernel_version, :machine])
       expect(event.contexts.dig(:runtime, :version)).to match(/ruby/)
+
+      attachment = event.attachments.first
+      expect(attachment.filename).to eql("test.txt")
+      expect(attachment.bytes).to eql("file-data")
     end
 
     it "does not apply the contextual data to a check-in event" do
@@ -361,4 +366,23 @@
       expect(result).to eq([:foo, :bar])
     end
   end
+
+  describe "#add_attachment" do
+    before { perform_basic_setup }
+
+    let(:opts) do
+      { bytes: "file-data", filename: "test.txt" }
+    end
+
+    subject do
+      described_class.new
+    end
+
+    it "adds a new attachment" do
+      attachment = subject.add_attachment(**opts)
+
+      expect(attachment.bytes).to eq("file-data")
+      expect(attachment.filename).to eq("test.txt")
+    end
+  end
 end
diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb
index 73b22d91d..b78dc7878 100644
--- a/sentry-ruby/spec/sentry/transport_spec.rb
+++ b/sentry-ruby/spec/sentry/transport_spec.rb
@@ -435,6 +435,24 @@
         end
       end
     end
+
+    context "event with attachments" do
+      let(:event) { client.event_from_exception(ZeroDivisionError.new("divided by 0")) }
+      let(:envelope) { subject.envelope_from_event(event) }
+
+      before do
+        event.attachments << Sentry::Attachment.new(filename: "test-1.txt", bytes: "test")
+        event.attachments << Sentry::Attachment.new(path: fixture_path("attachment.txt"))
+      end
+
+      it "sends the event and logs the action" do
+        expect(subject).to receive(:send_data)
+
+        subject.send_envelope(envelope)
+
+        expect(io.string).to match(/Sending envelope with items \[event, attachment, attachment\]/)
+      end
+    end
   end
 
   describe "#send_event" do
diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb
index 0e30d150e..dd3f85c93 100644
--- a/sentry-ruby/spec/sentry_spec.rb
+++ b/sentry-ruby/spec/sentry_spec.rb
@@ -713,6 +713,57 @@
     end
   end
 
+  describe ".add_attachment" do
+    it "adds a new attachment to the current scope with provided filename and bytes" do
+      described_class.add_attachment(filename: "test.txt", bytes: "test")
+
+      expect(described_class.get_current_scope.attachments.size).to be(1)
+
+      attachment = described_class.get_current_scope.attachments.first
+      expect(attachment.filename).to eq("test.txt")
+      expect(attachment.bytes).to eq("test")
+    end
+
+    it "adds a new attachment to the current scope with provided path to a file" do
+      described_class.add_attachment(path: fixture_path("attachment.txt"))
+
+      expect(described_class.get_current_scope.attachments.size).to be(1)
+
+      attachment = described_class.get_current_scope.attachments.first
+      expect(attachment.filename).to eq("attachment.txt")
+      expect(attachment.payload).to eq("hello world\n")
+    end
+
+    it "adds a new attachment to the current scope favoring bytes over path" do
+      described_class.add_attachment(path: fixture_path("attachment.txt"), bytes: "test", content_type: "text/plain")
+
+      expect(described_class.get_current_scope.attachments.size).to be(1)
+
+      attachment = described_class.get_current_scope.attachments.first
+      expect(attachment.filename).to eq("attachment.txt")
+      expect(attachment.content_type).to eq("text/plain")
+      expect(attachment.payload).to eq("test")
+    end
+
+    it "raises meaningful error when path is invalid" do
+      described_class.add_attachment(path: "/not-here/oops")
+
+      expect(described_class.get_current_scope.attachments.size).to be(1)
+
+      attachment = described_class.get_current_scope.attachments.first
+
+      expect { attachment.payload }
+        .to raise_error(
+          Sentry::Attachment::PathNotFoundError,
+            "Failed to read attachment file, file not found: /not-here/oops"
+          )
+    end
+
+    it "requires either filename or path" do
+      expect { described_class.add_attachment(bytes: "test") }.to raise_error(ArgumentError, "filename or path is required")
+    end
+  end
+
   describe ".csp_report_uri" do
     it "returns the csp_report_uri generated from the main Configuration" do
       expect(Sentry.configuration).to receive(:csp_report_uri).and_call_original
diff --git a/sentry-ruby/spec/spec_helper.rb b/sentry-ruby/spec/spec_helper.rb
index 6ba0ccee0..611205163 100644
--- a/sentry-ruby/spec/spec_helper.rb
+++ b/sentry-ruby/spec/spec_helper.rb
@@ -57,6 +57,14 @@
   end
 end
 
+def fixtures_root
+  @fixtures_root ||= Pathname(__dir__).join("fixtures")
+end
+
+def fixture_path(name)
+  fixtures_root.join(name).realpath
+end
+
 def build_exception_with_cause(cause = "exception a")
   begin
     raise cause