Skip to content

Commit

Permalink
Handle consecutive path segments containing IDs in collector
Browse files Browse the repository at this point in the history
Previously, we would fail to strip IDs from paths if they appeared in
consecutive path segments. This is because our regex that looked for IDs
and UUIDs would consume the `/` character that followed them, causing
the next one not to match.

This commit uses a lookahead to match against the `/` without consuming
it.

Signed-off-by: Chris Sinjakli <chris@sinjakli.co.uk>
  • Loading branch information
Sinjo committed Feb 1, 2022
1 parent 81a0e29 commit 2f512f9
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/prometheus/middleware/collector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ def generate_path(env)

def strip_ids_from_path(path)
path
.gsub(%r{/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}(/|$)}, '/:uuid\\1')
.gsub(%r{/\d+(/|$)}, '/:id\\1')
.gsub(%r{/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}(?=/|$)}, '/:uuid\\1')
.gsub(%r{/\d+(?=/|$)}, '/:id\\1')
end
end
end
Expand Down
28 changes: 28 additions & 0 deletions spec/prometheus/middleware/collector_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,34 @@
expect(registry.get(metric).get(labels: labels)).to include("0.1" => 0, "0.5" => 1)
end

it 'handles consecutive path segments containing IDs' do
expect(Benchmark).to receive(:realtime).and_yield.and_return(0.3)

get '/foo/42/24'

metric = :http_server_requests_total
labels = { method: 'get', path: '/foo/:id/:id', code: '200' }
expect(registry.get(metric).get(labels: labels)).to eql(1.0)

metric = :http_server_request_duration_seconds
labels = { method: 'get', path: '/foo/:id/:id' }
expect(registry.get(metric).get(labels: labels)).to include("0.1" => 0, "0.5" => 1)
end

it 'handles consecutive path segments containing UUIDs' do
expect(Benchmark).to receive(:realtime).and_yield.and_return(0.3)

get '/foo/5180349d-a491-4d73-af30-4194a46bdff3/5180349d-a491-4d73-af30-4194a46bdff2'

metric = :http_server_requests_total
labels = { method: 'get', path: '/foo/:uuid/:uuid', code: '200' }
expect(registry.get(metric).get(labels: labels)).to eql(1.0)

metric = :http_server_request_duration_seconds
labels = { method: 'get', path: '/foo/:uuid/:uuid' }
expect(registry.get(metric).get(labels: labels)).to include("0.1" => 0, "0.5" => 1)
end

it 'prefers sinatra.route to PATH_INFO' do
metric = :http_server_requests_total

Expand Down

0 comments on commit 2f512f9

Please sign in to comment.