Skip to content

Commit c110e09

Browse files
authored
chore: support boolean for telemetry config and refactor server instrumentations (#41)
* chore: move server instrumentations to telemetry directory * feat: support boolean for telemetry config
1 parent ff79cf0 commit c110e09

File tree

7 files changed

+95
-68
lines changed

7 files changed

+95
-68
lines changed

apps/dev-playground/server/reconnect-plugin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export class ReconnectPlugin extends Plugin {
1111
method: "get",
1212
path: "/",
1313
schema: z.object({ message: z.string() }),
14-
handler: async (req, res) => {
14+
handler: async (_req, res) => {
1515
res.json({ message: "Reconnected" });
1616
},
1717
});

packages/app-kit/src/server/index.ts

Lines changed: 5 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
import fs from "node:fs";
22
import type { Server as HTTPServer } from "node:http";
33
import path from "node:path";
4-
import { ExpressInstrumentation } from "@opentelemetry/instrumentation-express";
5-
import { HttpInstrumentation } from "@opentelemetry/instrumentation-http";
64
import { Plugin, toPlugin } from "../plugin";
75
import type { BasePluginConfig, PluginPhase } from "shared";
86
import { databricksClientMiddleware, isRemoteServerEnabled } from "../utils";
97
import dotenv from "dotenv";
108
import express from "express";
119
import { DevModeManager } from "./dev-mode";
1210
import { getQueries, getRoutes } from "./utils";
11+
import { instrumentations } from "../telemetry";
1312

1413
dotenv.config({ path: path.resolve(process.cwd(), "./server/.env") });
1514

@@ -44,7 +43,10 @@ export class ServerPlugin extends Plugin {
4443
this.serverApplication = express();
4544
this.server = null;
4645
this.serverExtensions = [];
47-
this.telemetry.registerInstrumentations(this.createInstrumentations());
46+
this.telemetry.registerInstrumentations([
47+
instrumentations.http,
48+
instrumentations.express,
49+
]);
4850
}
4951

5052
async setup() {
@@ -268,53 +270,6 @@ export class ServerPlugin extends Plugin {
268270
process.exit(0);
269271
}
270272
}
271-
272-
private createInstrumentations() {
273-
return [
274-
new HttpInstrumentation({
275-
applyCustomAttributesOnSpan(span: any, request: any) {
276-
let spanName: string | null = null;
277-
278-
if (request.route) {
279-
const baseUrl = request.baseUrl || "";
280-
const url = request.url?.split("?")[0] || "";
281-
const fullPath = baseUrl + url;
282-
if (fullPath) {
283-
spanName = `${request.method} ${fullPath}`;
284-
}
285-
} else if (request.url) {
286-
// No Express route (e.g., static assets) - use the raw URL path
287-
// Remove query string for cleaner trace names
288-
const path = request.url.split("?")[0];
289-
spanName = `${request.method} ${path}`;
290-
}
291-
292-
if (spanName) {
293-
span.updateName(spanName);
294-
}
295-
},
296-
}),
297-
new ExpressInstrumentation({
298-
requestHook: (span: any, info: any) => {
299-
const req = info.request;
300-
301-
// Only update span name for route handlers (layerType: request_handler)
302-
// This ensures we're not renaming middleware spans
303-
if (info.layerType === "request_handler" && req.route) {
304-
// Combine baseUrl with url to get full path with actual parameter values
305-
// e.g., baseUrl="/api/analytics" + url="/query/spend_data" = "/api/analytics/query/spend_data"
306-
const baseUrl = req.baseUrl || "";
307-
const url = req.url?.split("?")[0] || "";
308-
const fullPath = baseUrl + url;
309-
if (fullPath) {
310-
const spanName = `${req.method} ${fullPath}`;
311-
span.updateName(spanName);
312-
}
313-
}
314-
},
315-
}),
316-
];
317-
}
318273
}
319274

320275
const EXCLUDED_PLUGINS = [ServerPlugin.name];

packages/app-kit/src/telemetry/config.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,20 @@ export interface TelemetryProviderConfig {
77
}
88

99
export function normalizeTelemetryOptions(
10-
config: TelemetryOptions = {},
10+
config?: TelemetryOptions,
1111
): TelemetryProviderConfig {
12+
if (typeof config === "undefined" || typeof config === "boolean") {
13+
const value = config ?? true;
14+
return {
15+
traces: value,
16+
metrics: value,
17+
logs: value,
18+
};
19+
}
20+
1221
return {
13-
traces: config.traces ?? true,
14-
metrics: config.metrics ?? true,
15-
logs: config.logs ?? true,
22+
traces: config?.traces ?? true,
23+
metrics: config?.metrics ?? true,
24+
logs: config?.logs ?? true,
1625
};
1726
}

packages/app-kit/src/telemetry/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export { context, SpanKind, SpanStatusCode } from "@opentelemetry/api";
1111
export type { LogAttributes, Logger, LogRecord } from "@opentelemetry/api-logs";
1212
export { SeverityNumber } from "@opentelemetry/api-logs";
1313
export { normalizeTelemetryOptions } from "./config";
14+
export { instrumentations } from "./instrumentations";
1415
export { TelemetryManager } from "./telemetry-manager";
1516
export { TelemetryProvider } from "./telemetry-provider";
1617
export type {
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { ExpressInstrumentation } from "@opentelemetry/instrumentation-express";
2+
import { HttpInstrumentation } from "@opentelemetry/instrumentation-http";
3+
import type { Instrumentation } from "@opentelemetry/instrumentation";
4+
5+
/**
6+
* Registry of pre-configured instrumentations for common use cases.
7+
* These can be selectively registered by plugins that need them.
8+
*
9+
* While instrumentations are generally safe to re-register,
10+
* the recommended approach is to register them once in a corresponding plugin constructor.
11+
*/
12+
export const instrumentations: Record<string, Instrumentation> = {
13+
http: new HttpInstrumentation({
14+
applyCustomAttributesOnSpan(span: any, request: any) {
15+
let spanName: string | null = null;
16+
17+
if (request.route) {
18+
const baseUrl = request.baseUrl || "";
19+
const url = request.url?.split("?")[0] || "";
20+
const fullPath = baseUrl + url;
21+
if (fullPath) {
22+
spanName = `${request.method} ${fullPath}`;
23+
}
24+
} else if (request.url) {
25+
// No Express route (e.g., static assets) - use the raw URL path
26+
// Remove query string for cleaner trace names
27+
const path = request.url.split("?")[0];
28+
spanName = `${request.method} ${path}`;
29+
}
30+
31+
if (spanName) {
32+
span.updateName(spanName);
33+
}
34+
},
35+
}),
36+
express: new ExpressInstrumentation({
37+
requestHook: (span: any, info: any) => {
38+
const req = info.request;
39+
40+
// Only update span name for route handlers (layerType: request_handler)
41+
// This ensures we're not renaming middleware spans
42+
if (info.layerType === "request_handler" && req.route) {
43+
// Combine baseUrl with url to get full path with actual parameter values
44+
// e.g., baseUrl="/api/analytics" + url="/query/spend_data" = "/api/analytics/query/spend_data"
45+
const baseUrl = req.baseUrl || "";
46+
const url = req.url?.split("?")[0] || "";
47+
const fullPath = baseUrl + url;
48+
if (fullPath) {
49+
const spanName = `${req.method} ${fullPath}`;
50+
span.updateName(spanName);
51+
}
52+
}
53+
},
54+
}),
55+
};

packages/app-kit/src/telemetry/tests/config.test.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,19 @@ describe("normalizeTelemetryOptions", () => {
1010
expected: { traces: true, metrics: true, logs: true },
1111
},
1212
{
13-
description: "should return config object as-is when provided",
14-
input: { traces: true, metrics: false, logs: true },
15-
expected: { traces: true, metrics: false, logs: true },
13+
description: "should return all enabled when config is true",
14+
input: true,
15+
expected: { traces: true, metrics: true, logs: true },
1616
},
1717
{
18-
description: "should handle all traces/metrics/logs disabled in object",
19-
input: { traces: false, metrics: false, logs: false },
18+
description: "should return all disabled when config is false",
19+
input: false,
2020
expected: { traces: false, metrics: false, logs: false },
2121
},
2222
{
23-
description: "should handle all traces/metrics/logs enabled in object",
24-
input: { traces: true, metrics: true, logs: true },
25-
expected: { traces: true, metrics: true, logs: true },
23+
description: "should return config object as-is when provided",
24+
input: { traces: true, metrics: false, logs: true },
25+
expected: { traces: true, metrics: false, logs: true },
2626
},
2727
{
2828
description:
@@ -35,6 +35,11 @@ describe("normalizeTelemetryOptions", () => {
3535
input: { traces: false, metrics: true, logs: false },
3636
expected: { traces: false, metrics: true, logs: false },
3737
},
38+
{
39+
description: "should handle partial configuration",
40+
input: { traces: false },
41+
expected: { traces: false, metrics: true, logs: true },
42+
},
3843
])("$description", ({ input, expected }) => {
3944
const result = normalizeTelemetryOptions(input);
4045
expect(result).toEqual(expected);

packages/shared/src/plugin.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,13 @@ export interface BasePluginConfig {
2626
telemetry?: TelemetryOptions;
2727
}
2828

29-
export type TelemetryOptions = {
30-
traces?: boolean;
31-
metrics?: boolean;
32-
logs?: boolean;
33-
};
29+
export type TelemetryOptions =
30+
| boolean
31+
| {
32+
traces?: boolean;
33+
metrics?: boolean;
34+
logs?: boolean;
35+
};
3436

3537
export interface PluginConfig {
3638
config?: unknown;

0 commit comments

Comments
 (0)