Repository navigation
docker-compose: Remove unused settings, fix the default collector config, and correct the examples - #1507
Draft
marcleblanc2 wants to merge 5 commits into
Draft
docker-compose: Remove unused settings, fix the default collector config, and correct the examples#1507marcleblanc2 wants to merge 5 commits into
marcleblanc2 wants to merge 5 commits into
Conversation
Remove environment variables no service reads: SRC_GIT_SERVERS, SEARCHER_URL, and INDEXED_SEARCH_SERVERS on worker, and UID and GID on the executor. Remove the redis-cache and redis-store Prometheus targets: no Redis exporter runs, so both targets were always down. Scrape syntactic-code-intel-worker, which serves metrics but had no target. Correct comments that named ports nothing listens on, and document that the frontend address variables also accept a replica count. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d40c-fdd9-73bd-a123-0a3e4f0dc0c3 Co-authored-by: Amp <amp@ampcode.com>
Inside the prometheus container, localhost:9323 is the container itself, so the documented Linux target was always down. Name the host through host.docker.internal, mapped to host-gateway, and have the daemon listen on an address containers can reach. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d40c-fdd9-73bd-a123-0a3e4f0dc0c3 Co-authored-by: Amp <amp@ampcode.com>
…volumes and ports The base file's collector config listens on localhost only, so no service can send it traces and Prometheus cannot scrape it. Use the mounted otel-collector/config.yaml by default, and make that file valid for the bundled collector. Remove the cache volumes, environment variables, published ports, and Caddy matcher which nothing uses, and correct the examples and comments which described them. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d40c-fdd9-73bd-a123-0a3e4f0dc0c3 Co-authored-by: Amp <amp@ampcode.com>
…ches nothing The frontend's /-/debug/ page lists the services in SRC_PROF_SERVICES. precise-code-intel-worker and syntactic-code-intel-worker both serve a debug server on 6060, but were not in the list. container:process_cpu_seconds_total:ratio_rate5m divides a series with only an instance label by engine_daemon_engine_cpus_cpus, which also has a job label, so the two sides never match and the rule records nothing, even when Prometheus scrapes the Docker daemon. Nothing reads it. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d40c-fdd9-73bd-a123-0a3e4f0dc0c3 Co-authored-by: Amp <amp@ampcode.com>
…ase http_proxy With the built-in databases removed, Prometheus keeps the three exporter targets and shows them as down. Git ignores the uppercase HTTP_PROXY for http:// URLs. Amp-Thread-ID: https://ampcode.com/threads/T-01a1d40c-fdd9-73bd-a123-0a3e4f0dc0c3 Co-authored-by: Amp <amp@ampcode.com>
Draft
5 of 6 tasks
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
An audit of every config and example file in this repository against the v8.2.0 source and a running deployment found:
workersetsSRC_GIT_SERVERS,SEARCHER_URL, andINDEXED_SEARCH_SERVERS, which only the frontend reads.zoekt-indexserver-0setsOPENTELEMETRY_DISABLEDandOTEL_EXPORTER_OTLP_ENDPOINT, but only zoekt-webserver starts a tracer. The executor setsUIDandGID, which it does not read./mnt/cachevolumes ofsourcegraph-frontend-0,sourcegraph-frontend-internal, andworkerstay empty: only searcher readsCACHE_DIR. Prometheus scrapesredis-cache:9121andredis-store:9121, but the stack has no Redis exporter, so both targets are always down.expose:on cadvisor and node-exporter does nothing on a user-defined network. The Jaeger overlay publishes 4317, 4318, and 5778 on the host, but only otel-collector sends to Jaeger, over the Compose network.logging.yaml, which binds OTLP and the metrics port to loopback, so no service can send spans and theotel-collector:8888target is down. The repository's ownotel-collector/config.yaml, which the docs call the default, is mounted but unused, and the collector rejects it (invalid keys: address).Jaegerdatasource has the wrong type (Jaeger) and a hostname that does not exist (jaeger-query). TheGF_SERVER_ROOT_URLline in the Grafana comment breaks the/-/debug/grafanaproxy, and twoGF_*lines there have stray quotes. The@httpredirect block inhttps.custom-cert.Caddyfilenever matches. The Linux steps inprometheus/README.mdscrapelocalhost:9323, which is the Prometheus container itself.external-databases: uncommenting the two IAM lines puts them at the top level of the file, which Compose rejects.migrator-operations: uncommenting acommand:line next to the active one is a duplicate key.aws-rds-databases: setsPG_CONNECTION_UPDATERon five services which never read it.aws-alb: fails withport is already allocatedunless applied with--remove-orphans.pgsqlGrafana datasource is configured by sixGRAFANA_PGSQL_*variables which appear nowhere.syntactic-code-intel-worker:6060serves metrics which Prometheus does not scrape. Several "Ports exposed" comments list ports with no listener (3443, 9121, 9187) or omit real ones.Solution
Remove what is unused, fix what is broken, and document what works.
Removed from the base file (customer override files may refer to these):
worker:SRC_GIT_SERVERS,SEARCHER_URL,INDEXED_SEARCH_SERVERSzoekt-indexserver-0:OPENTELEMETRY_DISABLED,OTEL_EXPORTER_OTLP_ENDPOINTexecutor:UID,GIDsourcegraph-frontend-0,sourcegraph-frontend-internal-0,worker, and their/mnt/cachemounts4317,4318,5778;16686staysexpose:48080, node-exporter9100redis-cache:9121,redis-store:9121container:process_cpu_seconds_total:ratio_rate5m: its two sides have different labels, so it never recorded a seriesAn override file which sets one of the removed variables still works, since Compose adds it back. An override file which mounts something else at one of the removed named volumes must now declare that volume itself. Existing volumes are left on disk;
docker volume rmremoves them.Changed behaviour:
otel-collectornow starts with--config /etc/otel-collector/config.yaml, and that file is rewritten for the collector version in the image: OTLP on0.0.0.0:4317and0.0.0.0:4318, adebugexporter, and metrics on0.0.0.0:8888. An override which already setscommandor mounts its own file over that path is unaffected. The Jaeger overlay still selects the image'sjaeger.yaml.syntactic-code-intel-worker:6060, and the frontend's/-/debug/page lists both code intel workers (SRC_PROF_SERVICES).Jaegerdatasource points athttp://jaeger:16686/-/debug/jaegerwith typejaeger.https.custom-cert.Caddyfileloses the redirect block which never matched; behaviour is unchanged.Documentation and examples:
GRAFANA_PGSQL_*variables, and that the four replica variables on the frontends take a list or a replica count.sourcegraph-uploads.envpointer.prometheus/README.md: Linux steps which work (metrics-addron0.0.0.0:9323,host.docker.internal:host-gateway).external-databasespoints ataws-rds-databasesfor IAM;aws-rds-databasessetsPG_CONNECTION_UPDATERonly on the migrator and both frontends, the services which read it;migrator-operationssays to replace thecommandline;aws-albsays to use--remove-orphans; both database examples say that Prometheus shows the three removed exporters as down;outbound-http-proxysays that Git needs lowercasehttp_proxyforhttp://code hosts.Not in this pull request:
Checklist
image:line changed)Test plan
npm ci && npm run prettier-checkpasses, and every changed YAML file parses.bffc6b7.sourcegraph/sourcegraphand the pinnedsourcegraph/zoektrevision for a reader, and found none.mainin an Amp orb running the full stack: the three cache volumes were empty; both Redis targets were down withconnection refused; the collector withlogging.yamllistened on loopback only and rejected the oldconfig.yaml; the GrafanaJaegerdatasource failed until its type and URL were changed to the values in this pull request; the custom-cert Caddyfile behaved the same without the redirect block; the README'slocalhost:9323target was down, and the new steps brought thedockertarget up; each example failure above was reproduced, and the corrected instructions ran. Evidence: base stack, examples.301672a(bffc6b7differs by three comment lines in the examples) and compared withmainatd671015(verification thread):docker compose config -qexits 0 for the base file, alone and with each of the jaeger, executors, and dev files and each of the six examples, and fordb-only-migrate;.buildkite/validate-docker-compose.shexits 0.main. The frontend services andworkerlog nothing about/mnt/cacheor missing service addresses, andworkerstill connects to gitserver, searcher, and zoekt. A repository clones and indexes; literal and symbol searches return matches.main,otel-collector:8888and both Redis targets are down.config.yaml, listens on all interfaces on 4317, 4318, and 8888, and accepted and sent 707 spans./-/debug/jaegerproxy, and by the Grafana Jaeger datasource (main's datasource file returns 500 "Unable to find datasource plugin").main.prometheus/README.mdLinux steps: thedockertarget is up; with127.0.0.1:9323it is down, as the README says.migrator-operations: theadd-logcommand exits 0 and adds themigration_logsrow.GRAFANA_PGSQL_*lines make the datasource'sSELECT 1return a row; without them it returns 500./-/debug/links both code intel workers and both links return 200;docker_rules.ymlloads 3 rules with healthok.aws-rds-databasesexample against real RDS with IAM (source-backed only); upgrading a deployment whose three/mnt/cachevolumes already exist.