Added metrics to Scheduler and track in Process state

[AndersGM]

#532

[bensheldon]

I'm happy to accept this. Briefly, it needs:

• threadsafety
• have the value show up when the Process serializes the Schedulers so that it can be queried
• I think the value should be reset when the the Scheduler is restarted (otherwise it might get weird when forking, especially with Puma's worker_fork

[AndersGM]

@bensheldon Thank you for taking your time to look into this! Regarding the serialization - as far as I can see process uses name when serializing, so I've added the metrics to the name here in 78d8aba. Not sure if that is the best place though, but not sure where else to put it, without messing up the enclosing parentheses. What do you think? Also considered adding something like:

succeeded_count: GoodJob::Scheduler.instances.sum(&:succeeded_count),
failed_count: GoodJob::Scheduler.instances.sum(&:failed_count),

to Process.current_state.

[bensheldon]

@AndersGM Good things to flag! What do you think about making a method like Scheduler#statistics that contained the name and those values and then dropping that into the Process state? (I also think they should be succeeded_executions_count for clarity).

Ok, and then I have a totally random thought that I want to share, but isn't telling you to do instead: What if we tracked executed_by_process_id on the Execution/DiscreteExecution object in the database? Would that be equivalent? (it could be joined in the database against the Process record)

[AndersGM]

@bensheldon That would be better, but maybe not statistics since there already is a method named stats so it might be confusing. What about name_with_metrics or just adding a parameter include_metrics to the name method? Agree about the more longer more explicit variable names (succeeded_executions_count).

Sound interesting. I actually do something similar in one of my apps that then sends the metrics to prometheus. However, the issue is that the executions can be destroyed through the UI (and cleanup?). Consequently, the resulting counter could actually decrement, without being reset. That is the reason I implemented it this way, to avoid that. Did you think of another way or have I misunderstood something? :-)

[bensheldon]

@AndersGM I made a few tweaks and pushed this back up. It's looking great!

I just merged in #977 which will update the Process record in the database every 30 seconds.

One thing I noticed that I'd like your feedback on. Currently this PR only records a job failure if it bubbles up to the thread-handler. This will cover only a small number of execution failures. We want to cover all the circumstances where a job's execution does not succeed, right? (e.g. if an execution fails and is retried, we'd still count that). That seem right? If you agree, I can make that tweak.

[AndersGM]

@bensheldon indeed - thank you! I wasn't aware of that - let me know if I can be of any help!