From 906b3b62616bfe272b1e8e043530ea23f6cdcb98 Mon Sep 17 00:00:00 2001 From: Mikkel Malmberg Date: Fri, 28 Jun 2019 09:22:01 +0200 Subject: [PATCH 1/6] Implement Central Config --- CHANGELOG.md | 6 + docs/configuration.asciidoc | 13 ++ lib/elastic_apm/agent.rb | 11 +- lib/elastic_apm/central_config.rb | 139 ++++++++++++++ .../central_config/cache_control.rb | 34 ++++ lib/elastic_apm/config.rb | 1 + lib/elastic_apm/config/options.rb | 5 + .../central_config/cache_control_spec.rb | 28 +++ spec/elastic_apm/central_config_spec.rb | 176 ++++++++++++++++++ 9 files changed, 411 insertions(+), 2 deletions(-) create mode 100644 lib/elastic_apm/central_config.rb create mode 100644 lib/elastic_apm/central_config/cache_control.rb create mode 100644 spec/elastic_apm/central_config/cache_control_spec.rb create mode 100644 spec/elastic_apm/central_config_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 3819b1c95..e9394fffb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## Unreleased + +### Added + +- Support for APM Agent Configuration via Kibana ([#464](https://github.com/elastic/apm-agent-ruby/pull/464)) + ## 2.9.1 (2019-06-28) ### Fixed diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index 0a75297b3..a4e860b4d 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -213,6 +213,19 @@ Whether or not to attach the request headers to transactions and errors. Whether or not to attach `ENV` from Rack to transactions and errors. +[float] +[[config-central-config]] +==== `central_config` +|============ +| Environment | `Config` key | Default +| `ELASTIC_APM_CENTRAL_CONFIG` | `central_config` | `true` +|============ + +Enable {kibana-ref}/agent-configuration.html[APM Agent Configuration via Kibana]. +If set to `true`, the client will poll the APM Server regularly for new agent configuration. + +NOTE: This feature requires APM Server v7.3 or later and that the APM Server is configured with `kibana.enabled: true`. + [float] [[config-custom-key-filters]] ==== `custom_key_filters` diff --git a/lib/elastic_apm/agent.rb b/lib/elastic_apm/agent.rb index 7f453056c..fd0f7e689 100644 --- a/lib/elastic_apm/agent.rb +++ b/lib/elastic_apm/agent.rb @@ -1,13 +1,17 @@ # frozen_string_literal: true +require 'elastic_apm/error' + require 'elastic_apm/context_builder' require 'elastic_apm/error_builder' require 'elastic_apm/stacktrace_builder' -require 'elastic_apm/error' + +require 'elastic_apm/central_config' require 'elastic_apm/transport/base' -require 'elastic_apm/spies' require 'elastic_apm/metrics' +require 'elastic_apm/spies' + module ElasticAPM # rubocop:disable Metrics/ClassLength # @api private @@ -57,6 +61,7 @@ def self.running? !!@instance end + # rubocop:disable Metrics/MethodLength def initialize(config) @config = config @@ -64,6 +69,7 @@ def initialize(config) @context_builder = ContextBuilder.new(config) @error_builder = ErrorBuilder.new(self) + @central_config = CentralConfig.new(config) @transport = Transport::Base.new(config) @instrumenter = Instrumenter.new( config, @@ -71,6 +77,7 @@ def initialize(config) ) { |event| enqueue event } @metrics = Metrics.new(config) { |event| enqueue event } end + # rubocop:enable Metrics/MethodLength attr_reader :config, :transport, :instrumenter, :stacktrace_builder, :context_builder, :error_builder, :metrics diff --git a/lib/elastic_apm/central_config.rb b/lib/elastic_apm/central_config.rb new file mode 100644 index 000000000..23a9bf841 --- /dev/null +++ b/lib/elastic_apm/central_config.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require 'elastic_apm/central_config/cache_control' + +module ElasticAPM + # @api private + class CentralConfig + include Logging + + # @api private + class ResponseError < InternalError + def initialize(response) + @response = response + end + + attr_reader :response + end + class ClientError < ResponseError; end + class ServerError < ResponseError; end + + DEFAULT_MAX_AGE = 300 + + def initialize(config) + @config = config + @modified_options = {} + @service_info = { + 'service.name': config.service_name, + 'service.environment': config.environment + }.to_json + end + + attr_reader :config, :task + + def start + return unless config.central_config? + + fetch_and_apply_config + end + + def fetch_and_apply_config + @task&.cancel + @task = + Concurrent::Promise + .execute(&method(:fetch_config)) + .on_success(&method(:handle_success)) + .rescue(&method(:handle_error)) + end + + def stop + @task&.cancel + end + + # rubocop:disable Metrics/MethodLength + def fetch_config + resp = perform_request + + case resp.status + when 200..299 + resp + when 300..399 + resp + when 400..499 + raise ClientError, resp + when 500..599 + raise ServerError, resp + end + end + # rubocop:enable Metrics/MethodLength + + def assign(update) + update.each_key do |key| + @modified_options[key] ||= config.get(key.to_sym)&.value + end + + @modified_options.each_key do |key| + next if update.key?(key) + update[key] = @modified_options.delete(key) + end + + config.assign(update) + end + + private + + # rubocop:disable Metrics/MethodLength + def handle_success(resp) + update = + if !resp.body.empty? + JSON.parse(resp.body.to_s) + else + {} + end + + assign(update) + + info 'Updated config from APM Server' + + schedule_next_fetch(resp) + + true + rescue Exception => e + error 'Failed to apply remote config, %s', e.inspect + debug { e.backtrace.join('\n') } + end + # rubocop:enable Metrics/MethodLength + + def handle_error(error) + error( + 'Failed fetching config: %s, trying again in %d seconds', + error.response.body, DEFAULT_MAX_AGE + ) + + assign({}) + + schedule_next_fetch(error.response) + end + + def perform_request + Http.post( + config.server_url + '/agent/v1/config/', + body: @service_info, + headers: { etag: 1, content_type: 'application/json' } + ) + end + + def schedule_next_fetch(resp) + seconds = + if (cache_header = resp.headers['Cache-Control']) + CacheControl.new(cache_header).max_age + else + DEFAULT_MAX_AGE + end + + @task = + Concurrent::ScheduledTask + .execute(seconds, &method(:fetch_and_apply_config)) + end + end +end diff --git a/lib/elastic_apm/central_config/cache_control.rb b/lib/elastic_apm/central_config/cache_control.rb new file mode 100644 index 000000000..6e54c3454 --- /dev/null +++ b/lib/elastic_apm/central_config/cache_control.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module ElasticAPM + class CentralConfig + # @api private + class CacheControl + def initialize(value) + @header = value + parse!(value) + end + + attr_reader( + :must_revalidate, + :no_cache, + :no_store, + :no_transform, + :public, + :private, + :proxy_revalidate, + :max_age, + :s_maxage + ) + + private + + def parse!(value) + value.split(',').each do |token| + k, v = token.split('=').map(&:strip) + instance_variable_set(:"@#{k.gsub('-', '_')}", v ? v.to_i : true) + end + end + end + end +end diff --git a/lib/elastic_apm/config.rb b/lib/elastic_apm/config.rb index 29ee82a60..b0b6e8786 100644 --- a/lib/elastic_apm/config.rb +++ b/lib/elastic_apm/config.rb @@ -41,6 +41,7 @@ class Config option :capture_body, type: :string, default: 'off' option :capture_headers, type: :bool, default: true option :capture_env, type: :bool, default: true + option :central_config, type: :bool, default: true option :current_user_email_method, type: :string, default: 'email' option :current_user_id_method, type: :string, default: 'id' option :current_user_username_method, type: :string, default: 'username' diff --git a/lib/elastic_apm/config/options.rb b/lib/elastic_apm/config/options.rb index 9fd001745..7be566ff0 100644 --- a/lib/elastic_apm/config/options.rb +++ b/lib/elastic_apm/config/options.rb @@ -112,6 +112,11 @@ def method_missing(name, *value) end # rubocop:enable Metrics/MethodLength, Metrics/AbcSize + def [](key) + options[key] + end + alias :get :[] + def set(key, value) options.fetch(key.to_sym).set(value) rescue KeyError diff --git a/spec/elastic_apm/central_config/cache_control_spec.rb b/spec/elastic_apm/central_config/cache_control_spec.rb new file mode 100644 index 000000000..f25217c62 --- /dev/null +++ b/spec/elastic_apm/central_config/cache_control_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module ElasticAPM + RSpec.describe CentralConfig::CacheControl do + let(:header) { nil } + subject { described_class.new(header) } + + context 'with max-age' do + let(:header) { 'max-age=300' } + its(:max_age) { should be 300 } + its(:must_revalidate) { should be nil } + end + + context 'with must-revalidate' do + let(:header) { 'must-revalidate' } + its(:max_age) { should be nil } + its(:must_revalidate) { should be true } + end + + context 'with multiple values' do + let(:header) { 'must-revalidate, public, max-age=300' } + its(:max_age) { should be 300 } + its(:must_revalidate) { should be true } + its(:public) { should be true } + its(:private) { should be nil } + end + end +end diff --git a/spec/elastic_apm/central_config_spec.rb b/spec/elastic_apm/central_config_spec.rb new file mode 100644 index 000000000..7bc0dd4d5 --- /dev/null +++ b/spec/elastic_apm/central_config_spec.rb @@ -0,0 +1,176 @@ +# frozen_string_literal: true + +module ElasticAPM + RSpec.describe CentralConfig do + let(:config) { Config.new } + subject { described_class.new(config) } + + describe '#start' do + it 'polls for config' do + req_stub = stub_response(transaction_sample_rate: '0.5') + subject.start + sleep 0.1 + expect(req_stub).to have_been_requested + end + + context 'when disabled' do + let(:config) { Config.new(central_config: false) } + + it 'does nothing' do + req_stub = stub_response(transaction_sample_rate: '0.5') + subject.start + sleep 0.1 + expect(req_stub).to_not have_been_requested + end + end + end + + describe '#fetch_and_apply_config' do + it 'queries APM Server and applies config' do + req_stub = stub_response(transaction_sample_rate: '0.5') + expect(config.logger).to receive(:info) + + subject.fetch_and_apply_config + sleep 0.1 + + expect(req_stub).to have_been_requested + + expect(config.transaction_sample_rate).to eq(0.5) + end + + it 'reverts config if later changed' do + stub_response(transaction_sample_rate: '0.5') + + subject.fetch_and_apply_config + sleep 0.1 + + stub_response({}) + + subject.fetch_and_apply_config + sleep 0.1 + + expect(config.transaction_sample_rate).to eq(1.0) + end + + it 'reverts config if later 404' do + stub_response(transaction_sample_rate: '0.5') + + subject.fetch_and_apply_config + sleep 0.1 + + stub_response('Not found', status: 404) + + subject.fetch_and_apply_config + sleep 0.1 + + expect(config.transaction_sample_rate).to eq(1.0) + end + + context 'when server responds 200 and cache-control' do + it 'schedules a new poll' do + stub_response( + {}, + headers: { 'Cache-Control': 'must-revalidate, max-age=123' } + ) + + subject.fetch_and_apply_config + sleep 0.1 + + expect(subject.task).to be_pending + expect(subject.task.initial_delay).to eq 123 + end + end + + context 'when server responds 304' do + it 'schedules a new poll' do + stub_response( + nil, + status: 304, + headers: { 'Cache-Control': 'must-revalidate, max-age=123' } + ) + + subject.fetch_and_apply_config + sleep 0.1 + + expect(subject.task).to be_pending + expect(subject.task.initial_delay).to eq 123 + end + end + + context 'when server responds 404' do + it 'schedules a new poll' do + stub_response('Not found', status: 404) + + subject.fetch_and_apply_config + sleep 0.1 + + expect(subject.task).to be_pending + expect(subject.task.initial_delay).to eq 300 + end + end + end + + describe '#fetch_config' do + context 'when successful' do + it 'returns response object' do + stub_response(ok: 1) + + expect(subject.fetch_config).to be_a(HTTP::Response) + end + end + + context 'when not found' do + before do + stub_response('Not found', status: 404) + end + + it 'raises an error' do + expect { subject.fetch_config } + .to raise_error(CentralConfig::ClientError) + end + + it 'includes the response' do + begin + subject.fetch_config + rescue CentralConfig::ClientError => e + expect(e.response).to be_a(HTTP::Response) + end + end + end + + context 'when server error' do + it 'raises an error' do + stub_response('Server error', status: 500) + + expect { subject.fetch_config } + .to raise_error(CentralConfig::ServerError) + end + end + end + + describe '#assign' do + it 'updates config' do + subject.assign(transaction_sample_rate: 0.5) + expect(config.transaction_sample_rate).to eq 0.5 + end + + it 'reverts to previous when missing' do + subject.assign(transaction_sample_rate: 0.5) + subject.assign({}) + expect(config.transaction_sample_rate).to eq 1.0 + end + + it 'goes back and forth' do + subject.assign(transaction_sample_rate: 0.5) + subject.assign({}) + subject.assign(transaction_sample_rate: 0.5) + expect(config.transaction_sample_rate).to eq 0.5 + end + end + + def stub_response(body, **opts) + stub_request(:post, 'http://localhost:8200/agent/v1/config/') + .to_return(body: body && body.to_json, **opts) + end + end +end From b116ffaae23570dc7fe5ba04afbb6503c611a446 Mon Sep 17 00:00:00 2001 From: Mikkel Malmberg Date: Mon, 12 Aug 2019 12:20:58 +0200 Subject: [PATCH 2/6] Don't update values when 304, add comments --- lib/elastic_apm/central_config.rb | 23 ++++++++++------------- spec/elastic_apm/central_config_spec.rb | 12 +++++++++++- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/elastic_apm/central_config.rb b/lib/elastic_apm/central_config.rb index 23a9bf841..848d8f550 100644 --- a/lib/elastic_apm/central_config.rb +++ b/lib/elastic_apm/central_config.rb @@ -38,9 +38,7 @@ def start end def fetch_and_apply_config - @task&.cancel - @task = - Concurrent::Promise + Concurrent::Promise .execute(&method(:fetch_config)) .on_success(&method(:handle_success)) .rescue(&method(:handle_error)) @@ -68,10 +66,14 @@ def fetch_config # rubocop:enable Metrics/MethodLength def assign(update) + # For each updated option, store the original value, + # unless already stored update.each_key do |key| @modified_options[key] ||= config.get(key.to_sym)&.value end + # If the new update doesn't set a previously modified option, + # revert it to the original @modified_options.each_key do |key| next if update.key?(key) update[key] = @modified_options.delete(key) @@ -82,16 +84,12 @@ def assign(update) private - # rubocop:disable Metrics/MethodLength def handle_success(resp) - update = - if !resp.body.empty? - JSON.parse(resp.body.to_s) - else - {} - end - - assign(update) + # 304 Not Modified + unless resp.status == 304 + update = JSON.parse(resp.body.to_s) + assign(update) + end info 'Updated config from APM Server' @@ -102,7 +100,6 @@ def handle_success(resp) error 'Failed to apply remote config, %s', e.inspect debug { e.backtrace.join('\n') } end - # rubocop:enable Metrics/MethodLength def handle_error(error) error( diff --git a/spec/elastic_apm/central_config_spec.rb b/spec/elastic_apm/central_config_spec.rb index 7bc0dd4d5..0b51d09bb 100644 --- a/spec/elastic_apm/central_config_spec.rb +++ b/spec/elastic_apm/central_config_spec.rb @@ -82,7 +82,16 @@ module ElasticAPM end context 'when server responds 304' do - it 'schedules a new poll' do + it 'doesn\'t restore config, schedules a new poll' do + stub_response( + { transaction_sample_rate: 0.5 }, + headers: { 'Cache-Control': 'must-revalidate, max-age=0.1' } + ) + + subject.fetch_and_apply_config + + sleep 0.1 + stub_response( nil, status: 304, @@ -94,6 +103,7 @@ module ElasticAPM expect(subject.task).to be_pending expect(subject.task.initial_delay).to eq 123 + expect(config.transaction_sample_rate).to eq 0.5 end end From c7f32cee2ab5548ed2cd29819245651d968a4172 Mon Sep 17 00:00:00 2001 From: Mikkel Malmberg Date: Mon, 12 Aug 2019 13:47:04 +0200 Subject: [PATCH 3/6] Logs --- lib/elastic_apm/central_config.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/elastic_apm/central_config.rb b/lib/elastic_apm/central_config.rb index 848d8f550..43c7f8ffc 100644 --- a/lib/elastic_apm/central_config.rb +++ b/lib/elastic_apm/central_config.rb @@ -84,14 +84,16 @@ def assign(update) private + # rubocop:disable Metrics/MethodLength def handle_success(resp) - # 304 Not Modified - unless resp.status == 304 + if resp.status == 304 + info 'Received 304 Not Modified' + else update = JSON.parse(resp.body.to_s) assign(update) - end - info 'Updated config from APM Server' + info 'Updated config from Kibana' + end schedule_next_fetch(resp) @@ -100,6 +102,7 @@ def handle_success(resp) error 'Failed to apply remote config, %s', e.inspect debug { e.backtrace.join('\n') } end + # rubocop:enable Metrics/MethodLength def handle_error(error) error( From 6f259c202efb7a400a35c39799124b26c205c9d3 Mon Sep 17 00:00:00 2001 From: Mikkel Malmberg Date: Mon, 12 Aug 2019 14:57:59 +0200 Subject: [PATCH 4/6] Wait slightly longer, jruby? --- spec/elastic_apm/central_config_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/elastic_apm/central_config_spec.rb b/spec/elastic_apm/central_config_spec.rb index 0b51d09bb..e64feec43 100644 --- a/spec/elastic_apm/central_config_spec.rb +++ b/spec/elastic_apm/central_config_spec.rb @@ -9,7 +9,7 @@ module ElasticAPM it 'polls for config' do req_stub = stub_response(transaction_sample_rate: '0.5') subject.start - sleep 0.1 + sleep 0.2 expect(req_stub).to have_been_requested end @@ -19,7 +19,7 @@ module ElasticAPM it 'does nothing' do req_stub = stub_response(transaction_sample_rate: '0.5') subject.start - sleep 0.1 + sleep 0.2 expect(req_stub).to_not have_been_requested end end @@ -31,7 +31,7 @@ module ElasticAPM expect(config.logger).to receive(:info) subject.fetch_and_apply_config - sleep 0.1 + sleep 0.2 expect(req_stub).to have_been_requested @@ -42,12 +42,12 @@ module ElasticAPM stub_response(transaction_sample_rate: '0.5') subject.fetch_and_apply_config - sleep 0.1 + sleep 0.2 stub_response({}) subject.fetch_and_apply_config - sleep 0.1 + sleep 0.2 expect(config.transaction_sample_rate).to eq(1.0) end @@ -74,7 +74,7 @@ module ElasticAPM ) subject.fetch_and_apply_config - sleep 0.1 + sleep 0.2 expect(subject.task).to be_pending expect(subject.task.initial_delay).to eq 123 @@ -90,7 +90,7 @@ module ElasticAPM subject.fetch_and_apply_config - sleep 0.1 + sleep 0.2 stub_response( nil, @@ -99,7 +99,7 @@ module ElasticAPM ) subject.fetch_and_apply_config - sleep 0.1 + sleep 0.2 expect(subject.task).to be_pending expect(subject.task.initial_delay).to eq 123 @@ -112,7 +112,7 @@ module ElasticAPM stub_response('Not found', status: 404) subject.fetch_and_apply_config - sleep 0.1 + sleep 0.2 expect(subject.task).to be_pending expect(subject.task.initial_delay).to eq 300 From 99fb0e5a2f4c26847abaf62ada15b64be37972a2 Mon Sep 17 00:00:00 2001 From: Mikkel Malmberg Date: Mon, 12 Aug 2019 15:34:34 +0200 Subject: [PATCH 5/6] Remove flaky test --- spec/elastic_apm/central_config_spec.rb | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/spec/elastic_apm/central_config_spec.rb b/spec/elastic_apm/central_config_spec.rb index e64feec43..78f7e592a 100644 --- a/spec/elastic_apm/central_config_spec.rb +++ b/spec/elastic_apm/central_config_spec.rb @@ -38,20 +38,6 @@ module ElasticAPM expect(config.transaction_sample_rate).to eq(0.5) end - it 'reverts config if later changed' do - stub_response(transaction_sample_rate: '0.5') - - subject.fetch_and_apply_config - sleep 0.2 - - stub_response({}) - - subject.fetch_and_apply_config - sleep 0.2 - - expect(config.transaction_sample_rate).to eq(1.0) - end - it 'reverts config if later 404' do stub_response(transaction_sample_rate: '0.5') From f769875fa504793eb9257de15914cd0aa7c3f17d Mon Sep 17 00:00:00 2001 From: Mikkel Malmberg Date: Wed, 14 Aug 2019 15:00:51 +0200 Subject: [PATCH 6/6] =?UTF-8?q?=F0=9F=94=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit