fix(observability): keep prometheus in profile set when enabling grafana sidecar - #238
Closed
srikanth-bitdynamics wants to merge 1 commit into
Closed
fix(observability): keep prometheus in profile set when enabling grafana sidecar#238srikanth-bitdynamics wants to merge 1 commit into
srikanth-bitdynamics wants to merge 1 commit into
Conversation
…ana sidecar
Enabling grafana via the observability toggle ran `docker compose
--profile grafana up -d grafana`, which filtered prometheus out of the
project model and broke grafana's `depends_on: prometheus`
("depends on undefined service prometheus: invalid compose project").
This failed even the enable-both path, where prometheus comes up fine
and the separate grafana invocation then rejects the project.
Activate the prometheus profile alongside grafana whenever grafana is
started, and scope the up with --no-deps so activating that profile
makes the dependency target defined without dragging a prometheus
container up in a grafana-only (external-scrape) setup. Both the CLI
verb and the Web UI Metrics toggle go through the shared
SetObservability, so parity holds.
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.
Problem
dpm localnet observability enable --name <inst>(and the Web UI Metrics "Enable observability now" button, which shares the orchestration) fails on a running instance with:Reproduced on Splice 0.6.4, Compose v5.0.2, on a healthy instance created without
--profile observability.Root cause
SetObservabilitybrings up prometheus and grafana in two separatedocker compose upinvocations, each activating only its own per-component profile. The grafana step ran--profile grafana up -d grafana, but the overlay (assets/compose/observability.yaml) gatesprometheusbehindprofiles: ["observability","prometheus"]and grafana declaresdepends_on: prometheus. With only thegrafanaprofile active, Compose filters prometheus out of the project model, so grafana'sdepends_onreferences an undefined service and the whole project is rejected. This bit even the enable-both path: prometheus comes up fine, then the separate grafana invocation fails.Fix
Whenever grafana is started, activate both
--profile prometheus --profile grafanaso prometheus is a defined service (satisfyingdepends_on), and scope the up with--no-depsso activating that profile does not drag a prometheus container up in the legitimate grafana-only / external-scrape case (which the existing warning documents).observabilityUpProfiles+observabilityUpArgshelpers build the argv;enableObservabilitySidecardrops its now-redundantprofileparam.depends_on— it still orders startup in the umbrellaup.gopath.SetObservability, so CLI ↔ Web UI parity holds with no per-surface docker drift.Verified scenarios: grafana onto an already-running prometheus, both from cold, and grafana-only (prometheus defined-but-not-started, warning still surfaced).
Test
TestObservabilityUpArgs_GrafanaKeepsPrometheusInProfileSetmaterializes the real overlay (injecting abytes.Bufferfor the drift writer per the repo testing rule) and asserts the generated grafanaupargv keeps prometheus in the active profile set and passes--no-deps, while a prometheus-only up does not activate the grafana profile.make testandmake lintare green.Related (not fixed here)
The same root cause also affects cold-start
dpm localnet up --profile grafana(grafana-only):up.goruns a whole-project up under--profile grafanawith prometheus's profile inactive, producing the identical rejection. That path can't use the toggle's--no-depstrick (no explicit service list), so the clean fix there is to drop grafana'sdepends_onand rely on runtime datasource discovery — a separate change to the overlay's dependency contract, left out of this scoped fix.