Parcourir la source

refactor(server): align route-span attrs with OTel semantic conventions (#23198)

Kit Langton il y a 4 jours
Parent
commit
eafbe5c57c

+ 17 - 4
packages/opencode/src/server/routes/instance/trace.ts

@@ -5,9 +5,12 @@ import { AppRuntime } from "@/effect/app-runtime"
 type AppEnv = Parameters<typeof AppRuntime.runPromise>[0] extends Effect.Effect<any, any, infer R> ? R : never
 
 // Build the base span attributes for an HTTP handler: method, path, and every
-// matched route param (sessionID, messageID, partID, providerID, ptyID, …)
-// prefixed with `opencode.`. This makes each request's root span searchable
-// by ID in motel without having to parse the path string.
+// matched route param. Names follow OTel attribute-naming guidance:
+// domain-first (`session.id`, `message.id`, …) so they match the existing
+// OTel `session.id` semantic convention and the bare `message.id` we
+// already emit from Tool.execute. Non-standard route params fall back to
+// `opencode.<name>` since those are internal implementation details
+// (per https://opentelemetry.io/blog/2025/how-to-name-your-span-attributes/).
 export interface RequestLike {
   readonly req: {
     readonly method: string
@@ -16,13 +19,23 @@ export interface RequestLike {
   }
 }
 
+// Normalize a Hono route param key (e.g. `sessionID`, `messageID`, `name`)
+// to an OTel attribute key. `fooID` → `foo.id` for ID-shaped params; any
+// other param is namespaced under `opencode.` to avoid colliding with
+// standard conventions.
+export function paramToAttributeKey(key: string): string {
+  const m = key.match(/^(.+)ID$/)
+  if (m) return `${m[1].toLowerCase()}.id`
+  return `opencode.${key}`
+}
+
 export function requestAttributes(c: RequestLike): Record<string, string> {
   const attributes: Record<string, string> = {
     "http.method": c.req.method,
     "http.path": new URL(c.req.url).pathname,
   }
   for (const [key, value] of Object.entries(c.req.param())) {
-    attributes[`opencode.${key}`] = value
+    attributes[paramToAttributeKey(key)] = value
   }
   return attributes
 }

+ 31 - 7
packages/opencode/test/server/trace-attributes.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test"
-import { requestAttributes } from "../../src/server/routes/instance/trace"
+import { paramToAttributeKey, requestAttributes } from "../../src/server/routes/instance/trace"
 
 function fakeContext(method: string, url: string, params: Record<string, string>) {
   return {
@@ -11,6 +11,25 @@ function fakeContext(method: string, url: string, params: Record<string, string>
   }
 }
 
+describe("paramToAttributeKey", () => {
+  test("converts fooID to foo.id", () => {
+    expect(paramToAttributeKey("sessionID")).toBe("session.id")
+    expect(paramToAttributeKey("messageID")).toBe("message.id")
+    expect(paramToAttributeKey("partID")).toBe("part.id")
+    expect(paramToAttributeKey("projectID")).toBe("project.id")
+    expect(paramToAttributeKey("providerID")).toBe("provider.id")
+    expect(paramToAttributeKey("ptyID")).toBe("pty.id")
+    expect(paramToAttributeKey("permissionID")).toBe("permission.id")
+    expect(paramToAttributeKey("requestID")).toBe("request.id")
+    expect(paramToAttributeKey("workspaceID")).toBe("workspace.id")
+  })
+
+  test("namespaces non-ID params under opencode.", () => {
+    expect(paramToAttributeKey("name")).toBe("opencode.name")
+    expect(paramToAttributeKey("slug")).toBe("opencode.slug")
+  })
+})
+
 describe("requestAttributes", () => {
   test("includes http method and path", () => {
     const attrs = requestAttributes(fakeContext("GET", "http://localhost/session", {}))
@@ -23,7 +42,7 @@ describe("requestAttributes", () => {
     expect(attrs["http.path"]).toBe("/file/search")
   })
 
-  test("tags route params with opencode.<param> prefix", () => {
+  test("emits OTel-style <domain>.id for ID-shaped route params", () => {
     const attrs = requestAttributes(
       fakeContext("GET", "http://localhost/session/ses_abc/message/msg_def/part/prt_ghi", {
         sessionID: "ses_abc",
@@ -31,22 +50,27 @@ describe("requestAttributes", () => {
         partID: "prt_ghi",
       }),
     )
-    expect(attrs["opencode.sessionID"]).toBe("ses_abc")
-    expect(attrs["opencode.messageID"]).toBe("msg_def")
-    expect(attrs["opencode.partID"]).toBe("prt_ghi")
+    expect(attrs["session.id"]).toBe("ses_abc")
+    expect(attrs["message.id"]).toBe("msg_def")
+    expect(attrs["part.id"]).toBe("prt_ghi")
+    // No camelCase leftovers:
+    expect(attrs["opencode.sessionID"]).toBeUndefined()
+    expect(attrs["opencode.messageID"]).toBeUndefined()
+    expect(attrs["opencode.partID"]).toBeUndefined()
   })
 
   test("produces no param attributes when no params are matched", () => {
     const attrs = requestAttributes(fakeContext("POST", "http://localhost/config", {}))
-    expect(Object.keys(attrs).filter((k) => k.startsWith("opencode."))).toEqual([])
+    expect(Object.keys(attrs).filter((k) => k !== "http.method" && k !== "http.path")).toEqual([])
   })
 
-  test("handles non-ID params (e.g. mcp :name) without mangling", () => {
+  test("namespaces non-ID params under opencode. (e.g. mcp :name)", () => {
     const attrs = requestAttributes(
       fakeContext("POST", "http://localhost/mcp/exa/connect", {
         name: "exa",
       }),
     )
     expect(attrs["opencode.name"]).toBe("exa")
+    expect(attrs["name"]).toBeUndefined()
   })
 })