Store extension install report as JSON (#796)

Store the extension install report as JSON. This makes it consistent across all integrations. Closes #783 I've picked this up earlier than originally planned, because the Ruby 3.1.0 release changes some things with YAML that makes it more difficult to maintain across different Ruby versions. I'd rather remove as much as YAML backwards compatibility logic as possible and use JSON instead that does not have the same issue. `YAML.load` now calls `YAML.safe_load` by default in Ruby 3.1.0. `YAML.safe_load` isn't implemented in all the Ruby versions we still support, so we can't switch to that method call, and calling `YAML.load` will cause differences between Ruby versions. I looked into making parsing YAML standardized for all cases we use it, but different options are needed for `YAML.load` which would default the purpose. Instead I think removing all uses of YAML, except the config file, is the best way to go. The only other use I can find is the `agent.yml` file, we can also convert that to JSON.
appsignal · Jan 7, 2022 · 2587eae · 2587eae
1 parent 382cab3
commit 2587eae
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 22 deletions.
diff --git a/.changesets/store-extension-install-report-as-json.md b/.changesets/store-extension-install-report-as-json.md
@@ -0,0 +1,6 @@
+---
+bump: "patch"
+type: "change"
+---
+
+Store the extension install report as JSON, instead of YAML. Reduces internal complexity.
diff --git a/ext/base.rb b/ext/base.rb
@@ -2,6 +2,7 @@
 require "fileutils"
 require "open-uri"
 require "zlib"
+require "json"
 require "yaml"
 require "rubygems/package"
 require File.expand_path("../../lib/appsignal/version.rb", __FILE__)
@@ -60,7 +61,7 @@ def report
 
 def write_report
   File.open(File.join(EXT_PATH, "install.report"), "w") do |file|
-    file.write YAML.dump(report)
+    file.write JSON.generate(report)
   end
 end
 

diff --git a/lib/appsignal/cli/diagnose.rb b/lib/appsignal/cli/diagnose.rb
@@ -352,8 +352,8 @@ def library_information
         def fetch_installation_report
           path = File.expand_path("../../../../ext/install.report", __FILE__)
           raw_report = File.read(path)
-          Utils.parse_yaml(raw_report)
-        rescue StandardError, Psych::SyntaxError => e # rubocop:disable Lint/ShadowedException
+          JSON.parse(raw_report)
+        rescue StandardError, JSON::ParserError => e # rubocop:disable Lint/ShadowedException
           {
             "parsing_error" => {
               "error" => "#{e.class}: #{e}",
@@ -411,7 +411,7 @@ def print_installation_download_report(report)
         def print_installation_build_report(report)
           report = report.fetch("build", {})
           puts "  Build details"
-          puts_format "Install time", report["time"].to_s, :level => 2
+          puts_format "Install time", report["time"], :level => 2
           puts_format "Architecture", report["architecture"], :level => 2
           puts_format "Target", report["target"], :level => 2
           puts_format "Musl override", report["musl_override"], :level => 2

diff --git a/lib/appsignal/cli/diagnose/utils.rb b/lib/appsignal/cli/diagnose/utils.rb
@@ -32,20 +32,6 @@ def self.read_file_content(path, bytes_to_read)
 
           IO.binread(path, length, offset)
         end
-
-        def self.parse_yaml(contents)
-          if YAML.respond_to? :safe_load
-            if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("2.6.0")
-              # Use keyword params for Ruby 2.6 and up
-              YAML.safe_load(contents, :permitted_classes => [Time])
-            else
-              YAML.safe_load(contents, [Time])
-            end
-          else
-            # Support for Ruby versions without YAML.safe_load
-            YAML.load(contents) # rubocop:disable Security/YAMLLoad
-          end
-        end
       end
     end
   end

diff --git a/spec/integration/diagnose b/spec/integration/diagnose
diff --git a/spec/lib/appsignal/cli/diagnose_spec.rb b/spec/lib/appsignal/cli/diagnose_spec.rb
@@ -321,7 +321,7 @@ def dont_accept_prompt_to_send_diagnostics_report
           expect(File).to receive(:read)
             .with(File.expand_path("../../../../../ext/install.report", __FILE__))
             .and_return(
-              YAML.dump(
+              JSON.generate(
                 "result" => {
                   "status" => "error",
                   "error" => "RuntimeError: some error",
@@ -384,8 +384,8 @@ def dont_accept_prompt_to_send_diagnostics_report
         end
       end
 
-      context "when report is invalid YAML" do
-        let(:raw_report) { "foo:\nbar" }
+      context "when report is invalid JSON" do
+        let(:raw_report) { "{}-" }
         before do
           allow(File).to receive(:read).and_call_original
           expect(File).to receive(:read)
+6 −15		spec/diagnose_spec.rb
+32 −23		spec/support/runner.rb