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

Don't add custom fields before controller #113

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions lib/logstasher/rails_ext/action_controller/metal/instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,28 @@ def process_action(*args)
logstasher_add_custom_fields_to_request_context(LogStasher.request_context)
end

if self.respond_to?(:logtasher_add_custom_fields_to_payload)
before_keys = raw_payload.keys.clone
logtasher_add_custom_fields_to_payload(raw_payload)
after_keys = raw_payload.keys
# Store all extra keys added to payload hash in payload itself. This is a thread safe way
LogStasher.custom_fields += after_keys - before_keys
begin
result = super
payload[:status] = response.status
result
ensure
if self.respond_to?(:logtasher_add_custom_fields_to_payload)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it should be :logstasher_add_custom_fields_to_payload instead of :logtasher_add_custom_fields_to_payload

before_keys = raw_payload.keys.clone
logtasher_add_custom_fields_to_payload(raw_payload)
after_keys = raw_payload.keys
# Store all extra keys added to payload hash in payload itself. This is a thread safe way
LogStasher::CustomFields.custom_fields += after_keys - before_keys
end
append_info_to_payload(payload)
LogStasher.store.each do |key, value|
payload[key] = value
end

LogStasher.request_context.each do |key, value|
payload[key] = value
end
LogStasher::CustomFields.clear
end

result = super

payload[:status] = response.status
append_info_to_payload(payload)
LogStasher.store.each do |key, value|
payload[key] = value
end

LogStasher.request_context.each do |key, value|
payload[key] = value
end
LogStasher::CustomFields.clear
result
end
end
alias :logstasher_process_action :process_action
Expand Down
9 changes: 9 additions & 0 deletions logstasher.iml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>
<module type="RUBY_MODULE" version="4">
<component name="NewModuleRootManager" inherit-compiler-output="true">
<exclude-output />
<content url="file://$MODULE_DIR$" />
<orderEntry type="jdk" jdkName="rbenv: 2.2.3" jdkType="RUBY_SDK" />
<orderEntry type="sourceFolder" forTests="false" />
</component>
</module>
39 changes: 39 additions & 0 deletions spec/lib/logstasher/instrumentation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,44 @@ def subject.index(*args)
end
end
end

context "payload has custom fields defined" do
before :each do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:each part is redundant. before { some_code } is same as before(:each) { some_code }. This also applies to after blocks

LogStasher.add_custom_fields do |fields|
fields[:some_field] = 'value'
end

ActiveSupport::Notifications.subscribe('process_action.action_controller') do |_, _, _, _, payload|
@payload = payload
end
end

it "should retain the value in the request context" do
subject.process_action(:index)
end

context 'exception thrown by controller' do
before :each do
def subject.index_with_exception(*args)
fail 'Exception'
end
end

it 'should retain the value in the request context' do
expect { subject.process_action(:index_with_exception) }.to raise_exception('Exception')
end
end

after :each do
expect(@payload[:some_field]).to eq('value')

ActionController::Metal.class_eval do
undef logtasher_add_custom_fields_to_payload
end
ActionController::Base.class_eval do
undef logtasher_add_custom_fields_to_payload
end
end
end
end
end