From f7de49697293a664960071a8a6e9b240084853d8 Mon Sep 17 00:00:00 2001 From: Ian Kerins Date: Mon, 9 Oct 2023 17:14:52 -0400 Subject: [PATCH] Add prometheus metrics This results in some complications. See the README and the new `main.js` entrypoint. --- .github/workflows/ci.yml | 2 +- README.md | 16 ++++ demo/Dockerfile.neogrok | 7 +- demo/Dockerfile.neogrok.dockerignore | 1 + demo/fly.neogrok.toml | 5 ++ main.js | 89 +++++++++++++++++++++++ package-lock.json | 39 +++++++++- package.json | 4 + src/hooks.server.ts | 34 ++++++++- src/lib/server/configuration.ts | 4 + src/lib/server/metrics.ts | 55 ++++++++++++++ src/lib/server/search-api.ts | 10 +-- src/lib/server/zoekt-client.ts | 35 +++++++++ src/lib/server/zoekt-list-repositories.ts | 10 +-- src/routes/metrics/+server.ts | 17 +++++ 15 files changed, 303 insertions(+), 25 deletions(-) create mode 100644 main.js create mode 100644 src/lib/server/metrics.ts create mode 100644 src/lib/server/zoekt-client.ts create mode 100644 src/routes/metrics/+server.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dd3453d..87f3e38 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 with: - node-version: 18.x + node-version: 20.x - run: npm install - run: npm run check - run: npm run lint diff --git a/README.md b/README.md index 3b9bbaa..c07508f 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,22 @@ Note that you can also configure, among other things, which ports/addresses will be bound, using SvelteKit's Node environment variables. See the list [here](https://kit.svelte.dev/docs/adapter-node#environment-variables). +### Prometheus metrics + +Neogrok exports some basic [Prometheus](https://prometheus.io/) +[metrics](./src/lib/server/metrics.ts) on an opt-in basis, by setting a +`PROMETHEUS_PORT` or `PROMETHEUS_SOCKET_PATH`, plus an optional +`PROMETHEUS_HOST`. These variables have the exact same semantics as the +above-described SvelteKit environment variables, but the port/socket must be +different than those of the main application. When opting in with these +variables, `/metrics` will be served. + +`/metrics` is required to be served with a different port/socket so as to not +expose it on the main site; serving one port to end users and another to the +prometheus scraper is the easiest way to ensure proper segmentation of the +neogrok site from internal infrastructure concerns, without having to run a +particularly configured HTTP reverse proxy in front of neogrok. + ## OpenGrok compatibility As an added bonus, neogrok can serve as a replacement for existing deployments diff --git a/demo/Dockerfile.neogrok b/demo/Dockerfile.neogrok index 4058b9b..7608fa4 100644 --- a/demo/Dockerfile.neogrok +++ b/demo/Dockerfile.neogrok @@ -1,13 +1,14 @@ -FROM node:18-slim as app-builder +FROM node:20-slim as app-builder WORKDIR /app COPY . . RUN npm install \ && npm run build \ && npm install --omit=dev -FROM gcr.io/distroless/nodejs18-debian11 +FROM gcr.io/distroless/nodejs20-debian12 WORKDIR /app COPY package.json . +COPY main.js . COPY --from=app-builder /app/build build COPY --from=app-builder /app/node_modules node_modules -ENTRYPOINT ["/nodejs/bin/node", "build"] +ENTRYPOINT ["/nodejs/bin/node", "main.js"] diff --git a/demo/Dockerfile.neogrok.dockerignore b/demo/Dockerfile.neogrok.dockerignore index 7bc1333..3f7bcdf 100644 --- a/demo/Dockerfile.neogrok.dockerignore +++ b/demo/Dockerfile.neogrok.dockerignore @@ -9,3 +9,4 @@ !tailwind.config.js !vite.config.ts !demo/Dockerfile.neogrok* +!main.js diff --git a/demo/fly.neogrok.toml b/demo/fly.neogrok.toml index 6653e30..ad5d54f 100644 --- a/demo/fly.neogrok.toml +++ b/demo/fly.neogrok.toml @@ -6,6 +6,7 @@ primary_region = "ewr" [env] PORT = 8080 + PROMETHEUS_PORT = 9901 ZOEKT_URL = "http://neogrok-demo-zoekt.internal:8080" [experimental] @@ -36,3 +37,7 @@ primary_region = "ewr" interval = "15s" restart_limit = 0 timeout = "2s" + +[metrics] +port = 9091 +path = "/metrics" diff --git a/main.js b/main.js new file mode 100644 index 0000000..4b66b34 --- /dev/null +++ b/main.js @@ -0,0 +1,89 @@ +// This is the main "production" entrypoint into neogrok. It wraps the SvelteKit +// server to do some additional things. Note that the vite dev server used by +// SvelteKit bypasses this module entirely, as it's a fundamentally different +// entrypoint. +// +// See https://kit.svelte.dev/docs/adapter-node#custom-server for details. + +import { createServer } from "node:http"; +import { register } from "prom-client"; +import { handler } from "./build/handler.js"; +import { env } from "./build/env.js"; + +// These are copied straight out of the default SvelteKit entrypoint. +const path = env("SOCKET_PATH", false); +const host = env("HOST", "0.0.0.0"); +const port = env("PORT", !path && "3000"); + +const abortController = new AbortController(); +// The default SvelteKit node server does not handle standard "well-behaved http +// server signals". +process.once("SIGTERM", () => { + console.log( + "Received SIGTERM, draining connections in an attempt to gracefully shut down...", + ); + abortController.abort(); +}); +process.once("SIGINT", () => { + console.log( + "Received SIGINT, draining connections in an attempt to gracefully shut down...", + ); + abortController.abort(); +}); + +// @ts-expect-error the polka handler has an additional `next` property that is +// not actually used by the implementation. +const server = createServer(handler); +server.listen({ host, port, path, signal: abortController.signal }, () => { + console.log(`Listening on ${path ? path : host + ":" + port}`); +}); + +// Support binding a prometheus /metrics server on a different port/path, so +// that it is not exposed on the site. +const promPath = env("PROMETHEUS_SOCKET_PATH", false); +const promHost = env("PROMETHEUS_HOST", "0.0.0.0"); +const promPort = env("PROMETHEUS_PORT", false); +if (promPort && promPort === port) { + throw new Error( + `PROMETHEUS_PORT ${promPort} cannot be the same as PORT ${port}`, + ); +} +if (promPath && promPath === path) { + throw new Error( + `PROMETHEUS_SOCKET_PATH ${promPath} cannot be the same as SOCKET_PATH ${path}`, + ); +} +if (promPort || promPath) { + const promServer = createServer((req, res) => { + if (req.method === "GET" && req.url?.endsWith("/metrics")) { + res.statusCode = 200; + res.setHeader("content-type", register.contentType); + register.metrics().then( + (body) => res.end(body), + (err) => { + console.error("failed to generate prometheus /metrics response", err); + res.writeHead(500, "Internal Server Error"); + res.end(); + }, + ); + } else { + res.writeHead(404, "Not found"); + res.end("Not found"); + } + }); + promServer.listen( + { + host: promHost, + port: promPort, + path: promPath, + signal: abortController.signal, + }, + () => { + console.log( + `Prometheus listening on ${ + promPath ? promPath : promHost + ":" + promPort + }`, + ); + }, + ); +} diff --git a/package-lock.json b/package-lock.json index 24d3e0a..172858f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,11 @@ "@badrap/valita": "0.3.0", "lucene": "2.1.1", "lucide-svelte": "0.256.1", - "pretty-bytes": "6.1.1" + "pretty-bytes": "6.1.1", + "prom-client": "15.0.0" + }, + "bin": { + "neogrok": "main.js" }, "devDependencies": { "@sveltejs/adapter-node": "1.3.1", @@ -279,6 +283,14 @@ "node": ">= 8" } }, + "node_modules/@opentelemetry/api": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.6.0.tgz", + "integrity": "sha512-OWlrQAnWn9577PhVgqjUvMr1pg57Bc4jv0iL4w0PRuOSRvq67rvHW9Ie/dZVMvCzhSCB+UxhcY/PmCmFj33Q+g==", + "engines": { + "node": ">=8.0.0" + } + }, "node_modules/@polka/url": { "version": "1.0.0-next.23", "resolved": "https://registry.npmjs.org/@polka/url/-/url-1.0.0-next.23.tgz", @@ -1003,6 +1015,11 @@ "node": ">=8" } }, + "node_modules/bintrees": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/bintrees/-/bintrees-1.0.2.tgz", + "integrity": "sha512-VOMgTMwjAaUG580SXn3LacVgjurrbMme7ZZNYGSSV7mmtY6QQRh0Eg3pwIcntQ77DErK1L0NxkbetjcoXzVwKw==" + }, "node_modules/brace-expansion": { "version": "1.1.11", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", @@ -2866,6 +2883,18 @@ "url": "https://github.com/chalk/ansi-styles?sponsor=1" } }, + "node_modules/prom-client": { + "version": "15.0.0", + "resolved": "https://registry.npmjs.org/prom-client/-/prom-client-15.0.0.tgz", + "integrity": "sha512-UocpgIrKyA2TKLVZDSfm8rGkL13C19YrQBAiG3xo3aDFWcHedxRxI3z+cIcucoxpSO0h5lff5iv/SXoxyeopeA==", + "dependencies": { + "@opentelemetry/api": "^1.4.0", + "tdigest": "^0.1.1" + }, + "engines": { + "node": "^16 || ^18 || >=20" + } + }, "node_modules/punycode": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/punycode/-/punycode-2.3.0.tgz", @@ -3556,6 +3585,14 @@ "node": ">= 14" } }, + "node_modules/tdigest": { + "version": "0.1.2", + "resolved": "https://registry.npmjs.org/tdigest/-/tdigest-0.1.2.tgz", + "integrity": "sha512-+G0LLgjjo9BZX2MfdvPfH+MKLCrxlXSYec5DaPYP1fe6Iyhf0/fSmJ0bFiZ1F8BT6cGXl2LpltQptzjXKWEkKA==", + "dependencies": { + "bintrees": "1.0.2" + } + }, "node_modules/text-table": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/text-table/-/text-table-0.2.0.tgz", diff --git a/package.json b/package.json index 3478e2b..908071a 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,9 @@ "format": "prettier --write .", "test": "vitest" }, + "bin": { + "neogrok": "./main.js" + }, "author": "Ian Kerins ", "license": "MIT", "devDependencies": { @@ -37,6 +40,7 @@ }, "dependencies": { "@badrap/valita": "0.3.0", + "prom-client": "15.0.0", "lucene": "2.1.1", "lucide-svelte": "0.256.1", "pretty-bytes": "6.1.1" diff --git a/src/hooks.server.ts b/src/hooks.server.ts index 2078d54..c0de7f4 100644 --- a/src/hooks.server.ts +++ b/src/hooks.server.ts @@ -1,16 +1,42 @@ import { building } from "$app/environment"; import { resolveConfiguration } from "$lib/server/configuration"; -import type { HandleServerError } from "@sveltejs/kit"; +import { + neogrokRequestCount, + neogrokRequestDuration, + neogrokRequestConcurrency, +} from "$lib/server/metrics"; +import type { Handle, HandleServerError } from "@sveltejs/kit"; if (!building) { + // This seems to be the magic way to do truly one-time setup in both dev and + // prod. + // Resolve the configuration on startup, such that startup fails if the // configuration is invalid. We do this here because this hooks module runs on - // service startup, but not the build, unlike the configuration module. + // service startup, but not during the build, which is the case in most any + // other module. await resolveConfiguration(); } -// TODO we should have prom metrics, on, among other things, HTTP requests -// handled, and the `handle` hook would be a good way to do that. +// Handle request metrics on all SvelteKit requests. +export const handle: Handle = async ({ event, resolve }) => { + const routeLabel = event.route.id ?? "null"; + try { + neogrokRequestConcurrency.labels(routeLabel).inc(); + + const start = Date.now(); + const response = await resolve(event); + const durationSeconds = (Date.now() - start) / 1000; + + const labels = [routeLabel, response.status.toString()]; + neogrokRequestCount.labels(...labels).inc(); + neogrokRequestDuration.labels(...labels).inc(durationSeconds); + + return response; + } finally { + neogrokRequestConcurrency.labels(routeLabel).dec(); + } +}; // SvelteKit logs an error every time anything requests a URL that does not map // to a route. Bonkers. Override the default behavior to exclude such cases. diff --git a/src/lib/server/configuration.ts b/src/lib/server/configuration.ts index 01d7fee..b348d09 100644 --- a/src/lib/server/configuration.ts +++ b/src/lib/server/configuration.ts @@ -34,6 +34,10 @@ type Configuration = { // configuration will have to avoid dereferencing it in the module scope. export let configuration: Configuration; export const resolveConfiguration: () => Promise = async () => { + if (configuration !== undefined) { + return; + } + const configFilePath = process.env.NEOGROK_CONFIG_FILE ?? defaultConfigFilePath; let fileConfig: FileConfiguration | undefined; diff --git a/src/lib/server/metrics.ts b/src/lib/server/metrics.ts new file mode 100644 index 0000000..936d01a --- /dev/null +++ b/src/lib/server/metrics.ts @@ -0,0 +1,55 @@ +import { dev } from "$app/environment"; +import { + Counter, + Gauge, + collectDefaultMetrics, + Registry, + register, +} from "prom-client"; + +// The global registry persists across HMR reloads of this module, throwing on +// every reload due to duplicate names. So, in dev, use a registry scoped to +// this module. In prod, we can't do that as the entrypoint to the application, +// which mounts the prom /metrics server there, can't import from sveltekit +// application chunks. Thankfully, there is no HMR in prod, so we can just use +// the global registry in that case. +export const registry = dev ? new Registry() : register; + +collectDefaultMetrics({ register: registry }); +export const zoektRequestCount = new Counter({ + name: "zoekt_http_requests_total", + help: "Count of http requests made to zoekt", + labelNames: ["path", "status"], + registers: [registry], +}); +export const zoektRequestDuration = new Counter({ + name: "zoekt_http_request_seconds_total", + help: "Duration of http requests made to zoekt", + labelNames: ["path", "status"], + registers: [registry], +}); +export const zoektRequestConcurrency = new Gauge({ + name: "zoekt_http_requests", + help: "Gauge of concurrent requests being made to zoekt", + labelNames: ["path"], + registers: [registry], +}); + +export const neogrokRequestCount = new Counter({ + name: "neogrok_http_requests_total", + help: "Count of http requests handled by neogrok", + labelNames: ["route", "status"], + registers: [registry], +}); +export const neogrokRequestDuration = new Counter({ + name: "neogrok_http_request_seconds_total", + help: "Duration of http requests handled by neogrok", + labelNames: ["route", "status"], + registers: [registry], +}); +export const neogrokRequestConcurrency = new Gauge({ + name: "neogrok_http_requests", + help: "Gauge of concurrent requests being handled by neogrok", + labelNames: ["route"], + registers: [registry], +}); diff --git a/src/lib/server/search-api.ts b/src/lib/server/search-api.ts index c865347..fbb5c1d 100644 --- a/src/lib/server/search-api.ts +++ b/src/lib/server/search-api.ts @@ -1,11 +1,11 @@ import * as v from "@badrap/valita"; import type { ReadonlyDeep } from "type-fest"; -import { configuration } from "./configuration"; import { type ContentToken, parseChunkMatch, parseFileNameMatch, } from "./content-parser"; +import { makeZoektRequest } from "./zoekt-client"; export const searchQuerySchema = v.object({ query: v.string(), @@ -45,13 +45,7 @@ export const search = async ( }, }); - const response = await f(new URL("/api/search", configuration.zoektUrl), { - method: "POST", - headers: { - "content-type": "application/json", - }, - body, - }); + const response = await makeZoektRequest(f, "/api/search", body); if (!response.ok) { if (response.status === 400) { diff --git a/src/lib/server/zoekt-client.ts b/src/lib/server/zoekt-client.ts new file mode 100644 index 0000000..b76efc5 --- /dev/null +++ b/src/lib/server/zoekt-client.ts @@ -0,0 +1,35 @@ +import { + zoektRequestCount, + zoektRequestDuration, + zoektRequestConcurrency, +} from "./metrics"; +import { configuration } from "./configuration"; + +// This is a small wrapper primarily to handle metrics uniformly. +export const makeZoektRequest = async ( + f: typeof fetch, + path: string, + body: string, +): Promise => { + try { + zoektRequestConcurrency.labels(path).inc(); + + const start = Date.now(); + const response = await f(new URL(path, configuration.zoektUrl), { + method: "POST", + headers: { + "content-type": "application/json", + }, + body, + }); + const durationSeconds = (Date.now() - start) / 1000; + + const labels = [path, response.status.toString()]; + zoektRequestCount.labels(...labels).inc(); + zoektRequestDuration.labels(...labels).inc(durationSeconds); + + return response; + } finally { + zoektRequestConcurrency.labels(path).dec(); + } +}; diff --git a/src/lib/server/zoekt-list-repositories.ts b/src/lib/server/zoekt-list-repositories.ts index 0a35efc..c09e1eb 100644 --- a/src/lib/server/zoekt-list-repositories.ts +++ b/src/lib/server/zoekt-list-repositories.ts @@ -1,6 +1,6 @@ import * as v from "@badrap/valita"; import type { ReadonlyDeep } from "type-fest"; -import { configuration } from "./configuration"; +import { makeZoektRequest } from "./zoekt-client"; export type ListRepositoriesResponse = | { @@ -18,13 +18,7 @@ export async function listRepositories( ): Promise { const body = JSON.stringify({ q: query }); - const response = await f(new URL("/api/list", configuration.zoektUrl), { - method: "POST", - headers: { - "content-type": "application/json", - }, - body, - }); + const response = await makeZoektRequest(f, "/api/list", body); if (!response.ok) { if (response.status === 400) { diff --git a/src/routes/metrics/+server.ts b/src/routes/metrics/+server.ts new file mode 100644 index 0000000..92babdb --- /dev/null +++ b/src/routes/metrics/+server.ts @@ -0,0 +1,17 @@ +import { dev } from "$app/environment"; +import { error, type RequestHandler } from "@sveltejs/kit"; +import { registry } from "$lib/server/metrics"; + +export const GET = (async () => { + if (!dev) { + // This endpoint is only enabled in dev (i.e. SvelteKit vite dev server), as + // prom metrics are only served on the same port as the application in dev. + // In prod, prom metrics are exposed on a different port on an opt-in basis; + // this is the most generally-useful and not-harmful approach, as you don't + // want to expose /metrics to end users in prod. + throw error(404, "Not found"); + } + return new Response(await registry.metrics(), { + headers: { "content-type": registry.contentType }, + }); +}) satisfies RequestHandler;