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

remote_settings: send metainformation with every request to S3 #594

Merged
merged 2 commits into from
Jul 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ Airbrake Ruby Changelog

### master

* Started sending information about notifier name & version, operating system &
language with every remote config GET request
([#594](https://github.com/airbrake/airbrake-ruby/pull/594))

### [v5.0.0.rc.1][v5.0.0.rc.1] (July 13, 2020)

* Added the `error_notifications` option (enabled by default). This option
Expand Down
9 changes: 1 addition & 8 deletions lib/airbrake-ruby/notice.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,12 @@ module Airbrake
#
# @since v1.0.0
class Notice
# @return [Hash{Symbol=>String}] the information about the notifier library
NOTIFIER = {
name: 'airbrake-ruby'.freeze,
version: Airbrake::AIRBRAKE_RUBY_VERSION,
url: 'https://github.com/airbrake/airbrake-ruby'.freeze,
}.freeze

# @return [Hash{Symbol=>String,Hash}] the information to be displayed in the
# Context tab in the dashboard
CONTEXT = {
os: RUBY_PLATFORM,
language: "#{RUBY_ENGINE}/#{RUBY_VERSION}".freeze,
notifier: NOTIFIER,
notifier: Airbrake::NOTIFIER_INFO,
}.freeze

# @return [Integer] the maxium size of the JSON payload in bytes
Expand Down
17 changes: 16 additions & 1 deletion lib/airbrake-ruby/remote_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ class RemoteSettings
'../../config/config.json',
).freeze

# @return [Hash{Symbol=>String}] metadata to be attached to every GET
# request
QUERY_PARAMS = URI.encode_www_form(
notifier_name: Airbrake::NOTIFIER_INFO[:name],
notifier_version: Airbrake::NOTIFIER_INFO[:version],
os: RUBY_PLATFORM,
language: "#{RUBY_ENGINE}/#{RUBY_VERSION}".freeze,
).freeze

# Polls remote config of the given project.
#
# @param [Integer] project_id
Expand Down Expand Up @@ -84,7 +93,7 @@ def stop_polling
def fetch_config
response = nil
begin
response = Net::HTTP.get(URI(@data.config_route))
response = Net::HTTP.get(build_config_uri)
rescue StandardError => ex
logger.error(ex)
return {}
Expand All @@ -108,6 +117,12 @@ def fetch_config
json
end

def build_config_uri
uri = URI(@data.config_route)
uri.query = QUERY_PARAMS
uri
end

def load_config
config_dir = File.dirname(CONFIG_DUMP_PATH)
Dir.mkdir(config_dir) unless File.directory?(config_dir)
Expand Down
2 changes: 1 addition & 1 deletion lib/airbrake-ruby/sync_sender.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def build_request_body(req, data)
req['Authorization'] = "Bearer #{@config.project_key}"
req['Content-Type'] = CONTENT_TYPE
req['User-Agent'] =
"#{Airbrake::Notice::NOTIFIER[:name]}/#{Airbrake::AIRBRAKE_RUBY_VERSION}" \
"#{Airbrake::NOTIFIER_INFO[:name]}/#{Airbrake::AIRBRAKE_RUBY_VERSION}" \
" Ruby/#{RUBY_VERSION}"

req
Expand Down
10 changes: 10 additions & 0 deletions lib/airbrake-ruby/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,15 @@
# More information: http://semver.org/
module Airbrake
# @return [String] the library version
# @api public
AIRBRAKE_RUBY_VERSION = '5.0.0.rc.1'.freeze

# @return [Hash{Symbol=>String}] the information about the notifier library
# @since ?.?.?
# @api public
NOTIFIER_INFO = {
name: 'airbrake-ruby'.freeze,
version: Airbrake::AIRBRAKE_RUBY_VERSION,
url: 'https://github.com/airbrake/airbrake-ruby'.freeze,
}.freeze
end
54 changes: 36 additions & 18 deletions spec/remote_settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@
let(:config_path) { described_class::CONFIG_DUMP_PATH }
let(:config_dir) { File.dirname(config_path) }

before do
stub_request(:get, endpoint).to_return(status: 200, body: body.to_json)
let!(:stub) do
stub_request(:get, Regexp.new(endpoint))
.to_return(status: 200, body: body.to_json)
end

before do
# Do not create config dumps on disk.
allow(Dir).to receive(:mkdir).with(config_dir)
allow(File).to receive(:write).with(config_path, anything)
Expand All @@ -52,7 +55,7 @@
sleep(0.2)
remote_settings.stop_polling

expect(a_request(:get, endpoint)).to have_been_made.once
expect(stub).to have_been_requested.once
end

it "yields the config to the block twice" do
Expand All @@ -63,7 +66,7 @@
sleep(0.2)
remote_settings.stop_polling

expect(a_request(:get, endpoint)).to have_been_made.once
expect(stub).to have_been_requested.once
end

context "when config loading fails" do
Expand All @@ -77,7 +80,7 @@
sleep(0.2)
remote_settings.stop_polling

expect(a_request(:get, endpoint)).to have_been_made.once
expect(stub).to have_been_requested.once
end
end
end
Expand All @@ -88,7 +91,18 @@
sleep(0.1)
remote_settings.stop_polling

expect(a_request(:get, endpoint)).to have_been_made.at_least_once
expect(stub).to have_been_requested.at_least_once
end

it "sends params about the environment with the request" do
remote_settings = described_class.poll(project_id) {}
sleep(0.1)
remote_settings.stop_polling

stub_with_query_params = stub.with(
query: URI.decode_www_form(described_class::QUERY_PARAMS).to_h,
)
expect(stub_with_query_params).to have_been_requested.at_least_once
end

it "fetches remote settings" do
Expand Down Expand Up @@ -118,7 +132,7 @@
sleep(0.1)
remote_settings.stop_polling

expect(a_request(:get, endpoint)).not_to have_been_made
expect(stub).not_to have_been_requested
expect(settings.interval).to eq(600)
end
end
Expand All @@ -136,14 +150,15 @@
sleep(0.1)
remote_settings.stop_polling

expect(a_request(:get, endpoint)).to have_been_made.once
expect(stub).to have_been_requested.once
expect(settings.interval).to eq(600)
end
end

context "when API returns an XML response" do
before do
stub_request(:get, endpoint).to_return(status: 200, body: '<?xml ...')
let!(:stub) do
stub_request(:get, Regexp.new(endpoint))
.to_return(status: 200, body: '<?xml ...')
end

it "doesn't update settings data" do
Expand All @@ -154,20 +169,23 @@
sleep(0.1)
remote_settings.stop_polling

expect(a_request(:get, endpoint)).to have_been_made.once
expect(stub).to have_been_requested.once
expect(settings.interval).to eq(600)
end
end

context "when a config route is specified in the returned data" do
let(:new_endpoint) { 'http://example.com' }
let(:new_endpoint) do
"http://example.com"
end

let(:body) do
{ 'config_route' => new_endpoint, 'poll_sec' => 0.1 }
end

before do
stub_request(:get, new_endpoint).to_return(status: 200, body: body.to_json)
let!(:new_stub) do
stub_request(:get, Regexp.new(new_endpoint))
.to_return(status: 200, body: body.to_json)
end

it "makes the next request to the specified config route" do
Expand All @@ -176,8 +194,8 @@

remote_settings.stop_polling

expect(a_request(:get, endpoint)).to have_been_made.once
expect(a_request(:get, new_endpoint)).to have_been_made.once
expect(stub).to have_been_requested.once
expect(new_stub).to have_been_requested.once
end
end
end
Expand All @@ -191,7 +209,7 @@
sleep(0.2)
remote_settings.stop_polling

expect(a_request(:get, endpoint)).to have_been_made.once
expect(stub).to have_been_requested.once
end

context "when config dumping fails" do
Expand All @@ -205,7 +223,7 @@
sleep(0.2)
remote_settings.stop_polling

expect(a_request(:get, endpoint)).to have_been_made.once
expect(stub).to have_been_requested.once
end
end
end
Expand Down