-
Notifications
You must be signed in to change notification settings - Fork 18
feat(metrics): Add OTEL to api #1110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| import { OTLPMetricExporter } from "@opentelemetry/exporter-metrics-otlp-http"; | ||
| import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http"; | ||
| import { | ||
| ExpressInstrumentation, | ||
| ExpressLayerType, | ||
| } from "@opentelemetry/instrumentation-express"; | ||
| import { HttpInstrumentation } from "@opentelemetry/instrumentation-http"; | ||
| import { PgInstrumentation } from "@opentelemetry/instrumentation-pg"; | ||
| import { RuntimeNodeInstrumentation } from "@opentelemetry/instrumentation-runtime-node"; | ||
| import { resourceFromAttributes } from "@opentelemetry/resources"; | ||
| import { PeriodicExportingMetricReader } from "@opentelemetry/sdk-metrics"; | ||
| import { NodeSDK } from "@opentelemetry/sdk-node"; | ||
| import { | ||
| ParentBasedSampler, | ||
| TraceIdRatioBasedSampler, | ||
| } from "@opentelemetry/sdk-trace-node"; | ||
| import { ATTR_SERVICE_NAME } from "@opentelemetry/semantic-conventions"; | ||
|
|
||
| import { env } from "@/config.js"; | ||
|
|
||
| const sdk = new NodeSDK({ | ||
| resource: resourceFromAttributes({ | ||
| [ATTR_SERVICE_NAME]: env.OTEL_SERVICE_NAME, | ||
| }), | ||
| sampler: new ParentBasedSampler({ | ||
| root: new TraceIdRatioBasedSampler(env.OTEL_SAMPLER_RATIO), | ||
| }), | ||
| traceExporter: new OTLPTraceExporter(), | ||
| metricReader: new PeriodicExportingMetricReader({ | ||
| exporter: new OTLPMetricExporter(), | ||
| exportIntervalMillis: 10_000, | ||
| }), | ||
| instrumentations: [ | ||
| new HttpInstrumentation({ | ||
| ignoreIncomingRequestHook: (req) => req.url === "/api/healthz", | ||
| }), | ||
| new ExpressInstrumentation({ | ||
| ignoreLayersType: [ExpressLayerType.MIDDLEWARE], | ||
| }), | ||
| new PgInstrumentation(), | ||
| new RuntimeNodeInstrumentation(), | ||
| ], | ||
| }); | ||
|
|
||
| try { | ||
| sdk.start(); | ||
| console.log("OpenTelemetry started for service: ", env.OTEL_SERVICE_NAME); | ||
| } catch (err) { | ||
| console.error( | ||
| "OpenTelemetry failed to start, continuing without telemetry", | ||
| err, | ||
| ); | ||
| } | ||
|
|
||
| const shutdown = async () => { | ||
| try { | ||
| await sdk.shutdown(); | ||
| } catch (err) { | ||
| console.error("OpenTelemetry shutdown failed", err); | ||
| } finally { | ||
| process.exit(0); | ||
| } | ||
| }; | ||
|
|
||
| process.on("SIGTERM", () => void shutdown()); | ||
| process.on("SIGINT", () => void shutdown()); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -333,7 +333,7 @@ GITHUB_CLIENT_SECRET=your-client-secret | |
| # Logging | ||
| LOG_LEVEL=info | ||
|
|
||
| # OpenTelemetry (optional) | ||
| # OpenTelemetry (optional) - To fully disable OTEL, set OTEL_SDK_DISABLED=true | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For both workspace-engine and now api, we have localhost as the default so if OTEL is not disabled it'll keep trying to send traces to a non-existent local service. This is the standard env var to disable OTEL but just calling out out here in the docs. |
||
| OTEL_EXPORTER_OTLP_ENDPOINT=http://collector:4318 | ||
| ``` | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import type { Session } from "@ctrlplane/auth/server"; | ||
| import type { PermissionChecker } from "@ctrlplane/auth/utils"; | ||
| import { SpanStatusCode, trace } from "@opentelemetry/api"; | ||
| import { initTRPC, TRPCError } from "@trpc/server"; | ||
| import superjson from "superjson"; | ||
| import { ZodError } from "zod/v4"; | ||
|
|
@@ -43,7 +44,27 @@ const t = initTRPC | |
| }); | ||
|
|
||
| export const router = t.router; | ||
| export const publicProcedure = t.procedure; | ||
|
|
||
| const tracer = trace.getTracer("@ctrlplane/trpc"); | ||
|
|
||
| const tracingMiddleware = t.middleware(({ path, type, next }) => | ||
| tracer.startActiveSpan(`trpc.${type} ${path}`, async (span) => { | ||
| span.setAttribute("trpc.path", path); | ||
| span.setAttribute("trpc.type", type); | ||
| try { | ||
| const result = await next(); | ||
| if (!result.ok) { | ||
| span.setStatus({ code: SpanStatusCode.ERROR }); | ||
| span.setAttribute("trpc.error_code", result.error.code); | ||
| } | ||
| return result; | ||
| } finally { | ||
| span.end(); | ||
| } | ||
| }), | ||
| ); | ||
|
|
||
| export const publicProcedure = t.procedure.use(tracingMiddleware); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one isn't strictly required for tracing to work in the API, but without this we won't see which trpc procedure is called in the traces. You can see in my demo that this will add a sub-span for each trpc proc so it's clear which query this is coming from. |
||
|
|
||
| const authnProcedure = publicProcedure.use(({ ctx, next }) => { | ||
| if (ctx.session == null) throw new TRPCError({ code: "UNAUTHORIZED" }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this imported before express. Alternative to adding it here is to make it the first require in the server file, but I figured this was cleaner. Open to changing it though because I'm not super crazy about it here either.