Support external database modules - #176
Open
Areson wants to merge 5 commits into
Open
Conversation
This was referenced Aug 7, 2026
Areson
marked this pull request as ready for review
August 7, 2026 19:57
aparajon
approved these changes
Aug 7, 2026
aparajon
left a comment
Collaborator
There was a problem hiding this comment.
🤖 Reviewed on Armand's behalf — full read of the diff plus a local run of the entire suite with -race against the MySQL 8.0/8.4 test containers: all green.
What I verified:
- Registry + config gating fail closed.
RegisterDatabaseModulerejects nil/mysql/*/invalid names; external database types reject MySQL-only config (socket, my.cnf, heartbeat, exporter,plans.table) and require a registered module;Redacted()nils the opaqueDatabaseConfigso secrets can't leak through config dumps. - Credentials parity. The new engine-neutral
credentialspackage preserves the existing MySQL precedence (IAM → Secrets Manager → password file → static), including the IAM default-port behavior indbconn. - Plan/monitor compatibility. DB-type validation runs both at plan load and at
Plan()selection, so a shared plan can't silently attach to an incompatible monitor. - Monitor lifecycle. The generation-scoped
stopRunmeans a stale subsystem can't tear down a newer monitor generation, andcloseDB()on stop/restart fixes a pre-existing*sql.DBleak on subsystem restart. The engine'scollectorWGjoin plusExporter.Stop()close the remaining cleanup gaps. I also walked the stop path for deadlocks (wg.Wait under runMux vs the run goroutine's LIFO defers) — it's sound.
Two non-blocking notes:
interpolateEnv/interpolateMonswitching toReplaceAllStringFuncis a quiet bugfix: previously a value with multiple placeholders had every match replaced with the first var's value, and$in replacement values could be regex-expanded. Strictly more correct, but a config relying on the old quirk would change behavior.loader.monitorTypesentries are added onLoadMonitorbut never pruned when a monitor goes away, so a long-lived loader's shared-plan validation set can only grow. Harmless today; worth a TODO if monitor churn ever matters.
Reviewed by Armand's AI agent (Claude Fable 5).
Introduce explicit monitor database typing and PostgreSQL configuration while preserving MySQL defaults. Extract shared credential source construction without changing MySQL-specific reload behavior. Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true Co-authored-by: Goose <opensource@block.xyz>
Read database compatibility from an optional collector factory capability while defaulting legacy factories to MySQL. Reject plans whose collectors have no common database type, then validate the selected plan against each monitor before collector preparation. Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true
* Configure PostgreSQL database discovery Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true Co-authored-by: Goose <opensource@block.xyz> * Add monitor-owned database providers Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true * Keep provider constructors internal Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true * Harden PostgreSQL monitor lifecycle (#174) * Guard PostgreSQL monitor configuration Keep MySQL-only heartbeat and plan defaults off PostgreSQL monitors, reject explicit unsupported settings, and sign IAM tokens with the database-specific default port. Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true * Close PostgreSQL integration gaps Cancel and join plan preparation before closing monitor-owned database resources, and reject the remaining MySQL-only plan configurations for PostgreSQL. Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true * Require PostgreSQL exporter plans Avoid assigning the MySQL default exporter plan to PostgreSQL monitors. Require a named plan whenever exporter mode is enabled for PostgreSQL while preserving the public and MySQL defaulting behavior. Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true * Coordinate monitor subsystem teardown Roll back partial startup failures, bind subsystem stops to their startup generation, and retain exporter engines for cleanup before database providers close. Collector cleanup now also runs when a prepared collector is idle. Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true * Join collectors before provider shutdown Track every engine collector goroutine, cancel active runs during teardown, and wait for foreground or ErrMore background work before invoking cleanup and releasing the database provider. Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true * Generalize external database modules (#175) * Generalize external database modules Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true * Interpolate typed database config values Preserve named and typed containers when Blip expands environment and monitor placeholders in opaque module configuration. This keeps programmatic config loaders consistent with YAML-loaded config. Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true --------- Co-authored-by: Codex <noreply@openai.com> --------- Co-authored-by: Codex <noreply@openai.com> --------- Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Goose <opensource@block.xyz>
Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true
Co-authored-by: Codex <noreply@openai.com> Ai-assisted: true
Areson
force-pushed
the
ioberst/bgblip-module
branch
from
August 7, 2026 23:01
907ad5f to
0c4500e
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
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.
Why
Allow official external database modules to reuse Blip's monitor, plan, collection, transformation, and sink runtime while keeping Blip itself purpose-built for MySQL and preserving existing MySQL behavior.
What
Risk Assessment
Medium — this introduces public extension seams and adjusts monitor lifecycle internals, but Blip does not register or ship another database engine. Existing configurations, collectors, and factories retain MySQL behavior by default, and external database support remains explicitly opt-in.
Generated with Codex