feat(api): publish the admin API on an opt-in Service, and make self-metrics serve - #21
Draft
tdakkota wants to merge 2 commits into
Draft
feat(api): publish the admin API on an opt-in Service, and make self-metrics serve#21tdakkota wants to merge 2 commits into
tdakkota wants to merge 2 commits into
Conversation
…-metrics The admin API is served in every storage pod (bind defaults to :8090, and the block defaults unconditionally) but nothing routed to it. Before #13 it was reachable by accident, under a port named "metrics"; moving self-metrics to 9464 took that with it. spec.admin now publishes it on a dedicated <name>-admin Service, absent by default: the API triggers MaintainNow/CompactNow and StreamCosts, so it must not ride the client Service. The Service targets 8090 by number, leaving the pod template untouched. Also sets OTEL_METRICS_EXPORTER=prometheus. go-faster/sdk defaults it to otlp and only starts the /metrics server when it names prometheus, so 9464 was published and dead. This is a behaviour change: self-metrics previously left over OTLP only.
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.
Closes two related gaps left by #12 and #13.
1. The admin API is served but unreachable
oteldb registers the admin API unconditionally (
cmd/oteldb/admin.go: "It is alwaysregistered") and
config.Admin.SetDefaultsbinds:8090whether or not the config declares theblock. So every storage pod is already serving it — build info, health, runtime and storage
statistics,
MaintainNow/CompactNow, and the per-stream cost report — and no Service routed toit.
Before #13 it was reachable by accident: it was published under a port named
metrics. #13 movedself-metrics to 9464 and took the accidental exposure with it. It went from reachable-by-accident to
unreachable, and neither state was a decision.
spec.admin— a separate<name>-adminService, absent by default.Why separate and opt-in rather than a port on the client Service:
runActiontriggers the engine's maintenance and compaction passes, andgetStreamCostsisdocumented upstream as the heaviest call the storage engine exposes — it decodes every
accounted byte column of every live part. That must not ride the Service PromQL and OTLP share,
where an ingress or a broad NetworkPolicy picks it up by default.
ClusterIPwhile the client Service is aLoadBalancer, with its own annotations (auth proxy, internal-only LB).app.kubernetes.io/component: adminlabel lets a NetworkPolicy or ServiceMonitor selectit without matching client traffic.
Toggling it does not roll the StatefulSet. The Service targets
8090by number rather thandeclaring a matching container port: the listener exists regardless, so a container port would
document nothing new while changing the pod template.
TestAdminDoesNotRollPodspins that, and thatspec.adminrenders no config at all.The API has no auth of its own beyond
spec.extraConfig'sauthblock — the README says to treatthe Service as privileged and restrict it with a NetworkPolicy.
2. Port 9464 was published and served nothing
#12 established that
OTEL_EXPORTER_PROMETHEUS_HOST/PORTare set butOTEL_METRICS_EXPORTERneveris. go-faster/sdk's
autometerdefaults that variable tootlp(getEnvOr(envName, expOTLP)) andonly constructs the Prometheus reader — and with it the
/metricsserver — when it namesprometheus. So the port was published on all three Services and dead.This sets
OTEL_METRICS_EXPORTER=prometheuson storage, ingest and query pods.Unlike
spec.adminthis one is unconditional and does roll the pods — it is a fix, not a stanza:the current state is a published port that cannot answer.
Stack
Based on #20 (
spec.ingest.tenant) → #19 (spec.policy.ec) → #17 (spec.cluster.privateBackend) →#15 (
spec.query) → #14 (spec.ingest). This PR depends on #14/#15 only forstatelessPodEnv,which is where the exporter variable lands for the two stateless pools; the rest is stack ordering.
This is the tip of the stack — e2e runs here.