Merge pull request #134 from chef/SHACK-188/telemetry-cleanup-tasks

[SHACK-188] telemetry cleanup tasks

components/chef-cli/lib/chef-cli/cli.rb

@@ -79,22 +79,34 @@ def setup_cli
       # Creates the tree we need under ~/.chef-workstation
       # based on config settings:
       Config.create_directory_tree
-      # TODO because we have not loaded a command, we wil lalways be using
+
+      # TODO because we have not loaded a command, we will always be using
       #      the default location at this step.
       if Config.using_default_location? && !Config.exist?
-        UI::Terminal.output T.creating_config(Config.default_location)
-        Config.create_default_config_file
-        # Tell the user we're anonymously tracking, give brief opt-out
-        # and a link to detailed information.
-        UI::Terminal.output ""
-        UI::Terminal.output T.telemetry_enabled(Config.default_location)
-        UI::Terminal.output ""
+        setup_workstation
       end
+
       Config.load
       ChefCLI::Log.setup(Config.log.location, Config.log.level.to_sym)
       ChefCLI::Log.info("Initialized logger")
     end
 
+    # This setup command is run if ".chef-workstation" is missing prior to
+    # the run.  It will set up default configuration, generated an installation id
+    # for telemetry, and report telemetry & config info to the operator.
+    def setup_workstation
+      require "securerandom"
+      installation_id = SecureRandom.uuid
+      File.write(Config.telemetry_installation_identifier_file, installation_id)
+      UI::Terminal.output T.creating_config(Config.default_location)
+      Config.create_default_config_file
+      # Tell the user we're anonymously tracking, give brief opt-out
+      # and a link to detailed information.
+      UI::Terminal.output ""
+      UI::Terminal.output T.telemetry_enabled(Config.default_location)
+      UI::Terminal.output ""
+    end
+
     def perform_command
       update_args_for_help
       update_args_for_version

components/chef-cli/lib/chef-cli/config.rb

@@ -35,6 +35,10 @@ def telemetry_session_file
         File.join(telemetry_path, "TELEMETRY_SESSION_ID")
       end
 
+      def telemetry_installation_identifier_file
+        File.join(WS_BASE_PATH, "installation_id")
+      end
+
       def error_output_path
         File.join(File.dirname(log.location), "errors.txt")
       end

components/chef-cli/lib/chef-cli/telemeter.rb

@@ -32,6 +32,8 @@ module ChefCLI
   # a main 'timed_capture', and it would be good to see ordering within nested calls.
   class Telemeter
     include Singleton
+    DEFAULT_INSTALLATION_GUID = "00000000-0000-0000-0000-000000000000"
+
     class << self
       extend Forwardable
       def_delegators :instance, :timed_capture, :capture, :commit, :timed_action_capture, :timed_run_capture
@@ -93,15 +95,25 @@ def commit
     end
 
     def make_event_payload(name, data)
-      properties = {
-        # We will submit this payload in a future run, so capture the time of actual execution:
-        run_timestamp: run_timestamp,
-        # This lets us filter out testing/dev actions, which may not
-        # follow customer usage patterns:
-        telemetry_mode:  ChefCLI::Config.telemetry.dev ? "dev" : "prod",
-        host_platform: host_platform,
+      {
+        event: name,
+        properties: {
+          installation_id: installation_id,
+          run_timestamp: run_timestamp,
+          host_platform: host_platform,
+          event_data: data
+        }
       }
-      { event: name, properties: properties.merge(data) }
+    end
+
+    def installation_id
+      @installation_id ||=
+        begin
+          File.read(ChefCLI::Config.telemetry_installation_identifier_file).chomp
+        rescue
+          ChefCLI::Log.info "could not read #{ChefCLI::Config.telemetry_installation_identifier_file} - using default id"
+          DEFAULT_INSTALLATION_GUID
+        end
     end
 
     # For testing.
@@ -125,7 +137,8 @@ def host_platform
     end
 
     def convert_events_to_session
-      YAML.dump({ "entries" => @events_to_send })
+      YAML.dump({ "version" => ChefCLI::VERSION,
+                  "entries" => @events_to_send })
     end
 
     def write_session(session)

components/chef-cli/lib/chef-cli/telemeter/sender.rb

@@ -7,7 +7,20 @@ module ChefCLI
   class Telemeter
     class Sender
       def run
-        session_files.each { |path| process_session(path) }
+        if ChefCLI::Telemeter.enabled?
+          # dev mode telemetry gets sent to a different location
+          if ChefCLI::Config.telemetry.dev
+            ENV["CHEF_TELEMETRY_ENDPOINT"] ||= "https://telemetry-acceptance.chef.io"
+          end
+          session_files.each { |path| process_session(path) }
+        else
+          # If telemetry is not enabled, just clean up and return. Even though
+          # the telemetry gem will not send if disabled, log output saying that we're submitting
+          # it when it has been disabled can be alarming.
+          ChefCLI::Log.info("Telemetry disabled, clearing any existing session captures without sending them.")
+          session_files.each { |path| FileUtils.rm_rf(path) }
+        end
+        FileUtils.rm_rf(ChefCLI::Config.telemetry_session_file)
         ChefCLI::Log.info("Terminating, nothing more to do.")
       end
 
@@ -30,11 +43,14 @@ def submit_session(content)
         # Each run is one session, so we'll first remove remove the session file
         # to force creating a new one.
         FileUtils.rm_rf(ChefCLI::Config.telemetry_session_file)
+        # We'll use the version captured in the sesion file
+        entries = content["entries"]
+        cli_version = content["version"]
+        total = entries.length
         telemetry = Telemetry.new(product: "chef-workstation-cli",
                                   origin: "command-line",
-                                  product_version: ChefCLI::VERSION,
+                                  product_version: cli_version,
                                   install_context: "omnibus")
-        entries = content["entries"]
         total = entries.length
         entries.each_with_index do |entry, x|
           submit_entry(telemetry, entry, x + 1, total)

components/chef-cli/spec/unit/telemeter/sender_spec.rb

@@ -20,15 +20,64 @@
 
 RSpec.describe ChefCLI::Telemeter::Sender do
   let(:subject) { ChefCLI::Telemeter::Sender.new }
+  let(:enabled_flag) { true }
+  let(:dev_mode) { false }
+  let(:config) { double("config") }
+
+  before do
+    allow(config).to receive(:dev).and_return dev_mode
+    allow(ChefCLI::Config).to receive(:telemetry).and_return config
+    allow(ChefCLI::Telemeter).to receive(:enabled?).and_return enabled_flag
+    # Ensure this is not set for each test:
+    ENV.delete("CHEF_TELEMETRY_ENDPOINT")
+  end
+
   describe "#run" do
     let(:session_files) { %w{file1 file2} }
-    it "submits the session capture for each session file found" do
+    before do
       expect(subject).to receive(:session_files).and_return session_files
-      expect(subject).to receive(:process_session).with("file1")
-      expect(subject).to receive(:process_session).with("file2")
-      subject.run
     end
 
+    context "when telemetry is disabled" do
+      let(:enabled_flag) { false }
+      it "deletes session files without sending" do
+        expect(FileUtils).to receive(:rm_rf).with("file1")
+        expect(FileUtils).to receive(:rm_rf).with("file2")
+        expect(FileUtils).to receive(:rm_rf).with(ChefCLI::Config.telemetry_session_file)
+        expect(subject).to_not receive(:process_session)
+        subject.run
+      end
+    end
+
+    context "when telemetry is enabled" do
+      context "and telemetry dev mode is true" do
+        let(:dev_mode) { true }
+        let(:session_files) { [] } # Ensure we don't send anything without mocking :allthecalls:
+        context "and a custom telemetry endpoint is not set" do
+          it "configures the environment to submit to the Acceptance telemetry endpoint" do
+            subject.run
+            expect(ENV["CHEF_TELEMETRY_ENDPOINT"]).to eq "https://telemetry-acceptance.chef.io"
+          end
+        end
+
+        context "and a custom telemetry endpoint is already set" do
+          before do
+            ENV["CHEF_TELEMETRY_ENDPOINT"] = "https://localhost"
+          end
+          it "should not overwrite the custom value" do
+            subject.run
+            expect(ENV["CHEF_TELEMETRY_ENDPOINT"]).to eq "https://localhost"
+          end
+        end
+      end
+
+      it "submits the session capture for each session file found" do
+        expect(subject).to receive(:process_session).with("file1")
+        expect(subject).to receive(:process_session).with("file2")
+        expect(FileUtils).to receive(:rm_rf).with(ChefCLI::Config.telemetry_session_file)
+        subject.run
+      end
+    end
   end
 
   describe "#session_files" do
@@ -41,8 +90,8 @@
 
   describe "process_session" do
     it "loads the sesion and submits it" do
-      expect(subject).to receive(:load_and_clear_session).with("file1").and_return({ "entries" => [] })
-      expect(subject).to receive(:submit_session).with({ "entries" => [] })
+      expect(subject).to receive(:load_and_clear_session).with("file1").and_return({ "version" => "1.0.0", "entries" => [] })
+      expect(subject).to receive(:submit_session).with({ "version" => "1.0.0", "entries" => [] })
       subject.process_session("file1")
     end
   end
@@ -55,7 +104,8 @@
       expect(Telemetry).to receive(:new).and_return telemetry
       expect(subject).to receive(:submit_entry).with(telemetry, { "event" => "action1" }, 1, 2)
       expect(subject).to receive(:submit_entry).with(telemetry, { "event" => "action2" }, 2, 2)
-      subject.submit_session( { "entries" => [ { "event" => "action1" }, { "event" => "action2" } ] } )
+      subject.submit_session( { "version" => "1.0.0",
+                                "entries" => [ { "event" => "action1" }, { "event" => "action2" } ] } )
     end
   end
 
@@ -66,5 +116,4 @@
       subject.submit_entry(telemetry, { "test" => "this" }, 1, 1)
     end
   end
-
 end

components/chef-cli/spec/unit/telemeter_spec.rb

@@ -19,13 +19,9 @@
 
 RSpec.describe ChefCLI::Telemeter do
   subject { ChefCLI::Telemeter.instance }
-  let(:dev_mode) { true }
-  let(:config) { double("config") }
   let(:host_platform) { "linux" }
 
   before do
-    allow(config).to receive(:dev).and_return dev_mode
-    allow(ChefCLI::Config).to receive(:telemetry).and_return config
     allow(subject).to receive(:host_platform).and_return host_platform
   end
 
@@ -103,6 +99,38 @@
     end
   end
 
+  context "::enabled?" do
+    let(:enabled_flag) { false }
+    let(:config) { double("config") }
+    before do
+      allow(ChefCLI::Config).to receive(:telemetry).and_return(config)
+      allow(config).to receive(:enable).and_return(enabled_flag)
+    end
+
+    context "when config value is enabled" do
+      let(:enabled_flag) { true }
+      context "and CHEF_TELEMETRY_OPT_OUT is not present in env vars" do
+        it "returns false" do
+          ENV.delete("CHEF_TELEMETRY_OPT_OUT")
+          expect(subject.enabled?).to eq true
+        end
+      end
+      context "and CHEF_TELEMETRY_OPT_OUT is present in env vars" do
+        it "returns false" do
+          ENV["CHEF_TELEMETRY_OPT_OUT"] = ""
+          expect(subject.enabled?).to eq false
+        end
+      end
+    end
+
+    context "when config value is disabled" do
+      let(:enabled_flag) { false }
+      it "returns false" do
+        expect(subject.enabled?).to eq false
+      end
+    end
+  end
+
   context "#timed_run_capture" do
     it "invokes timed_capture with run data" do
       expected_data = { arguments: [ "arg1" ] }
@@ -141,17 +169,22 @@
   end
 
   context "#make_event_payload" do
-    context "when event is ':run'" do
-      it "adds expected properties" do
-        payload = subject.make_event_payload(:run, { hello: "world" })
-        expect(payload[:event]).to eq :run
-        props = payload[:properties]
-        expect(props[:telemetry_mode]).to eq "dev"
-        expect(props[:host_platform]).to eq host_platform
-        expect(props[:run_timestamp]).to_not eq nil
-        expect(props[:hello]).to eq "world"
-      end
+    before do
+      allow(subject).to receive(:installation_id).and_return "0000"
     end
 
+    it "adds expected properties" do
+      payload = subject.make_event_payload(:run, { hello: "world" })
+      expected_payload = {
+        event: :run,
+        properties: {
+          installation_id: "0000",
+          run_timestamp: subject.run_timestamp,
+          host_platform: host_platform,
+          event_data:  { hello: "world" }
+        }
+      }
+      expect(payload).to eq expected_payload
+    end
   end
 end