Monitor by tag_columns (defaults with name) - #117
Draft
BetterBoyer wants to merge 3 commits into
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
All
delayed.job.*monitor metrics are now grouped by — and tagged with — a configurable set of job columns, in addition topriorityandqueue. The default is the job'sname(when the jobs table has the name column), so out of the box every metric series answers "which job?":The tagged columns are configurable, including custom columns an application adds to its jobs table:
Why
When
delayed.job.max_agealerts, a common first question is "which job is stuck?" (and often "whose is it?"). Previously that required querying the database by hand. Now it's amax by {queue, priority, name}away in the metrics provider.How
grouped_queryacceptsextra_group_columns, threaded into both the inner and outerGROUP BYs, so tagging costs no additional queries — still one query each forlive/pending/failedcounts per runDelayed::Monitor.tag_columns=validates columns against the schema and raisesArgumentErrorat assignment (i.e. at boot when set in an initializer) rather than failing later in the monitor run loop->-> Note this means setting it in an initializer requires a DB connection at boot <-<-
%i(name)only when the name column exists, so older schemas need no configuration and no rollout — they just emit untagged metrics until migratedNULLtag values are reported as'unset'(e.g. jobs enqueued before the column existed)Behavioral notes / compatibility
priority,queue) regardless of whether tagged series existsum by/max byaccordinglyREADME.mddocuments the behavior, the cardinality tradeoff, and a three-deploy rollout guide for adding a custom tag columnTesting
priority,queue)tag_columnsnamecolumn'unset'reportingGROUP BYshape across sqlite/mysql/postgresOther notes
Initially, the idea was to just add the ability to tag ONLY by name. This is captured in the first commit. Then the thinking evolved to enable fully customizable (metric, tag column) configurations. This was rejected as it makes more sense to have each tag column emitted across all metrics which exist instead as it seemed unlikely we would want to configure
max_age_by_nameand not, for example,failed_count_by_name. The final commit is where this effort landed so that each tagged column is emitted against each metric instead. Noting this evolution in case it (notably the second commit) helps highlight future capabilities that might be desired.