Clear in memory log on logger start (#1060)

The `@in_memory_log` would continue to keep the messages in memory that were logged during the gem start, before a logger was initialized. This keeps memory allocated while apps are running. This issue was reported in #645, where it also kept logging those in memory logged messages whenever a process was forked and AppSignal started. Clear the in memory log so it uses less memory and doesn't result in stale logs being logged on process forks. Fix issues with the test suite where state on the `Appsignal` module persisted between tests by removing the `@in_memory_log` variable and clearing the `@internal_logger` variable. Closes #645
appsignal · Apr 25, 2024 · a2f4b31 · a2f4b31
1 parent 93b7e87
commit a2f4b31
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 3 deletions.
diff --git a/.changesets/clear-in-memory-log-after-logger-start.md b/.changesets/clear-in-memory-log-after-logger-start.md
@@ -0,0 +1,6 @@
+---
+bump: "patch"
+type: "fix"
+---
+
+Clear the AppSignal in memory logger, used during the gem start, after the file/STDOUT logger is started. This reduces memory usage of the AppSignal Ruby gem by a tiny bit, and prevent stale logs being logged whenever a process gets forked and starts AppSignal.
diff --git a/lib/appsignal.rb b/lib/appsignal.rb
@@ -218,7 +218,10 @@ def start_logger
         else
           Appsignal::Config::DEFAULT_LOG_LEVEL
         end
-      internal_logger << @in_memory_log.string if @in_memory_log
+      return unless @in_memory_log
+
+      internal_logger << @in_memory_log.string
+      @in_memory_log = nil
     end
 
     # Returns if the C-extension was loaded properly.

diff --git a/spec/lib/appsignal_spec.rb b/spec/lib/appsignal_spec.rb
@@ -1121,7 +1121,14 @@
     let(:log_path) { File.join(tmp_dir, "log") }
     let(:log_file) { File.join(log_path, "appsignal.log") }
 
-    before { FileUtils.mkdir_p(log_path) }
+    before do
+      FileUtils.mkdir_p(log_path)
+      # Clear state from previous test
+      Appsignal.internal_logger = nil
+      if Appsignal.instance_variable_defined?(:@in_memory_log)
+        Appsignal.remove_instance_variable(:@in_memory_log)
+      end
+    end
     after { FileUtils.rm_rf(log_path) }
 
     def initialize_config
@@ -1130,6 +1137,7 @@ def initialize_config
         :log_path => log_path
       )
       Appsignal.internal_logger.error("Log in memory")
+      expect(Appsignal.in_memory_log.string).to_not be_empty
     end
 
     context "when the log path is writable" do
@@ -1154,6 +1162,10 @@ def initialize_config
         it "amends in memory log to log file" do
           expect(log_file_contents).to include "[ERROR] appsignal: Log in memory"
         end
+
+        it "clears the in memory log after writing to the new logger" do
+          expect(Appsignal.in_memory_log.string).to be_empty
+        end
       end
 
       context "when the log file is not writable" do
@@ -1178,8 +1190,11 @@ def initialize_config
           expect(output).to include "[ERROR] appsignal: Log in memory"
         end
 
+        it "clears the in memory log after writing to the new logger" do
+          expect(Appsignal.in_memory_log.string).to be_empty
+        end
+
         it "outputs a warning" do
-          puts output
           expect(output).to include \
             "[WARN] appsignal: Unable to start internal logger with log path '#{log_file}'.",
             "[WARN] appsignal: Permission denied"
@@ -1237,6 +1252,10 @@ def initialize_config
       it "amends in memory log to stdout" do
         expect(output).to include "[ERROR] appsignal: Log in memory"
       end
+
+      it "clears the in memory log after writing to the new logger" do
+        expect(Appsignal.in_memory_log.string).to be_empty
+      end
     end
 
     describe "#logger#level" do