Monitoring section for README

[aried3r]

See #411.

Proposal for the new section. There's only one instrumentation event for now since I'll have to go over the code base and find all of them and I'd like to know if this is the direction we should be headed (also because markdown tables are a chore to generate).

Monitoring and instrumentation

GoodJob integrates with Rails' ActiveJob framework, which is instrumented and has existing monitoring tools, such as AppSignal, DataDog, NewRelic, Skylight etc.

In addition to that, GoodJob offers its own instrumentation:

enqueue_job.good_job

Key	Value
active_job	The ActiveJob instance to enqueue.
scheduled_at	Epoch timestamp when the job should be executed.
create_with_advisory_lock	Boolean. Whether to establish a lock on the GoodJob::Execution record after it is created.

[bensheldon]

Thanks for opening this discussion!

I agree that markdown tables are painful to generate. I'd like to avoid that if possible. I'm also fearful of documentation getting out of sync with implementation.

The instrumentation events are (mostly) exercised by the Log Subscriber: https://github.com/bensheldon/good_job/blob/main/lib/good_job/log_subscriber.rb

I wonder if the generated Log Subscriber documentation could be linked to:

https://www.rubydoc.info/gems/good_job/GoodJob/LogSubscriber

(rubydoc seems very unreliable, which also makes me wonder if we should self-host the generated yard docs)

That would mean the Log Subscriber subscribing to all the events, even if they don't generate a log, but I think that's fine to have some noop methods.

[aried3r]

I'm also fearful of documentation getting out of sync with implementation.

Yeah, that's especially tricky if documentation isn't generated from source like it is for the LogSubscriber.

rubydoc seems very unreliable

That has also been my experience, even though now it works fine. I wish https://rubyapi.org/ was open to gems as well, it's much nicer to browse.

That would mean the Log Subscriber subscribing to all the events, even if they don't generate a log, but I think that's fine to have some noop methods.

You mean having it subscribe to enqueue_job.good_job even though it would simply return? Is there a good way we can make sure it always takes the correct number of arguments and documents optional ones (if there are any, I haven't checked).

[bensheldon]

You mean having it subscribe to enqueue_job.good_job even though it would simply return?

Yes, I'm imagining having that file subscribe to all the events, regardless of whether it generates a log line. I dunno if that would have any performance implications.

Alternatively, there could be a test-harness that lives in the spec/test_app that subscribes to all the events and documents them; that might be better because it would be out of the gem release path, and might also make it easier to write assertions against separately in a singular place. I'd have to think more about that.

Is there a good way we can make sure it always takes the correct number of arguments and documents optional ones (if there are any, I haven't checked).

I was wondering the same thing. It would be nice to assert against the payload. Also, generally, I'm not quite sure how to indicate deprecations in instrumentation (e.g. detect that the application has subscribed to a deprecated notification event, and output a deprecation warning); it seems like it would be impossible to deprecate payload keys without doing something drastic like returning a payload object that overloas def [] or something.

To move forward., what do you think of this (in order of descending importance):

• Link to the LogSubscriber from the Readme in reference to monitoring
• Do a quick audit through GoodJob for instrumentation and add them as no-ops to the LogSubscriber
• Take a look at the YARD macro being used in the LogSubscriber to see if there's a straightforward way to include event payload details

[aried3r]

Alternatively, there could be a test-harness that lives in the spec/test_app that subscribes to all the events and documents them; that might be better because it would be out of the gem release path, and might also make it easier to write assertions against separately in a singular place. I'd have to think more about that.

That sounds good, although I wonder how we could generate documentation from that.

Alternatively, although maybe not best, we could also offer an additional file that is being generated on install which offers all the instrumentation points. Rpush does it like this so people can react to certain events. Not saying that's the best way, but since it comes during installation and everything is commented out there'd be no overhead.

Also, generally, I'm not quite sure how to indicate deprecations in instrumentation (e.g. detect that the application has subscribed to a deprecated notification event, and output a deprecation warning)

Maybe I don't understand correctly, but could we use ActiveSupport::Deprecation.warn on the call site?

To move forward., what do you think of this (in order of descending importance):

Sounds good, this week I'm a bit busy at work, but I'll try to have a look. But if somebody wants to do it via a Hacktoberfest issue, I'm all for it. It seems this attracted a few people or so it seems.

[henrydjacob]

Is any one used NewRelic with good_job