appsignal-ruby
appsignal-ruby copied to clipboard
Only log the unwritable log directory message once
When the configured or default log directory is not writable the following message can be printed multiple times, whenever the Appsignal::Config#log_file_path method is called:
appsignal: Unable to log to '/Users/tombruijn/appsignal/agent/integrations/appsignal-ruby/log'. Logging to '/private/tmp' instead. Please check the permissions for the application's (log) directory.
appsignal: Unable to log to '/Users/tombruijn/appsignal/agent/integrations/appsignal-ruby/log'. Logging to '/private/tmp' instead. Please check the permissions for the application's (log) directory.
appsignal: Unable to log to '/Users/tombruijn/appsignal/agent/integrations/appsignal-ruby/log'. Logging to '/private/tmp' instead. Please check the permissions for the application's (log) directory.
appsignal: Unable to log to '/Users/tombruijn/appsignal/agent/integrations/appsignal-ruby/log'. Logging to '/private/tmp' instead. Please check the permissions for the application's (log) directory.
We should only print this message once.
I tried this before and ran into some issues with the test suite, to assert this message is only printed once. My work so far:
diff --git lib/appsignal/config.rb lib/appsignal/config.rb
index 6c5a7fee..1d3fa9e8 100644
--- lib/appsignal/config.rb
+++ lib/appsignal/config.rb
@@ -240,9 +240,12 @@ module Appsignal
end
def log_file_path
+ return @log_file_path if defined? @log_file_path
+
path = config_hash[:log_path] || root_path && File.join(root_path, "log")
if path && File.writable?(path)
- return File.join(File.realpath(path), "appsignal.log")
+ @log_file_path = File.join(File.realpath(path), "appsignal.log")
+ return @log_file_path
end
system_tmp_dir = self.class.system_tmp_dir
@@ -250,7 +253,7 @@ module Appsignal
$stdout.puts "appsignal: Unable to log to '#{path}'. Logging to "\
"'#{system_tmp_dir}' instead. Please check the "\
"permissions for the application's (log) directory."
- File.join(system_tmp_dir, "appsignal.log")
+ @log_file_path = File.join(system_tmp_dir, "appsignal.log")
else
$stdout.puts "appsignal: Unable to log to '#{path}' or the "\
"'#{system_tmp_dir}' fallback. Please check the permissions "\
diff --git spec/lib/appsignal/config_spec.rb spec/lib/appsignal/config_spec.rb
index 8eee2bb2..5b711050 100644
--- spec/lib/appsignal/config_spec.rb
+++ spec/lib/appsignal/config_spec.rb
@@ -581,7 +581,10 @@ describe Appsignal::Config do
let(:out_stream) { std_stream }
let(:output) { out_stream.read }
let(:config) { project_fixture_config("production", :log_path => log_path) }
- subject { capture_stdout(out_stream) { config.log_file_path } }
+
+ def log_file_path
+ capture_stdout(out_stream) { config.log_file_path }
+ end
context "when path is writable" do
let(:log_path) { File.join(tmp_dir, "writable-path") }
@@ -589,11 +592,11 @@ describe Appsignal::Config do
after { FileUtils.rm_rf(log_path) }
it "returns log file path" do
- expect(subject).to eq File.join(log_path, "appsignal.log")
+ expect(log_file_path).to eq File.join(log_path, "appsignal.log")
end
it "prints no warning" do
- subject
+ log_file_path
expect(output).to be_empty
end
end
@@ -607,13 +610,17 @@ describe Appsignal::Config do
before { FileUtils.chmod(0o777, system_tmp_dir) }
it "returns returns the tmp location" do
- expect(subject).to eq(File.join(system_tmp_dir, "appsignal.log"))
+ expect(log_file_path).to eq(File.join(system_tmp_dir, "appsignal.log"))
end
- it "prints a warning" do
- subject
- expect(output).to include "appsignal: Unable to log to '#{log_path}'. "\
+ it "prints a warning once" do
+ capture_stdout(out_stream) do
+ config.log_file_path
+ config.log_file_path
+ end
+ message = "appsignal: Unable to log to '#{log_path}'. "\
"Logging to '#{system_tmp_dir}' instead."
+ expect(output.scan(message).count).to eq(1)
end
end
@@ -621,13 +628,22 @@ describe Appsignal::Config do
before { FileUtils.chmod(0o555, system_tmp_dir) }
it "returns nil" do
- expect(subject).to be_nil
+ expect(log_file_path).to be_nil
end
- it "prints a warning" do
- subject
- expect(output).to include "appsignal: Unable to log to '#{log_path}' "\
- "or the '#{system_tmp_dir}' fallback."
+ it "prints a warning once", :only do
+ puts "\n\n\n"
+ capture_stdout(out_stream) do
+ config.log_file_path
+ config.log_file_path
+ end
+ puts output
+ message = "appsignal: Unable to log to '#{log_path}'. "\
+ "Logging to '#{system_tmp_dir}' instead."
+ puts "=" * 80
+ puts message
+ puts "-" * 80
+ expect(output.scan(message).count).to eq(1)
end
end
end
@@ -643,11 +659,11 @@ describe Appsignal::Config do
context "when root_path is set" do
it "returns returns the project log location" do
- expect(subject).to eq File.join(config.root_path, "log/appsignal.log")
+ expect(log_file_path).to eq File.join(config.root_path, "log/appsignal.log")
end
it "prints no warning" do
- subject
+ log_file_path
expect(output).to be_empty
end
end
@@ -707,7 +723,7 @@ describe Appsignal::Config do
end
it "returns real path of log path" do
- expect(subject).to eq(File.join(real_path, "appsignal.log"))
+ expect(log_file_path).to eq(File.join(real_path, "appsignal.log"))
end
end
end