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

Custom fields handling causing unexpected behaviour with devise #114

Closed
peterox opened this issue Jul 8, 2016 · 3 comments
Closed

Custom fields handling causing unexpected behaviour with devise #114

peterox opened this issue Jul 8, 2016 · 3 comments

Comments

@peterox
Copy link

peterox commented Jul 8, 2016

Adding a Devise users details as a custom field was causing devise to be initialised prior to the controller action being executed. This is due to the fact that the custom fields were being added before the controller action.

Also, custom fields are not being added if an exception was thrown from the controller.

@johnowenatala
Copy link

Hi,

checking code, I think problem is here:

LogStasher.custom_fields += after_keys - before_keys
https://github.com/shadabahmed/logstasher/blob/master/lib/logstasher/rails_ext/action_controller/metal/instrumentation.rb#L31

This other file has the same, but now changed to this:
LogStasher::CustomFields.add(*(after_keys - before_keys))
https://github.com/shadabahmed/logstasher/blob/master/lib/logstasher/rails_ext/action_controller/base.rb#L27

By the way, please check this two classes - they seem very similar and looks like they are trying to do the same method override with very little differences, which doesnt look right - and looks like it can be broken very easy.

@MarcGrimme
Copy link
Collaborator

MarcGrimme commented Jul 14, 2016

You are right. I overlooked this line.

LogStasher.custom_fields += after_keys - before_keys

in lib/logstasher/rails_ext/action_controller/metal/instrumentation.rb. So it needs to be fixed (See #115).

FYI: Both classes are indeed similar should deliver the SAME functionality. I introduced them to offer two ways of using logstasher one with implicit monkey patching and one by explicit documented including.

MarcGrimme added a commit that referenced this issue Jul 23, 2016
…om-fields

[Parly #114] Fixes a regression found in instrumentation with customer_fields
drewda added a commit to transitland/transitland-datastore that referenced this issue Jul 25, 2016
related to a past problem:

- in Datastore: #674
- in Logstasher gem: shadabahmed/logstasher#114
@shadabahmed
Copy link
Owner

shadabahmed commented Mar 6, 2017

Closing this as fixed in #115

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants