dry-rb / dry-monitor Goto Github PK

Using Process.clock_gettime(Process::CLOCK_MONOTONIC, :millisecond) is faster than Time.now

I have an notifications table in my database, after adding a Notifications relation I started receiving very strange errors.

Upon investigation it turns out that instrumentation will try to log to my relation instead of Dry::Monitor::Notifications

SQL logger can be configured with colorized output disabled, but we have a hard dep on rouge, which turned out to be a heavy gem, so we should make it optional.

I imagine that we could lazy-require rouge when it's needed, and raise a nice exception if it failed to require it (so rescue LoadError and re-raise something nicer).

See https://discourse.dry-rb.org/t/improvement-better-memory-usage-in-dry-monitor/468

dry-monitor isn't documented or linked from dry-rb.org.

Mentioned only in the dry-events docs as a backend for the pub/sub, but no real own documentation.

Is this intentional or an oversight?

Thanks!

Hey, I would like to use it in multi thread environment. Is it safe?

I guess title says clearly what the issue is

notifications = Dry::Monitor::Notifications.new(:app)
Dry::Monitor::Notifications.register_event(:sql, { name: 'rom[sql]' })

notifications.instrument(:sql) {}

ends up with:

NameError:
uninitialized constant Dry::Monitor::Notifications::EMPTY_HASH
# ./lib/dry/monitor/notifications.rb:41:in `instrument'

#37 turned out to be necessitated only by some misbehaviour with the middleware mounting inside Hanami's new router (see hanami/hanami#1046).

We should revert it because it is unnecessary to support extra initialization args, and in a way, they may be misleading to users of the middleware (suggesting that additional args could be passed in the first place, which is not the case).

Here we require rack: https://github.com/dry-rb/dry-monitor/blob/master/lib/dry/monitor/rack/middleware.rb
but dry-monitor does not have it in the dependencies, so when rack is not used in the project it fails with this error:

LoadError:
  cannot load such file -- rack/utils

@solnic do we want to add it as a dep or make rack/middleware as a plugin same as rouge?

I'd like to use this gem in a library that doesn't depend on Rack, but I get the following error when requiring dry/monitor:

LoadError: cannot load such file -- rack/utils
  /Users/steve/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/dry-monitor-0.1.2/lib/dry/monitor/rack/middleware.rb:1:in `require'
  /Users/steve/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/dry-monitor-0.1.2/lib/dry/monitor/rack/middleware.rb:1:in `<top (required)>'
  /Users/steve/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/dry-monitor-0.1.2/lib/dry/monitor.rb:3:in `require'
  /Users/steve/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/dry-monitor-0.1.2/lib/dry/monitor.rb:3:in `<top (required)>'

Should the dependency on Rack be optional? If it is then that needs taking into account when attempting to load the rack middleware. If not then rack should be added to the gemspec. I'm guessing the intention is for it to be an optional dependency. Maybe a separate gem would be better for that?

Any usage of #instrument without payload will fail with following error:

This causes the following error when trying to use notifications:

NameError: uninitialized constant Dry::Monitor::Notifications::EMPTY_HASH
      def instrument(event_id, payload = EMPTY_HASH, &block)        

ref https://github.com/dry-rb/dry-monitor/blob/main/lib/dry/monitor/notifications.rb#L39=

When I include dry-monitor 0.1.2 for a non-rack based app, I get this error during the boot

/home/mencio/.rvm/rubies/ruby-2.5.0/lib/ruby/2.5.0/irb/init.rb:280: warning: LoadError: cannot load such file -- rack/util

Hey guys,

We use dry-monitor together with dry-events in karafka. In order to provide a "safe" API (safe in terms of typo mistakes etc) we've implemented two additional methods for the monitor that could be useful when working with it. Would it fit as a PR in this library?

      # Allows us to subscribe to events with a code that will be yielded upon events
      # @param event_name_or_listener [String, Object] name of the event we want to subscribe to
      #   or a listener if we decide to go with object listener
      def subscribe(event_name_or_listener)
        return super unless event_name_or_listener.is_a?(String)
        return super if available_events.include?(event_name_or_listener)
        raise Errors::UnregisteredMonitorEvent, event_name_or_listener
      end

      # @return [Array<String>] names of available events to which we can subscribe
      def available_events
        __bus__.events.keys
      end

I believe that this behavior would prevent users from subscribing to non-registered events.

Whole monitor can be found here: https://github.com/karafka/karafka/blob/master/lib/karafka/instrumentation/monitor.rb

First of all, thank you for dry-monitor and dry-events 🙏. I'm really grateful that we have an alternative to ActiveSupport::Notifications, and I'm glad to add support for it in Shrine 😃

I noticed that when I forget to register an event, I don't get a pretty exception:

require "dry-monitor"

notifications = Dry::Monitor::Notifications.new(:test)
notifications.subscribe("my_event") { |event| }
notifications.instrument("my_event") { }

Traceback (most recent call last):
        5: from tmp/monitor.rb:5:in `<main>'
        4: from /Users/janko/.rbenv/versions/2.6.0/lib/ruby/gems/2.6.0/gems/dry-monitor-0.3.1/lib/dry/monitor/notifications.rb:45:in `instrument'
        3: from /Users/janko/.rbenv/versions/2.6.0/lib/ruby/gems/2.6.0/gems/dry-events-0.1.1/lib/dry/events/publisher.rb:231:in `process'
        2: from /Users/janko/.rbenv/versions/2.6.0/lib/ruby/gems/2.6.0/gems/dry-events-0.1.1/lib/dry/events/bus.rb:32:in `process'
        1: from /Users/janko/.rbenv/versions/2.6.0/lib/ruby/gems/2.6.0/gems/dry-events-0.1.1/lib/dry/events/bus.rb:32:in `each'
/Users/janko/.rbenv/versions/2.6.0/lib/ruby/gems/2.6.0/gems/dry-events-0.1.1/lib/dry/events/bus.rb:33:in `block in process': undefined method `payload' for nil:NilClass (NoMet
hodError)

Simple container class;

require 'dry/web/container'
require 'dry/system/components'

module NotificationService
  class Container < Dry::Web::Container

    configure do
      config.name = :notification_service
      config.listeners = true
      config.default_namespace = 'api'
      config.auto_register = %w[lib/api]
    end
end

Line class Container < Dry::Web::Container will throw:

  uninitialized constant Dry::Monitor::Rack

The full stack trace is:

NameError:
  uninitialized constant Dry::Monitor::Rack
  Did you mean?  Rack
# ./vendor/bundle/ruby/2.4.0/gems/dry-web-0.8.0/lib/dry/web/container.rb:25:in `register_rack_monitor'
# ./vendor/bundle/ruby/2.4.0/gems/dry-web-0.8.0/lib/dry/web/container.rb:16:in `block in inherited'
# ./vendor/bundle/ruby/2.4.0/gems/dry-system-0.10.1/lib/dry/system/container.rb:107:in `instance_eval'
# ./vendor/bundle/ruby/2.4.0/gems/dry-system-0.10.1/lib/dry/system/container.rb:107:in `block in configure'
# ./vendor/bundle/ruby/2.4.0/gems/dry-system-0.10.1/lib/dry/system/container.rb:107:in `each'
# ./vendor/bundle/ruby/2.4.0/gems/dry-system-0.10.1/lib/dry/system/container.rb:107:in `configure'
# ./system/notification_service/container.rb:11:in `<class:Container>'

Apart from tracking the performance (time), it would be really good to keep track of the allocations count from the block execution (if provided).

Describe the bug

When running bin/rails zeitwerk:check I'm getting:

Hold on, I am eager loading the application.
expected file /Users/azdaroth/.rvm/gems/ruby-3.1.2/gems/dry-monitor-0.6.1/lib/dry/monitor/version.rb to define constant Dry::Monitor::Version

Has anyone else experiences this issue?

My environment

• Rails version 7.0.3.1
• Ruby version: 3.1.2