CHEF-13: remove magic from the logger/formatter settings

closes #2514

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

6 files changed, 78 insertions, 86 deletions

diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index b16550b8e6..fa3b34e08d 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -269,3 +269,26 @@ use `recipe.rb` in the root of the cookbook.
 The `use_inline_resources` provider mode is always enabled when using the
 `action :name do ... end` syntax. You can remove the `use_inline_resources`
 line.
+
+### Magic removed from logger and formatter settings
+
+There is no more magic involved in the setting of the logger and the formatter, the default is now the formatter.
+
+Setting the `log_location` in the config.rb now applies to both daemonized and command-line invocation and there is
+no magic which dups the logger in order to log to STDOUT on the command line in that case.
+
+Setting logger settings in config.rb is most likely no longer appropriate and those settings should be changed to
+command line flags on the config script or cronjob that invokes chef-client daemonized.  In other words:
+
+```
+chef-client -d -L /var/log/client.rb --force-logger
+```
+
+If your logfiles switch to the formatter, you need to include `--force-logger` for your daemonized runs.
+
+If your command line invocations have no output, delete the config.rb setting for the log_location and move that
+to the command line that invokes your daemon process.
+
+Redirecting output to a file with `chef-client > /tmp/chef.out` now captures the same output as invoking it directly on the command
+line with no redirection.
+
diff --git a/lib/chef/application.rb b/lib/chef/application.rb
index eba0d48e57..005418d184 100644
--- a/lib/chef/application.rb
+++ b/lib/chef/application.rb
@@ -144,22 +144,12 @@ class Chef
     # unattended, the `force_formatter` option is provided.
     #
     # === Auto Log Level
-    # When `log_level` is set to `:auto` (default), the log level will be `:warn`
-    # when the primary output mode is an output formatter (see
-    # +using_output_formatter?+) and `:info` otherwise.
+    # The `log_level` of `:auto` means `:warn` in the formatter and `:info` in
+    # the logger.
     #
-    # === Automatic STDOUT Logging
-    # When `force_logger` is configured (e.g., Chef 10 mode), a second logger
-    # with output on STDOUT is added when running in a console (STDOUT is a tty)
-    # and the configured log_location isn't STDOUT. This accounts for the case
-    # that a user has configured a log_location in client.rb, but is running
-    # chef-client by hand to troubleshoot a problem.
     def configure_logging
       configure_log_location
       Chef::Log.init(MonoLogger.new(Chef::Config[:log_location]))
-      if want_additional_logger?
-        configure_stdout_logger
-      end
       Chef::Log.level = resolve_log_level
     rescue StandardError => error
       Chef::Log.fatal("Failed to open or create log file at #{Chef::Config[:log_location]}: #{error.class} (#{error.message})")
@@ -180,22 +170,9 @@ class Chef
         end
     end
 
-    def configure_stdout_logger
-      stdout_logger = MonoLogger.new(STDOUT)
-      stdout_logger.formatter = Chef::Log.logger.formatter
-      Chef::Log.loggers << stdout_logger
-    end
-
-    # Based on config and whether or not STDOUT is a tty, should we setup a
-    # secondary logger for stdout?
-    def want_additional_logger?
-      ( Chef::Config[:log_location] != STDOUT ) && STDOUT.tty? && (!Chef::Config[:daemonize]) && (Chef::Config[:force_logger])
-    end
-
-    # Use of output formatters is assumed if `force_formatter` is set or if
-    # `force_logger` is not set and STDOUT is to a console (tty)
+    # Use of output formatters is assumed if `force_formatter` is set or if `force_logger` is not set
     def using_output_formatter?
-      Chef::Config[:force_formatter] || (!Chef::Config[:force_logger] && STDOUT.tty?)
+      Chef::Config[:force_formatter] || !Chef::Config[:force_logger]
     end
 
     def auto_log_level?
diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index b219fdfdd6..c3eaa02bed 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -372,7 +372,7 @@ class Chef
 
     # @api private
     def default_formatter
-      if (STDOUT.tty? && !Chef::Config[:force_logger]) || Chef::Config[:force_formatter]
+      if !Chef::Config[:force_logger] || Chef::Config[:force_formatter]
         [:doc]
       else
         [:null]
diff --git a/spec/integration/client/client_spec.rb b/spec/integration/client/client_spec.rb
index 1cd74a6fd1..733503de42 100644
--- a/spec/integration/client/client_spec.rb
+++ b/spec/integration/client/client_spec.rb
@@ -294,10 +294,9 @@ chef_server_url 'http://omg.com/blah'
 cookbook_path "#{path_to('cookbooks')}"
 EOM
 
-      result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -r 'x::default' -z", :cwd => chef_dir)
+      result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -r 'x::default' -z -l info", :cwd => chef_dir)
       expect(result.stdout).not_to include("Overridden Run List")
       expect(result.stdout).to include("Run List is [recipe[x::default]]")
-      #puts result.stdout
       result.error!
     end
 
@@ -445,7 +444,7 @@ control_group "control group without top level control" do
 end
       RECIPE
 
-      result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -o 'audit_test::succeed'", :cwd => chef_dir)
+      result = shell_out("#{chef_client} -c \"#{path_to('config/client.rb')}\" -o 'audit_test::succeed' -l info", :cwd => chef_dir)
       expect(result.error?).to be_falsey
       expect(result.stdout).to include("Successfully executed all `control_group` blocks and contained examples")
     end
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index a12935fa78..72921c90ca 100644
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -1,7 +1,7 @@
 #
 # Author:: AJ Christensen (<aj@junglist.gen.nz>)
 # Author:: Mark Mzyk (mmzyk@chef.io)
-# Copyright:: Copyright 2008-2016, Chef Software Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -192,48 +192,49 @@ describe Chef::Application do
       end
 
       shared_examples_for "log_level_is_auto" do
-        context "when STDOUT is to a tty" do
+        before do
+          allow(STDOUT).to receive(:tty?).and_return(true)
+        end
+
+        it "configures the log level to :warn" do
+          @app.configure_logging
+          expect(Chef::Log.level).to eq(:warn)
+        end
+
+        context "when force_formater is configured" do
           before do
-            allow(STDOUT).to receive(:tty?).and_return(true)
+            Chef::Config[:force_formatter] = true
           end
 
-          it "configures the log level to :warn" do
+          it "configures the log level to warn" do
             @app.configure_logging
             expect(Chef::Log.level).to eq(:warn)
           end
-
-          context "when force_logger is configured" do
-            before do
-              Chef::Config[:force_logger] = true
-            end
-
-            it "configures the log level to info" do
-              @app.configure_logging
-              expect(Chef::Log.level).to eq(:info)
-            end
-          end
         end
 
-        context "when STDOUT is not to a tty" do
+        context "when force_logger is configured" do
           before do
-            allow(STDOUT).to receive(:tty?).and_return(false)
+            Chef::Config[:force_logger] = true
           end
 
-          it "configures the log level to :info" do
+          it "configures the log level to info" do
             @app.configure_logging
             expect(Chef::Log.level).to eq(:info)
           end
+        end
 
-          context "when force_formatter is configured" do
-            before do
-              Chef::Config[:force_formatter] = true
-            end
-            it "sets the log level to :warn" do
-              @app.configure_logging
-              expect(Chef::Log.level).to eq(:warn)
-            end
+        context "when both are is configured" do
+          before do
+            Chef::Config[:force_logger] = true
+            Chef::Config[:force_formatter] = true
+          end
+
+          it "configures the log level to warn" do
+            @app.configure_logging
+            expect(Chef::Log.level).to eq(:warn)
           end
         end
+
       end
 
       context "when log_level is not set" do
diff --git a/spec/unit/client_spec.rb b/spec/unit/client_spec.rb
index 7ffc17c4fc..a2bb573e15 100644
--- a/spec/unit/client_spec.rb
+++ b/spec/unit/client_spec.rb
@@ -2,7 +2,7 @@
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: Tim Hinderliter (<tim@chef.io>)
 # Author:: Christopher Walters (<cw@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software, Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -71,48 +71,40 @@ describe Chef::Client do
 
   describe "configuring output formatters" do
     context "when no formatter has been configured" do
-      context "and STDOUT is a TTY" do
+      it "configures the :doc formatter" do
+        expect(client.formatters_for_run).to eq([[:doc]])
+      end
+
+      context "and force_logger is set" do
         before do
-          allow(STDOUT).to receive(:tty?).and_return(true)
+          Chef::Config[:force_logger] = true
         end
 
-        it "configures the :doc formatter" do
-          expect(client.formatters_for_run).to eq([[:doc]])
+        it "configures the :null formatter" do
+          expect(client.formatters_for_run).to eq([[:null]])
         end
+      end
 
-        context "and force_logger is set" do
-          before do
-            Chef::Config[:force_logger] = true
-          end
-
-          it "configures the :null formatter" do
-            expect(Chef::Config[:force_logger]).to be_truthy
-            expect(client.formatters_for_run).to eq([[:null]])
-          end
-
+      context "and force_formatter is set" do
+        before do
+          Chef::Config[:force_formatter] = true
         end
 
+        it "configures the :doc formatter" do
+          expect(client.formatters_for_run).to eq([[:doc]])
+        end
       end
 
-      context "and STDOUT is not a TTY" do
+      context "both are set" do
         before do
-          allow(STDOUT).to receive(:tty?).and_return(false)
-        end
-
-        it "configures the :null formatter" do
-          expect(client.formatters_for_run).to eq([[:null]])
+          Chef::Config[:force_formatter] = true
+          Chef::Config[:force_logger] = true
         end
 
-        context "and force_formatter is set" do
-          before do
-            Chef::Config[:force_formatter] = true
-          end
-          it "it configures the :doc formatter" do
-            expect(client.formatters_for_run).to eq([[:doc]])
-          end
+        it "configures the :doc formatter" do
+          expect(client.formatters_for_run).to eq([[:doc]])
         end
       end
-
     end
 
     context "when a formatter is configured" do