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

CAPI logs should for request complete should include the response time #4089

Merged
merged 7 commits into from
Nov 19, 2024
8 changes: 6 additions & 2 deletions lib/cloud_controller/logs/request_logs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@ def start_request(request_id, env)
@incomplete_requests.store(request_id, env)
end

def complete_request(request_id, status)
def complete_request(request_id, status, env, time_taken)
return if @incomplete_requests.delete(request_id).nil?

request = ActionDispatch::Request.new(env)
@logger.info("Completed #{status} vcap-request-id: #{request_id}",
{ status_code: status })
{ status_code: status,
nookala marked this conversation as resolved.
Show resolved Hide resolved
time_taken_in_ms: time_taken,
request_method: request.request_method,
request_fullpath: request.filtered_path })
end

def log_incomplete_requests
Expand Down
7 changes: 5 additions & 2 deletions middleware/request_logs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ def initialize(app, request_logs)
def call(env)
request_id = env['cf.request_id']
@request_logs.start_request(request_id, env)
start_time = Time.now

status, headers, body = @app.call(env)

@request_logs.complete_request(request_id, status)
# convert to milliseconds
time_taken = (Time.now - start_time) * 1000
nookala marked this conversation as resolved.
Show resolved Hide resolved
time_taken = time_taken.to_i
@request_logs.complete_request(request_id, status, env, time_taken)

[status, headers, body]
end
Expand Down
13 changes: 10 additions & 3 deletions spec/unit/lib/cloud_controller/logs/request_logs_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module VCAP::CloudController::Logs
let(:fake_fullpath) { 'fullpath' }
let(:fake_request) { double('request', request_method: 'request_method', ip: fake_ip, filtered_path: 'filtered_path', fullpath: fake_fullpath) }
let(:request_id) { 'ID' }
let(:time_taken) { 30 }
let(:env) { { 'cf.user_guid' => 'user-guid' } }
let(:status) { 200 }

Expand Down Expand Up @@ -45,14 +46,20 @@ module VCAP::CloudController::Logs
end

it 'logs the completion of the request' do
request_logs.complete_request(request_id, status)
expect(logger).to have_received(:info).with(/Completed 200 vcap-request-id: ID/, { status_code: 200 })
request_logs.complete_request(request_id, status, env, time_taken)
expect(logger).to have_received(:info).with(
/\ACompleted 200 vcap-request-id: ID/,
time_taken_in_ms: 30,
request_method: 'request_method',
request_fullpath: 'filtered_path',
status_code: 200
)
end
end

context 'without a matching start request' do
it 'does not log the completion of the request' do
request_logs.complete_request(request_id, status)
request_logs.complete_request(request_id, status, env, time_taken)
expect(logger).not_to have_received(:info)
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/middleware/request_logs_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module Middleware

it 'calls complete request on request logs after the request' do
middleware.call(env)
expect(request_logs).to have_received(:complete_request).with('ID', 200)
expect(request_logs).to have_received(:complete_request).with('ID', 200, { 'cf.request_id' => 'ID' }, be_a(Numeric))
end
end
end
Expand Down