Skip to content

Commit

Permalink
[Sampler] rename 'paths' to 'targets', making it generic
Browse files Browse the repository at this point in the history
  • Loading branch information
bmansoob committed Sep 4, 2024
1 parent 6cec5eb commit e9e35aa
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 20 deletions.
16 changes: 8 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ These can be overridden like:
```ruby
AppProfiler.profile_sampler_config = AppProfiler::Sampler::Config.new(
sample_rate: 0.5,
paths: ["/foo"],
targets: ["/foo"],
backends_probability: { stackprof: 0.5, vernier: 0.5 },
backend_configs: {
stackprof: AppProfiler::Sampler::StackprofConfig.new,
Expand All @@ -367,7 +367,7 @@ AppProfiler.profile_sampler_config = AppProfiler::Sampler::Config.new(

Rails.application.config.app_profiler = AppProfiler::Sampler::Config.new(
sample_rate: 0.5,
paths: ["/foo"],
targets: ["/foo"],
backends_probability: { stackprof: 0.5, vernier: 0.5 },
backend_configs: {
stackprof: AppProfiler::Sampler::StackprofConfig.new,
Expand All @@ -380,12 +380,12 @@ All the configuration parameters are optional and have default values. The defau

```ruby

| Sampler Config | Default |
| -------- | ------- |
| sample_rate (0.0 - 1.0) | 0.001 (0.1 %) |
| paths | ['/'] |
| stackprof_probability | 1.0 |
| vernier_probability | 0.0 |
| Sampler Config | Default |
| -------- | ------- |
| sample_rate (0.0 - 1.0) | 0.001 (0.1 %) |
| targets (request paths, job_names etc ) | ['/'] |
| stackprof_probability | 1.0 |
| vernier_probability | 0.0 |

| StackprofConfig | Default |
| -------- | ------- |
Expand Down
1 change: 1 addition & 0 deletions lib/app_profiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ module Viewer
mattr_reader :after_process_queue, default: nil
mattr_accessor :forward_metadata_on_upload, default: false
mattr_accessor :profile_sampler_enabled, default: false
mattr_accessor :profile_sampler_killswitch_on, default: nil
mattr_accessor :profile_sampler_config

class << self
Expand Down
1 change: 1 addition & 0 deletions lib/app_profiler/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def profile_params(params)
return params if params.valid?

return unless AppProfiler.profile_sampler_enabled
return if AppProfiler.profile_sampler_killswitch_on&.call

AppProfiler::Sampler.profile_params(params, AppProfiler.profile_sampler_config)
end
Expand Down
1 change: 1 addition & 0 deletions lib/app_profiler/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class Railtie < Rails::Railtie
AppProfiler.backend = app.config.app_profiler.profiler_backend || :stackprof
AppProfiler.forward_metadata_on_upload = app.config.app_profiler.forward_metadata_on_upload || false
AppProfiler.profile_sampler_enabled = app.config.app_profiler.profile_sampler_enabled || false
AppProfiler.profile_sampler_killswitch_on = app.config.app_profiler.profile_sampler_killswitch_on || nil
AppProfiler.profile_sampler_config = app.config.app_profiler.profile_sampler_config ||
AppProfiler::Sampler::Config.new
end
Expand Down
11 changes: 7 additions & 4 deletions lib/app_profiler/sampler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,21 @@ module AppProfiler
module Sampler
class << self
def profile_params(request, config)
return unless sample?(config, request)
profile_params_for(request.path, config)
end

def profile_params_for(target, config)
return unless sample?(config, target)

get_profile_params(config)
end

private

def sample?(config, request)
def sample?(config, target)
return false if Kernel.rand > config.sample_rate

path = request.path
return false unless config.paths.any? { |p| path.match?(p) }
return false unless config.targets.any? { |t| target.match?(t) }

true
end
Expand Down
13 changes: 8 additions & 5 deletions lib/app_profiler/sampler/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,31 @@
module AppProfiler
module Sampler
class Config
attr_reader :sample_rate, :paths, :cpu_interval, :backends_probability
attr_reader :sample_rate, :targets, :cpu_interval, :backends_probability

SAMPLE_RATE = 0.001 # 0.1%
PATHS = ["/"]
TARGETS = ["/"]
BACKEND_PROBABILITES = { stackprof: 1.0, vernier: 0.0 }
@backends = {}

def initialize(sample_rate: SAMPLE_RATE,
paths: PATHS,
targets: TARGETS,
backends_probability: BACKEND_PROBABILITES,
backends_config: {
stackprof: StackprofConfig.new,
})
},
paths: nil)

if sample_rate < 0.0 || sample_rate > 1.0
raise ArgumentError, "sample_rate must be between 0 and 1"
end

raise ArgumentError, "mode probabilities must sum to 1" unless backends_probability.values.sum == 1.0

warn("paths is deprecated, use targets instead") if paths

@sample_rate = sample_rate
@paths = paths
@targets = paths || targets
@backends_config = backends_config
@backends_probability = backends_probability
end
Expand Down
17 changes: 16 additions & 1 deletion test/app_profiler/middleware_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ def call(env)
with_google_cloud_storage do
AppProfiler.profile_sampler_config = AppProfiler::Sampler::Config.new(
sample_rate: 1.0,
paths: ["/"],
targets: ["/"],
)

assert_profiles_dumped(0) do
Expand All @@ -399,6 +399,21 @@ def call(env)
end
end

test "request is not sampled when killswitch is on" do
old_killswitch = AppProfiler.profile_sampler_killswitch_on
with_google_cloud_storage do
AppProfiler.profile_sampler_config = AppProfiler::Sampler::Config.new(sample_rate: 1.0)
AppProfiler.profile_sampler_killswitch_on = -> { true }
assert_profiles_dumped(0) do
middleware = AppProfiler::Middleware.new(app_env)
response = middleware.call(mock_request_env(path: "/"))
assert_nil(response[1]["X-Profile-Async"])
end
end
ensure
AppProfiler.profile_sampler_killswitch_on = old_killswitch
end

private

def with_profile_sampler_enabled
Expand Down
2 changes: 1 addition & 1 deletion test/app_profiler/sampler/config_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class ConfigTest < TestCase
test "default config" do
config = Config.new
assert_equal(Config::SAMPLE_RATE, config.sample_rate)
assert_equal(Config::PATHS, config.paths)
assert_equal(Config::TARGETS, config.targets)
assert_equal(Config::BACKEND_PROBABILITES, config.backends_probability)
assert_not_nil(config.get_backend_config(:stackprof))
assert_nil(config.get_backend_config(:vernier))
Expand Down
2 changes: 1 addition & 1 deletion test/app_profiler/sampler/sampler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class SamplerTest < TestCase
Kernel.stubs(:rand).returns(0.1)
config = Config.new(
sample_rate: 1.0,
paths: ["/foo"],
targets: ["/foo"],
)

request = RequestParameters.new(Rack::Request.new({ "PATH_INFO" => "/foo" }))
Expand Down

0 comments on commit e9e35aa

Please sign in to comment.