Merge branch 'limit-metrics-content-type' into 'master'
Limit the action suffixes in transaction metrics See merge request gitlab-org/gitlab-ce!20032
This commit is contained in:
commit
88ceef87e3
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Limit the action suffixes in transaction metrics
|
||||
merge_request:
|
||||
author:
|
||||
type: fixed
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue