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

Configure rack defaults in load time #218

Closed
wants to merge 1 commit into from
Closed

Conversation

@etehtsea
Copy link
Contributor

etehtsea commented Mar 24, 2015

At the moment it execute on each request.

@jessicard
Copy link
Contributor

jessicard commented Mar 26, 2015

Hey @etehtsea - I'm not seeing this behavior (specifically, executing on each request). What was happening for you to make you see that?

@etehtsea
Copy link
Contributor Author

etehtsea commented Mar 29, 2015

There is a demo app - https://gist.github.com/etehtsea/9bd1f2ccc69d5c3490cc

Steps to reproduce:

/tmp/9bd1f2ccc69d5c3490cc [ master: ✔ ] 4d ⚡ bundle                                                                                                          jruby-1.7.18 ☺
WARN: Unresolved specs during Gem::Specification.reset:
      ffi (>= 0)
WARN: Clearing out unresolved specs.
Please report a bug if this causes problems.
Resolving dependencies...
Using multi_json 1.11.0
Using bugsnag 2.8.1
Using coderay 1.1.0
Using method_source 0.8.2
Using slop 3.6.0
Using ffi 1.9.8
Using spoon 0.0.4
Using pry 0.10.1
Using rack 1.5.2
Using bugsnag_threading_issue 0.0.1 from source at .
Using bundler 1.9.1
Bundle complete! 2 Gemfile dependencies, 11 gems now installed.
Use `bundle show [gemname]` to see where a bundled gem is installed.
/tmp/9bd1f2ccc69d5c3490cc [ master: ✔ ] 4d ⚡ bundle exec rackup                                                                                              jruby-1.7.18 ☺
[2015-03-30 04:26:36] INFO  WEBrick 1.3.1
[2015-03-30 04:26:36] INFO  ruby 1.9.3 (2014-12-22) [java]
[2015-03-30 04:26:36] INFO  WEBrick::HTTPServer#start: pid=83887 port=9292
"Bugsnag middlewares: 2"
127.0.0.1 - - [30/Mar/2015 04:26:42] "GET / HTTP/1.1" 200 - 0.0160
"Bugsnag middlewares: 3"
127.0.0.1 - - [30/Mar/2015 04:26:44] "GET / HTTP/1.1" 200 - 0.0040
"Bugsnag middlewares: 4"
127.0.0.1 - - [30/Mar/2015 04:26:45] "GET / HTTP/1.1" 200 - 0.0030
@ConradIrwin
Copy link
Contributor

ConradIrwin commented Mar 31, 2015

I've solved this in a way that should work for all our plugins.

Obviously the block doesn't execute that many times (once for each time you boot a server in a multi-threaded app), but we were previously assuming that each process would only have one server, so it's good to fix this.

Thanks!

@etehtsea
Copy link
Contributor Author

etehtsea commented Mar 31, 2015

As far as I see it does because rack middleware instance created on each request.
https://github.com/rack/rack/blob/master/lib/rack/builder.rb#L86

You can easily check this (bugsnag 2.8.1):

/tmp  ⚡ cat config.ru                                                                                                                                      jruby-1.7.18 ☺
%w(bugsnag rack).each do |g|
  gem g
  require g
end

class App
  def initialize
    @app = Rack::Builder.new do
      use Bugsnag::Rack

      map('/') do
        run ->(_) do
          count = Bugsnag.configuration.middleware.instance_variable_get(:@middlewares).count
          p "Bugsnag middlewares: #{count}"
          [200, {'Content-Type' => 'application/json' }, ["{\"middlewares\":#{count}}"]]
        end
      end
    end
  end

  def call(env)
    @app.call(env)
  end
end

run App.new

This is where I stopped:

"Bugsnag middlewares: 43997"
127.0.0.1 - - [01/Apr/2015:01:25:08 +0600] "GET / HTTP/1.1" 200 - 0.0440
[2015-04-01 01:25:08] INFO  WEBrick::HTTPServer#start done.
@ConradIrwin
Copy link
Contributor

ConradIrwin commented Mar 31, 2015

@etehtsea
Creating a new rack app to handle each request is extremely unusual in my experience, you'd normally see something closer to this:

app=Rack::Builder.new do
      use Bugsnag::Rack

      map('/') do
        run ->(_) do
          count = Bugsnag.configuration.middleware.instance_variable_get(:@middlewares).count
          p "Bugsnag middlewares: #{count}"
          [200, {'Content-Type' => 'application/json' }, ["{\"middlewares\":#{count}}"]]
        end
      end
    end

run app
@etehtsea
Copy link
Contributor Author

etehtsea commented Mar 31, 2015

@ConradIrwin did you run this code?

It doesn't change anything for me.

/tmp  ⚡ cat config2.ru                                                                                                                                     jruby-1.7.19 ☹
%w(bugsnag rack).each do |g|
  gem g
  require g
end

app=Rack::Builder.new do
      use Bugsnag::Rack

      map('/') do
        run ->(_) do
          count = Bugsnag.configuration.middleware.instance_variable_get(:@middlewares).count
          p "Bugsnag middlewares: #{count}"
          [200, {'Content-Type' => 'application/json' }, ["{\"middlewares\":#{count}}"]]
        end
      end
    end

run app
/tmp  ⚡ rackup config2.ru                                                                                                                                  jruby-1.7.19 ☺
/Users/kes/.rbenv/versions/jruby-1.7.19/lib/ruby/gems/shared/gems/rack-1.6.0/lib/rack/request.rb:149 warning: already initialized constant FORM_DATA_MEDIA_TYPES
/Users/kes/.rbenv/versions/jruby-1.7.19/lib/ruby/gems/shared/gems/rack-1.6.0/lib/rack/request.rb:157 warning: already initialized constant PARSEABLE_DATA_MEDIA_TYPES
/Users/kes/.rbenv/versions/jruby-1.7.19/lib/ruby/gems/shared/gems/rack-1.6.0/lib/rack/request.rb:164 warning: already initialized constant DEFAULT_PORTS
[2015-04-01 02:42:05] INFO  WEBrick 1.3.1
[2015-04-01 02:42:05] INFO  ruby 1.9.3 (2015-01-29) [java]
[2015-04-01 02:42:05] INFO  WEBrick::HTTPServer#start: pid=53921 port=9292
W, [2015-04-01T02:42:20.388000 #53921]  WARN -- : ** [Bugsnag] You should set your app's project_root (see https://bugsnag.com/docs/notifiers/ruby#project_root).
"Bugsnag middlewares: 2"
127.0.0.1 - - [01/Apr/2015:02:42:20 +0600] "GET / HTTP/1.1" 200 - 0.0120
W, [2015-04-01T02:42:21.017000 #53921]  WARN -- : ** [Bugsnag] You should set your app's project_root (see https://bugsnag.com/docs/notifiers/ruby#project_root).
"Bugsnag middlewares: 3"
127.0.0.1 - - [01/Apr/2015:02:42:21 +0600] "GET / HTTP/1.1" 200 - 0.0120
W, [2015-04-01T02:42:21.792000 #53921]  WARN -- : ** [Bugsnag] You should set your app's project_root (see https://bugsnag.com/docs/notifiers/ruby#project_root).
"Bugsnag middlewares: 4"
127.0.0.1 - - [01/Apr/2015:02:42:21 +0600] "GET / HTTP/1.1" 200 - 0.0110
W, [2015-04-01T02:42:22.279000 #53921]  WARN -- : ** [Bugsnag] You should set your app's project_root (see https://bugsnag.com/docs/notifiers/ruby#project_root).
"Bugsnag middlewares: 5"
127.0.0.1 - - [01/Apr/2015:02:42:22 +0600] "GET / HTTP/1.1" 200 - 0.0030
W, [2015-04-01T02:42:23.400000 #53921]  WARN -- : ** [Bugsnag] You should set your app's project_root (see https://bugsnag.com/docs/notifiers/ruby#project_root).
"Bugsnag middlewares: 6"
127.0.0.1 - - [01/Apr/2015:02:42:23 +0600] "GET / HTTP/1.1" 200 - 0.0050
W, [2015-04-01T02:42:24.855000 #53921]  WARN -- : ** [Bugsnag] You should set your app's project_root (see https://bugsnag.com/docs/notifiers/ruby#project_root).
"Bugsnag middlewares: 7"
127.0.0.1 - - [01/Apr/2015:02:42:24 +0600] "GET / HTTP/1.1" 200 - 0.0050
W, [2015-04-01T02:42:25.200000 #53921]  WARN -- : ** [Bugsnag] You should set your app's project_root (see https://bugsnag.com/docs/notifiers/ruby#project_root).
"Bugsnag middlewares: 8"
127.0.0.1 - - [01/Apr/2015:02:42:25 +0600] "GET / HTTP/1.1" 200 - 0.0030
W, [2015-04-01T02:42:25.550000 #53921]  WARN -- : ** [Bugsnag] You should set your app's project_root (see https://bugsnag.com/docs/notifiers/ruby#project_root).
"Bugsnag middlewares: 9"
127.0.0.1 - - [01/Apr/2015:02:42:25 +0600] "GET / HTTP/1.1" 200 - 0.0050
W, [2015-04-01T02:42:25.820000 #53921]  WARN -- : ** [Bugsnag] You should set your app's project_root (see https://bugsnag.com/docs/notifiers/ruby#project_root).
"Bugsnag middlewares: 10"
@ConradIrwin
Copy link
Contributor

ConradIrwin commented Mar 31, 2015

Did you upgrade to bugsnag 2.8.2? I just see the count stay at 2.

@etehtsea
Copy link
Contributor Author

etehtsea commented Mar 31, 2015

This don't matter in this case. This was the simplest way to show that Bugsnag.configure is calling on each request.

This means that after 2.8.2 all requests will wait for synchronization block to check is bugsnag middleware in the list or not. So I think it will somehow affect performance (but I haven't benched this yet).

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

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.