Telemetry - determine run context from stack introspection #4907

clintoncwolfe · 2020-02-11T21:06:08Z

Description

Provides ability to determine how InSpec is being run - whether via the CLI, via the Audit cookbook, or via kitchen-inspec, all via looking at the call stack.

Here is some of the stack data captured :

During an audit run under chef-solo (manipulated to include indices from the outermost frame):

"-29:[/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/inspec-core-4.18.85/lib/inspec/profile.rb:208:in `block in collect_tests'",
 "-28:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/inspec-core-4.18.85/lib/inspec/profile.rb:204:in `each'",
 "-27:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/inspec-core-4.18.85/lib/inspec/profile.rb:204:in `collect_tests'",
 "-26:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/inspec-core-4.18.85/lib/inspec/runner.rb:118:in`block in load'",
 "-25:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/inspec-core-4.18.85/lib/inspec/runner.rb:101:in `each'",
 "-24:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/inspec-core-4.18.85/lib/inspec/runner.rb:101:in `load'",
 "-23:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/inspec-core-4.18.85/lib/inspec/runner.rb:129:in `run'", "-22:/tmp/kitchen/cache/cookbooks/audit/files/default/handler/audit_report.rb:174:in `call'", "-21:/tmp/kitchen/cache/cookbooks/audit/files/default/handler/audit_report.rb:82:in `report'",
 "-20:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/handler.rb:256:in `run_report_unsafe'", "-19:/tmp/kitchen/cache/cookbooks/audit/files/default/handler/audit_report.rb:100:in `run_report_safely'",
 "-18:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/handler.rb:123:in `block in run_report_handlers'",
 "-17:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/handler.rb:121:in `each'",
 "-16:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/handler.rb:121:in `run_report_handlers'",
 "-15:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/handler.rb:133:in `block in <class:Handler>'",
 "-14:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/client.rb:417:in `block in run_completed_successfully'",
 "-13:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/client.rb:416:in `each'",
 "-12:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/client.rb:416:in `run_completed_successfully'",
 "-11:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/client.rb:292:in `run'",
 "-10:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/application.rb:320:in `run_with_graceful_exit_option'",
 "-9:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/application.rb:296:in `block in run_chef_client'",
 "-8:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/local_mode.rb:42:in `with_server_connectivity'",
 "-7:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/application.rb:279:in `run_chef_client'",
 "-6:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/application/base.rb:330:in `run_application'",
 "-5:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/application.rb:69:in`run'",
 "-4:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-15.8.23/lib/chef/application/solo.rb:59:in `run'",
 "-3:/opt/chef/embedded/lib/ruby/gems/2.6.0/gems/chef-bin-15.8.23/bin/chef-solo:24:in `<top (required)>'",
 "-2:/opt/chef/bin/chef-solo:177:in `load'",
 "-1:/opt/chef/bin/chef-solo:177:in `<main>'"

Related Issue

Needed for #4900

Types of changes

Bug fix (non-breaking change which fixes an issue)
New content (non-breaking change)
Breaking change (a content change which would break existing functionality or processes)

Checklist:

I have read the CONTRIBUTING document.

Aha! Link: https://chef.aha.io/features/SH-2163

codeclimate · 2020-02-11T21:08:21Z

Code Climate has analyzed commit 6051618 and detected 5 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	5

View more on Code Climate.

btm · 2020-02-18T22:13:01Z

lib/inspec/utils/telemetry/run_context_probe.rb

+
+      def self.audit_cookbook?(stack)
+        stack_match(stack: stack[-21..-11], path: "chef/handler", label: "run_report_handlers") &&
+        stack_match(stack: stack[-26..-16], path: "handler/audit_report", label: "report")


These fixed numbers like -26 feel likely to change and break, is that true? What happens if they do? What are the consequences of false positives/negatives?

At the least I think we'd want unit tests that run against a captured stack trace embedded in the tests. That would make it easier to understand what is being picked out by seeing the same. Of course that wouldn't detect any changes to the stack, so we'd probably want an integration tests for each of these, no?

I've removed the fixed frame ranges. I agree they would be brittle.

The consequence of a false positive would be miscategorizing a run in the telemetry data. A false negative would likely result in an "unknown" result.

I've added integration tests that exercise each of the three outcomes.

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

…tKitchen Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

clintoncwolfe · 2020-05-26T01:05:55Z

@james-stocks @Schwad Two of the tests on this require test-kitchen. We don't currently run any test-kitchen suites automatically, AFAIK, so I didn't add anything to BuildKite or Rake to enable that; let me know how you'd like that handled.

james-stocks · 2020-06-05T11:09:43Z

lib/inspec/cli.rb

@@ -387,11 +393,6 @@ def version
  end
  map %w{-v --version} => :version

-  desc "nothing", "does nothing"


It would be best to delete these lines in a separate commit.

james-stocks · 2020-06-05T11:17:35Z

Regarding test-kitchen, I'm not comfortable with the general approach of adding a test command to the CLI and having the integration suite depend on both kitchen and the audit cookbook.
It feels like this is somewhat jumping the gun on properly defining and making a start on end-to-end testing.

I'd prefer that the unit and integration tests stick to static fixtures and mocking; and exercise only the code contained in this project.

clintoncwolfe · 2020-06-08T16:08:47Z

@james-stocks, in order for this code to be integration tested, which @btm stated was required, it must run in its real environment, which means running under test-kitchen and audit cookbook.

james-stocks

Apologies for the confusion @clintoncwolfe - this looks good to me - as discussed I'll raise an issue for us to get test/integration running in buildkite

clintoncwolfe added Type: New Feature Adds new functionality Component: Telemetry labels Feb 11, 2020

clintoncwolfe requested a review from a team as a code owner February 11, 2020 21:06

clintoncwolfe requested review from zenspider and miah February 11, 2020 21:06

chef-expeditor bot assigned clintoncwolfe Feb 11, 2020

miah approved these changes Feb 11, 2020

View reviewed changes

zenspider requested review from a team and zenspider and removed request for zenspider and a team February 12, 2020 19:40

btm reviewed Feb 18, 2020

View reviewed changes

clintoncwolfe added 9 commits May 25, 2020 21:00

Add stack probe for detecting when run as CLI by Thor

e96ddbf

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

stack probe for test-kitchen

5be4ee9

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

stack probe for audit cookbook

cd9ca19

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

rename to match class name

4122157

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

Remove too-brittle frame ranges on stack locations

45e3421

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

Add a functional test to check Thor run context detection

439c93b

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

Refactor os_prepare cookbook to split out inspec installation

09cfced

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

Test kitchen setup to verify Telemetry RunContext detection under Tes…

43a5fd7

…tKitchen Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

Test kitchen setup to test detection of audit-cookbook

d5be4c2

Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>

clintoncwolfe force-pushed the cw/telemetry-run-context branch from be5448e to d5be4c2 Compare May 26, 2020 01:00

james-stocks reviewed Jun 5, 2020

View reviewed changes

james-stocks approved these changes Jun 17, 2020

View reviewed changes

Schwad merged commit dca57b6 into master Jun 17, 2020

Schwad deleted the cw/telemetry-run-context branch June 17, 2020 17:17

tyler-ball mentioned this pull request Jun 18, 2020

Update InSpec to fix issue with kitchen-inspec chef/chef-workstation#1271

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Telemetry - determine run context from stack introspection #4907

Telemetry - determine run context from stack introspection #4907

clintoncwolfe commented Feb 11, 2020 •

edited by kekaichinose

codeclimate bot commented Feb 11, 2020

btm Feb 18, 2020

clintoncwolfe May 26, 2020

clintoncwolfe commented May 26, 2020

james-stocks Jun 5, 2020

james-stocks commented Jun 5, 2020

clintoncwolfe commented Jun 8, 2020

james-stocks left a comment

Telemetry - determine run context from stack introspection #4907

Telemetry - determine run context from stack introspection #4907

Conversation

clintoncwolfe commented Feb 11, 2020 • edited by kekaichinose

Description

Related Issue

Types of changes

Checklist:

codeclimate bot commented Feb 11, 2020

btm Feb 18, 2020

Choose a reason for hiding this comment

clintoncwolfe May 26, 2020

Choose a reason for hiding this comment

clintoncwolfe commented May 26, 2020

james-stocks Jun 5, 2020

Choose a reason for hiding this comment

james-stocks commented Jun 5, 2020

clintoncwolfe commented Jun 8, 2020

james-stocks left a comment

Choose a reason for hiding this comment

clintoncwolfe commented Feb 11, 2020 •

edited by kekaichinose