feat: expose worker Prometheus metrics for matching and eval ingest#1184
feat: expose worker Prometheus metrics for matching and eval ingest#1184DarshanCode2005 wants to merge 11 commits into
Conversation
Application metrics panels filter job=node, but Instance was sourced only from sql_delta. List all scrape targets via label_values(up,instance) so node_exporter can be selected
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if port is None: | ||
| return | ||
| start_http_server(port) | ||
| _metrics_server_started = True |
|
|
||
| script = '' | ||
| mkdir -p "$PROMETHEUS_MULTIPROC_DIR" | ||
| wst-manage serve_worker_metrics |
There was a problem hiding this comment.
My hunch is this could be an ad hoc more-or-less-inline Python HTTP server here, which we put behind the proxy. Seems kind of wrong to weave this into the application code.
| def observe_matching(duration_seconds: float, candidates: int) -> None: | ||
| """Record matching wall time and candidate cardinality.""" | ||
| _matching_duration.observe(duration_seconds) | ||
| _matching_candidates.observe(float(candidates)) | ||
|
|
||
|
|
||
| def observe_eval_batch_ingest(duration_seconds: float, attributes: int) -> None: | ||
| """Record eval batch ingest wall time and batch size.""" | ||
| _eval_batch_duration.observe(duration_seconds) | ||
| _eval_batch_attributes.observe(float(attributes)) |
There was a problem hiding this comment.
These and their subroutines are used exactly once, they may as well be done in place. Rule of thumb: if there's no more than 2 repetitions, extracting (especially into a different place) almost never makes sense. Uncle-Bob-ing a bit for readability is not wrong, but putting into a separate file and wrapping a constructor in a procedure brings no benefit here.
| self.stdout.write("\n[1/6] Deleting stale matches") | ||
| self._delete_stale_matches(cutoff, batch_size, dry_run) | ||
| step_start = time.time() | ||
| for kind, count in self._delete_stale_matches( |
There was a problem hiding this comment.
Why not have the helper return its own duration? Then you get rid of the boilerplate at the call site.
And maybe you could also collect the returned dicts kind: count into a list and just sum them up at the end in one go? Then you don't need DELETED_KINDS at all, which confused me quite a bit until I understood what you're trying to do.
3 participants
Description
Related to #1002. Follows #1141 and #1183.
Problem
Oneshot batch jobs already export last-run gauges through the textfile collector (
job="node"). Long-running listen workers (matching and evaluation ingest) only log durations, so operators have no live Prometheus/Grafana visibility into these hot paths.What this PR does
Adds live
prometheus_clientHTTP/metricsendpoints for listen workers and instruments the key execution paths.Exports:
sectracker_matching_duration_secondssectracker_matching_candidatessectracker_nix_evaluation_batch_ingest_duration_secondssectracker_nix_evaluation_batch_ingest_attributesAlso:
METRICS_HTTP_PORT; metrics server starts only inlisten --workerprocesses:9252and evaluator multiprocess metrics on:9253sectracker-workerandsectracker-evaluatorjobs