Configuration in initialiser ignored

[malkovro]

Hello there 👋🏼!

I have been playing a bit more with the framework and started to loose patience waiting for my own messages to arrive while debugging 😅. I then tried to use the config associated mentioned in the changelog _Stealth.config.auto_insertdelays to remove them altogether but could not make it work… so I investigated why 🙃

• Whenever a new message arrives through a webhook, a job is pushed onto the queue.
• The job is accepted by one of the Sidekiq process and a Stealth::Dispatcher takes care of instantiating the appropriate MessageHandler adapter and pipe the request into the main controller

At this step the Dispatcher is looking for a config value whether to log or not the message.

To do so, it sends a message on the module Stealth who in turns looks for Thread.current[:configuration]… There seems to be nothing in there so it starts the load_services_config which loads the services.yaml file back again.

At this point any customisation of the configuration done in the app/boot.rb or any initialisers (as mentionned in the changelog) is lost because the load_environment message is not called.

Additionally, some values, if specified to false, are ignored because of the ||= syntax.

Example :

I wanted to disable the delay directly in my service.yml with:

default: &default
  auto_insert_delays: false # Does not work because the default value takes over the logical expression, see. https://github.com/hellostealth/stealth/blob/master/lib/stealth/base.rb#L76
  dynamic_delay_muliplier: 0.1 # Does work because it evaluates to true

  # /***/

I tried to take a look at how the specs are written to understand better how the delays are managed there but a before(:each) block sets the right value for the config.auto_insert_delays which is not how this works once it is "required into sidekiq".

I am not particularly familiar with the Thread.current part so I might be missing a (big) point here🙏🏼. If not then I would be glad to contribute in a way to fix/refactor the configuration section.

---

Quick fix (that does not solve the initialisers ignored) in the case of the _auto_insertdelays would be to change line 76 of the set_config_defaults to something like:

  config.auto_insert_delays ||= config.auto_insert_delays.nil?

👂🏽

[mgomes]

Thanks for the detailed report @malkovro! Recently, I've heard reports about another configuration option not being set.

I personally don't love how we're setting config options unrelated to services in services.yml, but we can change that after Stealth 2.0.

I am going to test these all out locally and hopefully a fix will be live for beta 2. Thanks again!

/cc @emorissettegregoire

[malkovro]

After researching a bit more the subject I ended up on a post of yours explaining the motivations behind the use of Thread fiber local variable and it totally make sense in your use case. If you want to allow each Sidekiq job to load a customer specific configuration without them altering each others’ config.

It sounds like a good way to solve this. However, you might have had an issue came up as Sidekiq is not clearing the thread fiber local variable between jobs and as the framework is loading the configuration only if the Thread.current[:configuration] is empty you might encounter some leaking configuration from one job to the other. Another quick fix here is to cleanup the configuration before processing the job in HandleMessageJob calling Stealth.load_services_config! (Method I did not see used anywhere 🤔).

Once again great work you doing here and great framework 👏 Looking forward for what v2 will be about 😬

[emorissettegregoire]

@mgomes Thanks so much!! session_ttl setup in services.yml works well!

[mgomes]

@emorissettegregoire awesome, glad to hear it! I've been testing locally and it's been working for me as well. I think I will cut 2.0.0.beta2 in a few hours. :+1: