diff options
| -rw-r--r-- | changelogs/unreleased/limit-metrics-content-type.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/metrics/web_transaction.rb | 9 | ||||
| -rw-r--r-- | spec/lib/gitlab/metrics/web_transaction_spec.rb | 11 |
3 files changed, 22 insertions, 3 deletions
diff --git a/changelogs/unreleased/limit-metrics-content-type.yml b/changelogs/unreleased/limit-metrics-content-type.yml new file mode 100644 index 00000000000..42cb4347771 --- /dev/null +++ b/changelogs/unreleased/limit-metrics-content-type.yml @@ -0,0 +1,5 @@ +--- +title: Limit the action suffixes in transaction metrics +merge_request: +author: +type: fixed diff --git a/lib/gitlab/metrics/web_transaction.rb b/lib/gitlab/metrics/web_transaction.rb index 3799aaebf1c..723ca576aab 100644 --- a/lib/gitlab/metrics/web_transaction.rb +++ b/lib/gitlab/metrics/web_transaction.rb @@ -3,6 +3,7 @@ module Gitlab class WebTransaction < Transaction CONTROLLER_KEY = 'action_controller.instance'.freeze ENDPOINT_KEY = 'api.endpoint'.freeze + ALLOWED_SUFFIXES = Set.new(%w[json js atom rss xml zip]) def initialize(env) super() @@ -32,9 +33,13 @@ module Gitlab # Devise exposes a method called "request_format" that does the below. # However, this method is not available to all controllers (e.g. certain # Doorkeeper controllers). As such we use the underlying code directly. - suffix = controller.request.format.try(:ref) + suffix = controller.request.format.try(:ref).to_s - if suffix && suffix != :html + # Sometimes the request format is set to silly data such as + # "application/xrds+xml" or actual URLs. To prevent such values from + # increasing the cardinality of our metrics, we limit the number of + # possible suffixes. + if suffix && ALLOWED_SUFFIXES.include?(suffix) action += ".#{suffix}" end diff --git a/spec/lib/gitlab/metrics/web_transaction_spec.rb b/spec/lib/gitlab/metrics/web_transaction_spec.rb index 6eb0600f49e..0b3b23e930f 100644 --- a/spec/lib/gitlab/metrics/web_transaction_spec.rb +++ b/spec/lib/gitlab/metrics/web_transaction_spec.rb @@ -194,7 +194,7 @@ describe Gitlab::Metrics::WebTransaction do expect(transaction.action).to eq('TestController#show') end - context 'when the response content type is not :html' do + context 'when the request content type is not :html' do let(:request) { double(:request, format: double(:format, ref: :json)) } it 'appends the mime type to the transaction action' do @@ -202,6 +202,15 @@ describe Gitlab::Metrics::WebTransaction do expect(transaction.action).to eq('TestController#show.json') end end + + context 'when the request content type is not' do + let(:request) { double(:request, format: double(:format, ref: 'http://example.com')) } + + it 'does not append the MIME type to the transaction action' do + expect(transaction.labels).to eq({ controller: 'TestController', action: 'show' }) + expect(transaction.action).to eq('TestController#show') + end + end end it 'returns no labels when no route information is present in env' do |