utils/analytics: general cleanup.
We have plans to add analytics for commands and `brew test-bot` This requires a certain amount of refactoring which I've done here. There was also a bunch of legacy `*_influx_?` usage from when we used both InfluxDB and Google Analytics that made sense to clean up and excessive indirection.
This commit is contained in:
Mike McQuaid
parent
8a852cf8c8
commit
9259c345cc
@ -112,7 +112,7 @@ module Cask
|
||||
install_artifacts(predecessor: predecessor)
|
||||
|
||||
if (tap = @cask.tap) && tap.should_report_analytics?
|
||||
::Utils::Analytics.report_event(:cask_install, package_name: @cask.token, tap_name: tap.name,
|
||||
::Utils::Analytics.report_package_event(:cask_install, package_name: @cask.token, tap_name: tap.name,
|
||||
on_request: true)
|
||||
end
|
||||
|
||||
|
||||
@ -419,7 +419,7 @@ class FormulaInstaller
|
||||
oh1 "Installing #{Formatter.identifier(formula.full_name)} #{options}".strip if show_header?
|
||||
|
||||
if (tap = formula.tap) && tap.should_report_analytics?
|
||||
Utils::Analytics.report_event(:formula_install, package_name: formula.name, tap_name: tap.name,
|
||||
Utils::Analytics.report_package_event(:formula_install, package_name: formula.name, tap_name: tap.name,
|
||||
on_request: installed_on_request?, options: options)
|
||||
end
|
||||
|
||||
|
||||
@ -8,47 +8,47 @@ RSpec.describe Utils::Analytics do
|
||||
described_class.clear_cache
|
||||
end
|
||||
|
||||
describe "::default_tags_influx" do
|
||||
describe "::default_package_tags" do
|
||||
let(:ci) { ", CI" if ENV["CI"] }
|
||||
|
||||
it "returns OS_VERSION and prefix when HOMEBREW_PREFIX is a custom prefix on intel" do
|
||||
expect(Homebrew).to receive(:default_prefix?).and_return(false).at_least(:once)
|
||||
expect(described_class.default_tags_influx).to have_key(:prefix)
|
||||
expect(described_class.default_tags_influx[:prefix]).to eq "custom-prefix"
|
||||
expect(described_class.default_package_tags).to have_key(:prefix)
|
||||
expect(described_class.default_package_tags[:prefix]).to eq "custom-prefix"
|
||||
end
|
||||
|
||||
it "returns OS_VERSION, ARM and prefix when HOMEBREW_PREFIX is a custom prefix on arm" do
|
||||
expect(Homebrew).to receive(:default_prefix?).and_return(false).at_least(:once)
|
||||
expect(described_class.default_tags_influx).to have_key(:arch)
|
||||
expect(described_class.default_tags_influx[:arch]).to eq HOMEBREW_PHYSICAL_PROCESSOR
|
||||
expect(described_class.default_tags_influx).to have_key(:prefix)
|
||||
expect(described_class.default_tags_influx[:prefix]).to eq "custom-prefix"
|
||||
expect(described_class.default_package_tags).to have_key(:arch)
|
||||
expect(described_class.default_package_tags[:arch]).to eq HOMEBREW_PHYSICAL_PROCESSOR
|
||||
expect(described_class.default_package_tags).to have_key(:prefix)
|
||||
expect(described_class.default_package_tags[:prefix]).to eq "custom-prefix"
|
||||
end
|
||||
|
||||
it "returns OS_VERSION, Rosetta and prefix when HOMEBREW_PREFIX is a custom prefix on Rosetta", :needs_macos do
|
||||
expect(Homebrew).to receive(:default_prefix?).and_return(false).at_least(:once)
|
||||
expect(described_class.default_tags_influx).to have_key(:prefix)
|
||||
expect(described_class.default_tags_influx[:prefix]).to eq "custom-prefix"
|
||||
expect(described_class.default_package_tags).to have_key(:prefix)
|
||||
expect(described_class.default_package_tags[:prefix]).to eq "custom-prefix"
|
||||
end
|
||||
|
||||
it "does not include prefix when HOMEBREW_PREFIX is the default prefix" do
|
||||
expect(Homebrew).to receive(:default_prefix?).and_return(true).at_least(:once)
|
||||
expect(described_class.default_tags_influx).to have_key(:prefix)
|
||||
expect(described_class.default_tags_influx[:prefix]).to eq HOMEBREW_PREFIX.to_s
|
||||
expect(described_class.default_package_tags).to have_key(:prefix)
|
||||
expect(described_class.default_package_tags[:prefix]).to eq HOMEBREW_PREFIX.to_s
|
||||
end
|
||||
|
||||
it "includes CI when ENV['CI'] is set" do
|
||||
ENV["CI"] = "1"
|
||||
expect(described_class.default_tags_influx).to have_key(:ci)
|
||||
expect(described_class.default_package_tags).to have_key(:ci)
|
||||
end
|
||||
|
||||
it "includes developer when ENV['HOMEBREW_DEVELOPER'] is set" do
|
||||
expect(Homebrew::EnvConfig).to receive(:developer?).and_return(true)
|
||||
expect(described_class.default_tags_influx).to have_key(:developer)
|
||||
expect(described_class.default_package_tags).to have_key(:developer)
|
||||
end
|
||||
end
|
||||
|
||||
describe "::report_event" do
|
||||
describe "::report_package_event" do
|
||||
let(:f) { formula { url "foo-1.0" } }
|
||||
let(:package_name) { f.name }
|
||||
let(:tap_name) { f.tap.name }
|
||||
@ -59,14 +59,14 @@ RSpec.describe Utils::Analytics do
|
||||
it "returns nil when HOMEBREW_NO_ANALYTICS is true" do
|
||||
ENV["HOMEBREW_NO_ANALYTICS"] = "true"
|
||||
expect(described_class).not_to receive(:report_influx)
|
||||
described_class.report_event(:install, package_name: package_name, tap_name: tap_name,
|
||||
described_class.report_package_event(:install, package_name: package_name, tap_name: tap_name,
|
||||
on_request: on_request, options: options)
|
||||
end
|
||||
|
||||
it "returns nil when HOMEBREW_NO_ANALYTICS_THIS_RUN is true" do
|
||||
ENV["HOMEBREW_NO_ANALYTICS_THIS_RUN"] = "true"
|
||||
expect(described_class).not_to receive(:report_influx)
|
||||
described_class.report_event(:install, package_name: package_name, tap_name: tap_name,
|
||||
described_class.report_package_event(:install, package_name: package_name, tap_name: tap_name,
|
||||
on_request: on_request, options: options)
|
||||
end
|
||||
|
||||
@ -76,7 +76,7 @@ RSpec.describe Utils::Analytics do
|
||||
ENV["HOMEBREW_ANALYTICS_DEBUG"] = "true"
|
||||
expect(described_class).to receive(:report_influx)
|
||||
|
||||
described_class.report_event(:install, package_name: package_name, tap_name: tap_name,
|
||||
described_class.report_package_event(:install, package_name: package_name, tap_name: tap_name,
|
||||
on_request: on_request, options: options)
|
||||
end
|
||||
end
|
||||
@ -85,16 +85,16 @@ RSpec.describe Utils::Analytics do
|
||||
ENV.delete("HOMEBREW_NO_ANALYTICS_THIS_RUN")
|
||||
ENV.delete("HOMEBREW_NO_ANALYTICS")
|
||||
ENV["HOMEBREW_ANALYTICS_DEBUG"] = "true"
|
||||
expect(described_class).to receive(:report_influx).with(:install, hash_including(package_name: package_name,
|
||||
on_request: on_request)).once
|
||||
described_class.report_event(:install, package_name: package_name, tap_name: tap_name,
|
||||
expect(described_class).to receive(:report_influx).with(:install, hash_including(on_request:),
|
||||
hash_including(package: package_name)).once
|
||||
described_class.report_package_event(:install, package_name: package_name, tap_name: tap_name,
|
||||
on_request: on_request, options: options)
|
||||
end
|
||||
end
|
||||
|
||||
describe "::report_influx" do
|
||||
let(:f) { formula { url "foo-1.0" } }
|
||||
let(:package_name) { f.name }
|
||||
let(:package) { f.name }
|
||||
let(:tap_name) { f.tap.name }
|
||||
let(:on_request) { false }
|
||||
let(:options) { "--HEAD" }
|
||||
@ -104,8 +104,7 @@ RSpec.describe Utils::Analytics do
|
||||
ENV.delete("HOMEBREW_NO_ANALYTICS")
|
||||
ENV["HOMEBREW_ANALYTICS_DEBUG"] = "true"
|
||||
expect(described_class).to receive(:deferred_curl).once
|
||||
described_class.report_influx(:install, package_name: package_name, tap_name: tap_name, on_request: on_request,
|
||||
options: options)
|
||||
described_class.report_influx(:install, { on_request: }, { package:, tap_name: })
|
||||
end
|
||||
end
|
||||
|
||||
@ -116,13 +115,13 @@ options: options)
|
||||
|
||||
it "reports event if BuildError raised for a formula with a public remote repository" do
|
||||
allow_any_instance_of(Tap).to receive(:custom_remote?).and_return(false)
|
||||
expect(described_class).to respond_to(:report_event)
|
||||
expect(described_class).to respond_to(:report_package_event)
|
||||
described_class.report_build_error(err)
|
||||
end
|
||||
|
||||
it "does not report event if BuildError raised for a formula with a private remote repository" do
|
||||
allow_any_instance_of(Tap).to receive(:private?).and_return(true)
|
||||
expect(described_class).not_to receive(:report_event)
|
||||
expect(described_class).not_to receive(:report_package_event)
|
||||
described_class.report_build_error(err)
|
||||
end
|
||||
end
|
||||
@ -132,7 +131,7 @@ options: options)
|
||||
let(:f) { instance_double(Formula, name: "foo", path: "blah", tap: nil) }
|
||||
|
||||
it "does not report event if BuildError is raised" do
|
||||
expect(described_class).not_to receive(:report_event)
|
||||
expect(described_class).not_to receive(:report_package_event)
|
||||
described_class.report_build_error(err)
|
||||
end
|
||||
end
|
||||
@ -143,7 +142,7 @@ options: options)
|
||||
|
||||
it "does not report event if BuildError is raised" do
|
||||
allow_any_instance_of(Pathname).to receive(:directory?).and_return(false)
|
||||
expect(described_class).not_to receive(:report_event)
|
||||
expect(described_class).not_to receive(:report_package_event)
|
||||
described_class.report_build_error(err)
|
||||
end
|
||||
end
|
||||
|
||||
@ -20,33 +20,28 @@ module Utils
|
||||
include Context
|
||||
|
||||
sig {
|
||||
params(measurement: Symbol, package_name: String, tap_name: String, on_request: T::Boolean,
|
||||
options: String).void
|
||||
params(measurement: Symbol,
|
||||
tags: T::Hash[Symbol, T.any(T::Boolean, String)],
|
||||
fields: T::Hash[Symbol, T.any(T::Boolean, String)]).void
|
||||
}
|
||||
def report_influx(measurement, package_name:, tap_name:, on_request:, options:)
|
||||
# ensure on_request is a boolean
|
||||
on_request = on_request ? true : false
|
||||
|
||||
# ensure options are removed (by `.compact` below) if empty
|
||||
options = nil if options.blank?
|
||||
def report_influx(measurement, tags, fields)
|
||||
return if not_this_run? || disabled?
|
||||
|
||||
# Tags are always implicitly strings and must have low cardinality.
|
||||
tags = default_tags_influx.merge(on_request: on_request)
|
||||
.map { |k, v| "#{k}=#{v}" }
|
||||
.join(",")
|
||||
tags_string = tags.map { |k, v| "#{k}=#{v}" }
|
||||
.join(",")
|
||||
|
||||
# Fields need explicitly wrapped with quotes and can have high cardinality.
|
||||
fields = default_fields_influx.merge(package: package_name, tap_name: tap_name, options: options)
|
||||
.compact
|
||||
.map { |k, v| %Q(#{k}="#{v}") }
|
||||
.join(",")
|
||||
fields_string = fields.compact
|
||||
.map { |k, v| %Q(#{k}="#{v}") }
|
||||
.join(",")
|
||||
|
||||
args = [
|
||||
"--max-time", "3",
|
||||
"--header", "Authorization: Token #{INFLUX_TOKEN}",
|
||||
"--header", "Content-Type: text/plain; charset=utf-8",
|
||||
"--header", "Accept: application/json",
|
||||
"--data-binary", "#{measurement},#{tags} #{fields} #{Time.now.to_i}"
|
||||
"--data-binary", "#{measurement},#{tags_string} #{fields_string} #{Time.now.to_i}"
|
||||
]
|
||||
|
||||
# Second precision is highest we can do and has the lowest performance cost.
|
||||
@ -72,29 +67,27 @@ module Utils
|
||||
params(measurement: Symbol, package_name: String, tap_name: String,
|
||||
on_request: T::Boolean, options: String).void
|
||||
}
|
||||
def report_event(measurement, package_name:, tap_name:, on_request:, options: "")
|
||||
report_influx_event(measurement, package_name: package_name, tap_name: tap_name, on_request: on_request,
|
||||
options: options)
|
||||
end
|
||||
|
||||
sig {
|
||||
params(measurement: Symbol, package_name: String, tap_name: String, on_request: T::Boolean,
|
||||
options: String).void
|
||||
}
|
||||
def report_influx_event(measurement, package_name:, tap_name:, on_request: false, options: "")
|
||||
def report_package_event(measurement, package_name:, tap_name:, on_request: false, options: "")
|
||||
return if not_this_run? || disabled?
|
||||
|
||||
report_influx(measurement, package_name: package_name, tap_name: tap_name, on_request: on_request,
|
||||
options: options)
|
||||
# ensure on_request is a boolean
|
||||
on_request = on_request ? true : false
|
||||
|
||||
# ensure options are removed (by `.compact` below) if empty
|
||||
options = nil if options.blank?
|
||||
|
||||
# Tags must have low cardinality.
|
||||
tags = default_package_tags.merge(on_request: on_request)
|
||||
|
||||
# Fields can have high cardinality.
|
||||
fields = default_package_fields.merge(package: package_name, tap_name: tap_name, options: options)
|
||||
.compact
|
||||
|
||||
report_influx(measurement, tags, fields)
|
||||
end
|
||||
|
||||
sig { params(exception: BuildError).void }
|
||||
def report_build_error(exception)
|
||||
report_influx_error(exception)
|
||||
end
|
||||
|
||||
sig { params(exception: BuildError).void }
|
||||
def report_influx_error(exception)
|
||||
return if not_this_run? || disabled?
|
||||
|
||||
formula = exception.formula
|
||||
@ -105,7 +98,7 @@ options: options)
|
||||
return unless tap.should_report_analytics?
|
||||
|
||||
options = exception.options.to_a.map(&:to_s).join(" ")
|
||||
report_influx_event(:build_error, package_name: formula.name, tap_name: tap.name, options: options)
|
||||
report_package_event(:build_error, package_name: formula.name, tap_name: tap.name, options: options)
|
||||
end
|
||||
|
||||
def influx_message_displayed?
|
||||
@ -285,13 +278,13 @@ options: options)
|
||||
end
|
||||
|
||||
def clear_cache
|
||||
remove_instance_variable(:@default_tags_influx) if instance_variable_defined?(:@default_tags_influx)
|
||||
remove_instance_variable(:@default_fields_influx) if instance_variable_defined?(:@default_fields_influx)
|
||||
remove_instance_variable(:@default_package_tags) if instance_variable_defined?(:@default_package_tags)
|
||||
remove_instance_variable(:@default_package_fields) if instance_variable_defined?(:@default_package_fields)
|
||||
end
|
||||
|
||||
sig { returns(T::Hash[Symbol, String]) }
|
||||
def default_tags_influx
|
||||
@default_tags_influx ||= begin
|
||||
def default_package_tags
|
||||
@default_package_tags ||= begin
|
||||
# Only display default prefixes to reduce cardinality and improve privacy
|
||||
prefix = Homebrew.default_prefix? ? HOMEBREW_PREFIX.to_s : "custom-prefix"
|
||||
|
||||
@ -311,8 +304,8 @@ options: options)
|
||||
# remove os_version starting with " or number
|
||||
# remove macOS patch release
|
||||
sig { returns(T::Hash[Symbol, String]) }
|
||||
def default_fields_influx
|
||||
@default_fields_influx ||= begin
|
||||
def default_package_fields
|
||||
@default_package_fields ||= begin
|
||||
version = if (match_data = HOMEBREW_VERSION.match(/^[\d.]+/))
|
||||
suffix = "-dev" if HOMEBREW_VERSION.include?("-")
|
||||
match_data[0] + suffix.to_s
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user