chore: Modernize signals pgbouncer-mixin - #1663
Open
postnati wants to merge 4 commits into
Open
Conversation
|
|
postnati
force-pushed
the
chore/pgbouncer-mixin-modernization
branch
from
July 16, 2026 13:03
fbaf322 to
8775594
Compare
postnati
marked this pull request as ready for review
July 16, 2026 13:03
Member
|
Hi @postnati just a quick note that all commits must be signed with a valid GPG key in order to unblock merging, will be needed across all your PRs |
schmikei
reviewed
Jul 16, 2026
schmikei
left a comment
schmikei
left a comment
Contributor
There was a problem hiding this comment.
Aside from rebasing your commits to be signed I think that the changes here look relatively straightforward!
Dasomeone
requested changes
Jul 23, 2026
Dasomeone
left a comment
Dasomeone
left a comment
Member
There was a problem hiding this comment.
Needs the same overhaul as mentioned in comments around panels here #1668 (review)
postnati
force-pushed
the
chore/pgbouncer-mixin-modernization
branch
2 times, most recently
from
July 24, 2026 14:50
240f1da to
b5b3aa3
Compare
Contributor
Author
|
Updated and ready for review. |
Contributor
Author
|
Converted the signals from raw to typed and used appropriate aggFunction, rangeFunction, and exprWrappers. |
Addresses review feedback (Case 2, the asTarget() feedback). Case 1 did not
apply: the signal files already spell out every signal object, with no local
signal-builder helper to expand.
Single-signal stat and timeSeries panels now use signals.<group>.<name>.asStat()
/ .asTimeSeries() + commonlib.panels.generic.<flavor>.stylize() + the unchanged
panel-specific mixins, instead of commonlib...base.new(targets=[sig.asTarget()]).
Panel title/unit/description are sourced from the signal spec; panel-level
withUnit() and inline description/targets are removed. Two signals shared between
a stat (no unit) and a timeSeries (with unit) keep their spec matched to the stat
and set title/unit at the timeSeries call site via withName()/withUnit().
Panels that must stay target-based (documented in panels.libsonnet): the three
multi-signal timeSeries (Network traffic, Server connections, Top databases by
network traffic) and the alertList panel — common-lib provides no signal
constructor for multi-series traffic panels or alert lists.
Verified with a Python per-panel comparator against the grafana/master
pre-modernization baseline: every panel of every dashboard matches modulo the
excluded editor/id/datasource fields. logs.json is byte-identical. Accepted
output deltas on converted panels:
1. one per-query fieldConfig override (matcher byFrameRefID, single unit
property == signal unit) plus fieldConfig.defaults.unit (== '' / absent on
the unit-less stat panels);
2. converted timeSeries descriptions drop the trailing newline that the legacy
||| block descriptions carried (cosmetic; markdown renders identically).
No expr, legend, title, or interval deltas.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…signal specs
Addresses the "everything is type: raw" review feedback. Raw signals drop from
7/22 (32%) to 3/24 (12%).
signals/cluster.libsonnet: four of the five "top database" signals were raw with
the whole legacy expression baked in. They are now typed — top_database_active_
connection as gauge, and top_database_query_processed / _network_received /
_network_sent as counter (rate is the framework default) — with the topk
by-clause and $top_database_count supplied through exprWrappers. The wrapper is
written out inline on each signal rather than hoisted into a local.
top_database_query_duration stays raw, as do stats_query_avg_duration and
stats_transaction_avg_duration: all three are a ratio of two range vectors
(1000 * increase(a) / clamp_min(increase(b), 1)), which no typed signal can
express. Only the topk wrapper is factored out of the cluster one.
Query shape moved out of panels.libsonnet and into the signal specs. The stat
panels need a bare sum() with no by-clause, which the aggregation template
cannot emit, so those signals carry exprWrappers: [['sum(', ')']] and an empty
legendCustomTemplate. Where one metric is shown both as a summed stat and as
per-database series, each view is now its own signal (the _total suffix marks
the summed one) instead of one signal mutated at two call sites. panels.libsonnet
now has no call-site signal mutation at all: every single-signal panel is
signals.<group>.<name>.asStat()/.asTimeSeries() + stylize().
Intentional semantic change: the three hand-written raw signals that use
increase() now carry the same window the framework emits for a typed increase
counter — [$__interval:] offset -$__interval, in both the numerator and the
denominator of each ratio. The negative offset shifts the window forward so the
newest bucket is complete rather than partial, so "Queries average duration",
"SQL average transaction duration" and "Top databases by average query duration"
stop under-reporting at the right-hand edge. Both halves of each ratio shift by
the same amount, so the ratio stays coherent.
Verified with a Python leaf-path comparator against both the branch HEAD baseline
and the grafana/master pre-modernization baseline. Against master, every panel
matches on title, gridPos, panel type, legend and target count, with these
categories of delta and nothing else:
21 expr: trailing-comma / whitespace only
10 description: trailing newline dropped (cosmetic, markdown identical)
6 stat panel unit: None <-> '' (renders identically)
3 expr: + offset -$__interval (the intentional change above)
The offset exprs were re-compared with the offset tokens stripped back out and
then match master exactly, confirming it is the only semantic addition.
prometheus_rules_out/ is byte-unchanged; the alerts are threshold comparisons on
instant gauges with no range functions and reference no signals. make fmt, make
lint and make -B all are clean and the build is reproducible.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
postnati
force-pushed
the
chore/pgbouncer-mixin-modernization
branch
from
August 6, 2026 16:16
a36a3fc to
76aeead
Compare
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
4 participants
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.
This PR modernizes pgbouncer-mixin to use the signals architecture pattern.