Skip to content

Support external database modules - #176

Open
Areson wants to merge 5 commits into
mainfrom
ioberst/bgblip-module
Open

Support external database modules#176
Areson wants to merge 5 commits into
mainfrom
ioberst/bgblip-module

Conversation

@Areson

@Areson Areson commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

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

  • Add opt-in database module registration, opaque module configuration with per-placeholder interpolation, and strict module-owned validation
  • Add engine-neutral credential callbacks and composable database/provider factories, including monitor-owned multi-database providers and strict provider validation
  • Add optional collector database compatibility metadata and validate plans against their monitors
  • Reject MySQL-only subsystems for external monitors while retaining MySQL defaults for existing configurations and implementations
  • Document the external module contract and cover its configuration, lifecycle, compatibility, and rollback paths

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

@Areson Areson changed the title Add PostgreSQL module foundations Support external database modules Aug 7, 2026
@Areson
Areson marked this pull request as ready for review August 7, 2026 19:57

@aparajon aparajon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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. RegisterDatabaseModule rejects 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 opaque DatabaseConfig so secrets can't leak through config dumps.
  • Credentials parity. The new engine-neutral credentials package preserves the existing MySQL precedence (IAM → Secrets Manager → password file → static), including the IAM default-port behavior in dbconn.
  • 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 stopRun means a stale subsystem can't tear down a newer monitor generation, and closeDB() on stop/restart fixes a pre-existing *sql.DB leak on subsystem restart. The engine's collectorWG join plus Exporter.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/interpolateMon switching to ReplaceAllStringFunc is 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.monitorTypes entries are added on LoadMonitor but 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).

Areson and others added 5 commits August 7, 2026 16:01
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
Areson force-pushed the ioberst/bgblip-module branch from 907ad5f to 0c4500e Compare August 7, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants