Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PDK-1342) Submit PDK analytics events #668

Merged
merged 1 commit into from
Jun 5, 2019
Merged

Conversation

rodjek
Copy link
Contributor

@rodjek rodjek commented May 28, 2019

OK, I think this implements analytics submission for all the items that we're interested in according to our spec.

Common values that are submitted for every analytics event:

  • A random non-identifying user ID (shared with Bolt analytics if installed and enabled)
  • PDK version
  • PDK installation method (package or gem)
  • Operating system

We submit a "screen view" for every successful CLI command invocation that records:

  • The command name
  • The version of Ruby used to execute the PDK command
  • The anonymised command options and arguments
  • All arguments are redacted
  • All non-boolean option values are redacted (except --puppet-version and --pe-version)
  • The output formats for the command
  • PDK_* environment variables and their values (if set)

Invalid commands are submitted as a distinct analytics events with the arguments and option values redacted.

Whenever a template repo is used, we submit an event recording if the template is default or custom (n.b. we do not record the path to the template repo itself). Additionally, if the template repo being used is the default one, we submit events for each file rendered, recording if the file is unmanaged, deleted, customized or default (n.b. in the event of a customized file, we do not record what was changed, only that it was changed via .sync.yml overrides).

Given the concerns of GDPR compliance you note that we're very carefully and strictly testing the analytics calls to ensure that no unexpected data is accidentally passed in.

@coveralls
Copy link

coveralls commented May 28, 2019

Coverage Status

Coverage decreased (-0.2%) to 93.093% when pulling b83d748 on rodjek:pdk-1342 into 6a6a139 on puppetlabs:master.

@rodjek rodjek force-pushed the pdk-1342 branch 2 times, most recently from 6a430ab to 6ae5be8 Compare May 29, 2019 02:41
Copy link
Contributor

@scotje scotje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I'm excited to start seeing this data come through!

I mostly just have some comments and questions although I did spot at least one thing that seemed like a bug with the .sync.yml customization reporting.

lib/pdk/cli.rb Show resolved Hide resolved
lib/pdk/module/templatedir.rb Outdated Show resolved Hide resolved
spec/unit/pdk/cli/build_spec.rb Show resolved Hide resolved
end

context 'and provided no flags' do
after(:each) do
PDK::CLI.run(['convert'])
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would these be better as subject blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, as the CLI run isn't the subject of the tests, it's just something that we have to do to trigger the logic being tested.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel super strongly about the subject thing but I do think this happening in the after hook obfuscates things a bit.

@rodjek rodjek merged commit 48bbbf1 into puppetlabs:master Jun 5, 2019
@rodjek rodjek deleted the pdk-1342 branch June 5, 2019 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants