Add stats about job classes

[arnaudlevy]

[bensheldon]

@arnaudlevy I'm excited where you're going with this! fyi, there is some discussion in #438 about such a feature. Let me know when you'd like me to review it and give you feedback.

[arnaudlevy]

Hey @bensheldon, glad you like it! Maybe what I did there could be a first step. Tell me what you think about the names, so I can walk in your steps and adjust. I put all chart config back in ruby, is that ok?

[arnaudlevy]

@bensheldon what do you think about denormalizing runtime_latency? It would be easier to manipulate as a field in the table.

[bensheldon]

@arnaudlevy there's a little about storing the latency in #1053.

I would say:

• Go for it 👍🏻
• Please do it as a separate PR (nows the time because I'm working on the next major GoodJob upgrade and it would be ideal to get in any database migrations before that)
• Store the column as a BigInt as like duration_ms on the good_job_executions table. I don't think we want to store a float.
• Do it as a monotonic time (get the monotonic start as a Ruby variable and then calculate the total duration in milliseconds and store that)

[bensheldon]

Tell me what you think about the names, so I can walk in your steps and adjust. I put all chart config back in ruby, is that ok?

• Chart config is ok 👍🏻
• I'd like to call this page "Performance" rather than "Statistics" (🚧 Necessary)
• The calculations must be done in the database, rather than by loading all the executions. (🚧 Necessary)
  • We should probably scope it by time ago (1 hour, 4 hours, 1 dday, etc.). When I've spiked on that in the past (but never made as much progress as you!) I had a drop-down that would set a paramter. But I would say just pick one for now and we can always go back and add more options in a later PR
• I'd ideally like this to (eventually, ok in a follow-on PR) be a top level list of queues, that then could be drilled down on to job class.

Sorry if that's a lot. Only two changes are truly necessary I think (calculate in DB, rename to Performance)

[arnaudlevy]

Hey @bensheldon ! Renaming to performance was easy, it's done. I did some SQL limits (start / end), but i'm not too sure what you think about. Here is the current graph (very suboptimal, as every execution is shown).

The idea, for us, is to spot the "slow elephants", the ones that are really long. What do you think?

[arnaudlevy]

Don't know if you noticed @bensheldon but @SebouChu added a fix to the filters

[bensheldon]

@SebouChu added a fix to the filters

Nice! Could you break that out into its own PR?

On that same note, I'd like to isolate this PR just down to the PerformancesController#index controller/view. That would help me really isolate down this feature and we can work on making it performant/possible. Once we get that done/shipped, then focus on additional features for drilling down (don't throw it away! I just want to make it easier to move forward the first piece)

[SebouChu]

Hi @bensheldon!

The filters fix is now on this PR if you want: https://github.com/bensheldon/good_job/pull/1373 !

[SebouChu]

@bensheldon Just finished adjustments here, mainly having one SQL request for the Performances#index, and focusing on the page only.

Right now, the request gets the execution duration with EXTRACT(EPOCH FROM (finished_at - created_at)), but with #1374, it will be simplified so feel free to merge this one so that I can adjust the SQL request here!

[SebouChu]

@bensheldon I made the adjustment in the Performance Controller to use duration instead of extracting EPOCH, should be good now!

[bensheldon]

I had to open a new PR (#1388) in order to push up changes to it (it seems like when a PR is opened from a branch named main I'm unable to push changes to it 🤷🏻). It looks great and I'm about to merge it 🎉

[arnaudlevy]

Thanks @bensheldon, that's really cool!

[SebouChu]

Perfect, thanks!