From f3d52628f632aff98dd5912531ad95cfe39d6a9e Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Mon, 1 Jun 2026 23:07:31 -0700 Subject: [PATCH] feat(observability): structured logger + Effect.fn name rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a structured, service-annotated logger (`@executor-js/sdk/observability`) and merge it into the local + cli runtimes via `Layer.provideMerge`. Client apps (cli/local/desktop) do not phone home: this is logging only, no OTLP exporter — `Effect.fn` span names stay in-process. Also: - add an `effect-source-of-truth` skill (grep `.reference/effect-smol`; don't write Effect from memory — this repo is effect 4 / effect-smol). - add the `require-effect-fn-name` lint rule (`Effect.fn` must carry a string span name; use `Effect.fnUntraced` for un-named helpers), enforced as error. - name the account API handlers `Effect.fn("account.")` as the template for the wider tracing sweep. Verified: typecheck (all 38 packages), lint (0/0), format, sdk/api/local tests. --- .oxlintrc.jsonc | 1 + .skills/effect-source-of-truth/SKILL.md | 31 +++++++ apps/cli/src/integrations.ts | 2 + apps/local/src/executor.ts | 3 +- apps/local/src/integrations.ts | 2 + apps/local/src/main.ts | 5 +- packages/core/api/src/account/handlers.ts | 81 +++++++++++------ packages/core/sdk/package.json | 3 +- packages/core/sdk/src/observability.ts | 86 +++++++++++++++++++ .../sdk/src/oxlint-plugin-executor.test.ts | 32 +++++++ scripts/oxlint-plugin-executor.js | 2 + .../rules/require-effect-fn-name.js | 32 +++++++ 12 files changed, 249 insertions(+), 31 deletions(-) create mode 100644 .skills/effect-source-of-truth/SKILL.md create mode 100644 packages/core/sdk/src/observability.ts create mode 100644 scripts/oxlint-plugin-executor/rules/require-effect-fn-name.js diff --git a/.oxlintrc.jsonc b/.oxlintrc.jsonc index f8897c394..694ae3cf2 100644 --- a/.oxlintrc.jsonc +++ b/.oxlintrc.jsonc @@ -9,6 +9,7 @@ "executor/no-cross-package-relative-imports": "error", "executor/no-direct-cloud-executor-schema-import": "error", "executor/require-reactivity-keys": "error", + "executor/require-effect-fn-name": "error", "executor/no-effect-escape-hatch": "error", "executor/no-effect-internal-tags": "error", "executor/no-error-constructor": "error", diff --git a/.skills/effect-source-of-truth/SKILL.md b/.skills/effect-source-of-truth/SKILL.md new file mode 100644 index 000000000..b5761982e --- /dev/null +++ b/.skills/effect-source-of-truth/SKILL.md @@ -0,0 +1,31 @@ +--- +name: effect-source-of-truth +description: Ground-truth reference for writing Effect in this repo. Use whenever writing, reviewing, or migrating Effect or Schema code — services, layers, schemas, tagged errors, runtimes, HTTP. This repo runs Effect 4 / effect-smol (beta), where most idioms differ from Effect 2/3, so verify against the vendored source instead of memory. +--- + +# Effect: source of truth + +This repo runs **Effect 4 / effect-smol** (`4.0.0-beta.*`), not Effect 3. Most APIs differ from older Effect examples, blog posts, and model memory. Guessing from memory is the most common way to introduce subtly-wrong Effect code here. + +## Do not answer from memory + +1. The current Effect source is vendored at **`.reference/effect-smol`**. Search it (rg/Read) for exact APIs, signatures, examples, and tests before writing or reviewing Effect code. +2. Read nearby in-repo code for local house style before introducing a new pattern. +3. Prefer answers backed by a specific source file or a nearby in-repo example over recollection. + +Concrete v4 differences already hit in this repo: + +- `class Service extends Context.Service()("@scope/Name") {}` — not `Context.Tag` / `Effect.Service`. A service tag is **directly yieldable** (`yield* Service`); there is no `.asEffect()`. +- Submodule imports: `effect/unstable/http`, `effect/unstable/httpapi`, `effect/unstable/observability`. +- `effect`'s structural cause: inspect `cause.reasons` (no `Cause.failures` / `Cause.defects`). +- `References.CurrentLogAnnotations` is a `Record` (iterate with `Object.entries`), not a `Map`. + +## Conventions are enforced — read the rule's `Skill:` pointer + +The custom oxlint plugin (`scripts/oxlint-plugin-executor/`) enforces the house style as CI errors (`--deny-warnings`). When a lint error ends with `Skill: `, read that skill for the _why_ and the fix. Highlights: + +- Trace public service methods with `Effect.fn("Domain.method")`; `Effect.fnUntraced` for internal helpers; `Effect.gen` for inline composition. +- Model errors with `Schema.TaggedErrorClass` and raise them as `yield* new MyError({...})` — never `Effect.fail(new MyError())`, never `Data.TaggedError` in public/wire types. +- No `Schema.Class`, no `switch`, no `try`/`catch`/`throw`, no raw `fetch`, no `JSON.parse`, no `Effect.die`/`orDie` for expected failures in domain code. Prefer Effect platform services (`FileSystem`, `HttpClient`). + +Before introducing any Effect API or pattern not already present in the repo, confirm it exists in `.reference/effect-smol` and fits these rules. diff --git a/apps/cli/src/integrations.ts b/apps/cli/src/integrations.ts index 8ff27c02c..deeeca717 100644 --- a/apps/cli/src/integrations.ts +++ b/apps/cli/src/integrations.ts @@ -1,4 +1,5 @@ import { Effect, Layer, ManagedRuntime } from "effect"; +import { Observability } from "@executor-js/sdk/observability"; import { FetchHttpClient } from "effect/unstable/http"; import { BunFileSystem } from "@effect/platform-bun"; @@ -26,6 +27,7 @@ export const fetchIntegrations = (): void => { integrationsRegistryLayer({ userAgent: USER_AGENT, recurring: false }).pipe( Layer.provide(FetchHttpClient.layer), Layer.provide(BunFileSystem.layer), + Layer.provideMerge(Observability.layer), ), ); runtime.runFork( diff --git a/apps/local/src/executor.ts b/apps/local/src/executor.ts index 007de3677..d05c00763 100644 --- a/apps/local/src/executor.ts +++ b/apps/local/src/executor.ts @@ -1,4 +1,5 @@ import { Context, Data, Effect, Layer, ManagedRuntime, Schema } from "effect"; +import { Observability } from "@executor-js/sdk/observability"; import { type Client } from "@libsql/client"; import { drizzle } from "drizzle-orm/libsql"; import { migrate } from "drizzle-orm/libsql/migrator"; @@ -749,7 +750,7 @@ const createLocalExecutorLayer = () => { export const createExecutorHandle = async () => { const layer = createLocalExecutorLayer(); - const runtime = ManagedRuntime.make(layer); + const runtime = ManagedRuntime.make(layer.pipe(Layer.provideMerge(Observability.layer))); const bundle = await runtime.runPromise(LocalExecutorTag); return { diff --git a/apps/local/src/integrations.ts b/apps/local/src/integrations.ts index ce2c5b9ca..a060485d4 100644 --- a/apps/local/src/integrations.ts +++ b/apps/local/src/integrations.ts @@ -1,4 +1,5 @@ import { Layer, ManagedRuntime } from "effect"; +import { Observability } from "@executor-js/sdk/observability"; import { FetchHttpClient } from "effect/unstable/http"; import { NodeFileSystem } from "@effect/platform-node"; @@ -18,6 +19,7 @@ const integrationsRuntime = ManagedRuntime.make( integrationsRegistryLayer({ userAgent: USER_AGENT }).pipe( Layer.provide(FetchHttpClient.layer), Layer.provide(NodeFileSystem.layer), + Layer.provideMerge(Observability.layer), ), ); diff --git a/apps/local/src/main.ts b/apps/local/src/main.ts index b8afc4b3d..c1885ea38 100644 --- a/apps/local/src/main.ts +++ b/apps/local/src/main.ts @@ -1,4 +1,5 @@ import { Context, Effect, Layer, ManagedRuntime } from "effect"; +import { Observability } from "@executor-js/sdk/observability"; import { createExecutionEngine } from "@executor-js/execution"; import { makeQuickJsExecutor } from "@executor-js/runtime-quickjs"; @@ -82,7 +83,9 @@ const ServerHandlersLive = Layer.effect(ServerHandlersService)( ), ); -const serverHandlersRuntime = ManagedRuntime.make(ServerHandlersLive); +const serverHandlersRuntime = ManagedRuntime.make( + ServerHandlersLive.pipe(Layer.provideMerge(Observability.layer)), +); export const getServerHandlers = (): Promise => serverHandlersRuntime.runPromise(ServerHandlersService); diff --git a/packages/core/api/src/account/handlers.ts b/packages/core/api/src/account/handlers.ts index 919519da9..45ff00f55 100644 --- a/packages/core/api/src/account/handlers.ts +++ b/packages/core/api/src/account/handlers.ts @@ -2,7 +2,13 @@ import { HttpApiBuilder } from "effect/unstable/httpapi"; import { HttpServerRequest } from "effect/unstable/http"; import { Effect } from "effect"; -import { AccountHttpApi } from "./api"; +import { + AccountHttpApi, + type CreateApiKeyBody, + type InviteMemberBody, + type UpdateMemberRoleBody, + type UpdateOrgNameBody, +} from "./api"; import { AccountProvider, type AccountHeaders } from "./service"; // --------------------------------------------------------------------------- @@ -11,6 +17,10 @@ import { AccountProvider, type AccountHeaders } from "./service"; // both cloud and self-host serve identical routes — only the service impl // differs. The neutral errors thrown by the service map directly to their HTTP // statuses (401/403/500) via the contract annotations. +// +// Each handler is an `Effect.fn("account.")` so it opens a named +// trace span / fiber. The `ctx` parameter is annotated explicitly because the +// endpoint-input type does not flow through `Effect.fn` by inference. // --------------------------------------------------------------------------- const requestHeaders = Effect.map( @@ -20,68 +30,83 @@ const requestHeaders = Effect.map( export const AccountHandlers = HttpApiBuilder.group(AccountHttpApi, "account", (handlers) => handlers - .handle("me", () => - Effect.gen(function* () { + .handle( + "me", + Effect.fn("account.me")(function* () { const headers = yield* requestHeaders; return yield* (yield* AccountProvider).me(headers); }), ) - .handle("listApiKeys", () => - Effect.gen(function* () { + .handle( + "listApiKeys", + Effect.fn("account.listApiKeys")(function* () { const headers = yield* requestHeaders; return yield* (yield* AccountProvider).listApiKeys(headers); }), ) - .handle("createApiKey", ({ payload }) => - Effect.gen(function* () { + .handle( + "createApiKey", + Effect.fn("account.createApiKey")(function* (ctx: { payload: typeof CreateApiKeyBody.Type }) { const headers = yield* requestHeaders; - return yield* (yield* AccountProvider).createApiKey(headers, payload.name); + return yield* (yield* AccountProvider).createApiKey(headers, ctx.payload.name); }), ) - .handle("revokeApiKey", ({ params }) => - Effect.gen(function* () { + .handle( + "revokeApiKey", + Effect.fn("account.revokeApiKey")(function* (ctx: { params: { apiKeyId: string } }) { const headers = yield* requestHeaders; - return yield* (yield* AccountProvider).revokeApiKey(headers, params.apiKeyId); + return yield* (yield* AccountProvider).revokeApiKey(headers, ctx.params.apiKeyId); }), ) - .handle("listMembers", () => - Effect.gen(function* () { + .handle( + "listMembers", + Effect.fn("account.listMembers")(function* () { const headers = yield* requestHeaders; return yield* (yield* AccountProvider).listMembers(headers); }), ) - .handle("listRoles", () => - Effect.gen(function* () { + .handle( + "listRoles", + Effect.fn("account.listRoles")(function* () { const headers = yield* requestHeaders; return yield* (yield* AccountProvider).listRoles(headers); }), ) - .handle("inviteMember", ({ payload }) => - Effect.gen(function* () { + .handle( + "inviteMember", + Effect.fn("account.inviteMember")(function* (ctx: { payload: typeof InviteMemberBody.Type }) { const headers = yield* requestHeaders; - return yield* (yield* AccountProvider).inviteMember(headers, payload); + return yield* (yield* AccountProvider).inviteMember(headers, ctx.payload); }), ) - .handle("removeMember", ({ params }) => - Effect.gen(function* () { + .handle( + "removeMember", + Effect.fn("account.removeMember")(function* (ctx: { params: { membershipId: string } }) { const headers = yield* requestHeaders; - return yield* (yield* AccountProvider).removeMember(headers, params.membershipId); + return yield* (yield* AccountProvider).removeMember(headers, ctx.params.membershipId); }), ) - .handle("updateMemberRole", ({ params, payload }) => - Effect.gen(function* () { + .handle( + "updateMemberRole", + Effect.fn("account.updateMemberRole")(function* (ctx: { + params: { membershipId: string }; + payload: typeof UpdateMemberRoleBody.Type; + }) { const headers = yield* requestHeaders; return yield* (yield* AccountProvider).updateMemberRole( headers, - params.membershipId, - payload.roleSlug, + ctx.params.membershipId, + ctx.payload.roleSlug, ); }), ) - .handle("updateOrgName", ({ payload }) => - Effect.gen(function* () { + .handle( + "updateOrgName", + Effect.fn("account.updateOrgName")(function* (ctx: { + payload: typeof UpdateOrgNameBody.Type; + }) { const headers = yield* requestHeaders; - return yield* (yield* AccountProvider).updateOrgName(headers, payload.name); + return yield* (yield* AccountProvider).updateOrgName(headers, ctx.payload.name); }), ), ); diff --git a/packages/core/sdk/package.json b/packages/core/sdk/package.json index 05f3f94c6..c26e3c722 100644 --- a/packages/core/sdk/package.json +++ b/packages/core/sdk/package.json @@ -23,7 +23,8 @@ "./http-source": "./src/http-source.ts", "./promise": "./src/promise.ts", "./client": "./src/client.ts", - "./testing": "./src/testing.ts" + "./testing": "./src/testing.ts", + "./observability": "./src/observability.ts" }, "publishConfig": { "access": "public", diff --git a/packages/core/sdk/src/observability.ts b/packages/core/sdk/src/observability.ts new file mode 100644 index 000000000..9bbb1fc27 --- /dev/null +++ b/packages/core/sdk/src/observability.ts @@ -0,0 +1,86 @@ +import { Cause, Logger, References } from "effect"; + +// --------------------------------------------------------------------------- +// Structured logging for executor's client runtimes (cli / local / desktop). +// +// These apps do NOT phone home: this installs a structured, service-annotated +// logger only. There is no OTLP exporter and no external endpoint — nothing +// leaves the machine. `Effect.fn("Domain.method")` span names still produce +// named fibers and log/span context here, but spans stay in-process. +// +// Cloud is the only surface that exports telemetry, and it keeps its own +// (Axiom) setup — this module is intentionally not wired there. +// --------------------------------------------------------------------------- + +type Fields = Record; + +const LEVEL: Record = { + Trace: "debug", + Debug: "debug", + Warn: "warn", + Error: "error", + Fatal: "error", +}; + +const level = (label: string): string => LEVEL[label] ?? "info"; + +const render = (value: unknown): string => { + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean" || typeof value === "bigint") { + return String(value); + } + // Non-throwing stringify: handle bigint + circular refs inline so domain + // code never needs a try/catch boundary. + const seen = new WeakSet(); + return ( + JSON.stringify(value, (_key, candidate) => { + if (typeof candidate === "bigint") return candidate.toString(); + if (typeof candidate === "object" && candidate !== null) { + if (seen.has(candidate)) return "[Circular]"; + seen.add(candidate); + } + return candidate; + }) ?? String(value) + ); +}; + +const message = (input: unknown): string => { + if (Array.isArray(input)) return input.map(render).join(" "); + return render(input); +}; + +// A single yieldable logger: structured key=value annotations, log-span +// durations, and a pretty-printed cause on failures. Writes to stderr so CLI +// stdout stays reserved for machine-readable output. +export const logger = Logger.make((options) => { + const fields: Fields = {}; + for (const [key, value] of Object.entries( + options.fiber.getRef(References.CurrentLogAnnotations), + )) { + if (value !== undefined && value !== null) fields[key] = value; + } + const now = options.date.getTime(); + for (const [key, start] of options.fiber.getRef(References.CurrentLogSpans)) { + fields[`${key}.ms`] = now - start; + } + + // `service` is promoted into the line prefix rather than rendered as a field. + const service = typeof fields.service === "string" ? fields.service : undefined; + delete fields.service; + + const prefix = service ? `${level(options.logLevel)} ${service}:` : `${level(options.logLevel)}:`; + const annotations = Object.entries(fields) + .map(([key, value]) => `${key}=${render(value)}`) + .join(" "); + const cause = options.cause.reasons.length > 0 ? `\n${Cause.pretty(options.cause)}` : ""; + + const line = [prefix, message(options.message), annotations] + .filter((part) => part.length > 0) + .join(" "); + process.stderr.write(`${line}${cause}\n`); +}); + +// Replaces the default logger in the runtimes it is merged into. +export const layer = Logger.layer([logger], { mergeWithExisting: false }); + +export * as Observability from "./observability"; diff --git a/packages/core/sdk/src/oxlint-plugin-executor.test.ts b/packages/core/sdk/src/oxlint-plugin-executor.test.ts index 0bb7fdc64..8d79e00e0 100644 --- a/packages/core/sdk/src/oxlint-plugin-executor.test.ts +++ b/packages/core/sdk/src/oxlint-plugin-executor.test.ts @@ -181,4 +181,36 @@ describe("executor oxlint plugin", () => { expect(result.status).toBe(0); expect(result.stdout).toContain("Found 0 warnings and 0 errors."); }); + + it("rejects Effect.fn without a string span name", async () => { + const result = await runOxlintOn( + "unnamed-effect-fn.ts", + ` + import { Effect } from "effect"; + + export const run = Effect.fn(function* () { + return yield* Effect.succeed(1); + }); + `, + ); + + expect(result.status).toBe(1); + expect(result.stdout).toContain("executor(require-effect-fn-name)"); + }); + + it("allows Effect.fn with a string span name", async () => { + const result = await runOxlintOn( + "named-effect-fn.ts", + ` + import { Effect } from "effect"; + + export const run = Effect.fn("Demo.run")(function* () { + return yield* Effect.succeed(1); + }); + `, + ); + + expect(result.status).toBe(0); + expect(result.stdout).toContain("Found 0 warnings and 0 errors."); + }); }); diff --git a/scripts/oxlint-plugin-executor.js b/scripts/oxlint-plugin-executor.js index f390a8365..47d8c1a17 100644 --- a/scripts/oxlint-plugin-executor.js +++ b/scripts/oxlint-plugin-executor.js @@ -31,6 +31,7 @@ import preferEffectPredicate from "./oxlint-plugin-executor/rules/prefer-effect- import preferSchemaInferredTypes from "./oxlint-plugin-executor/rules/prefer-schema-inferred-types.js"; import preferYieldTaggedError from "./oxlint-plugin-executor/rules/prefer-yield-tagged-error.js"; import preferValueInferredExtensionTypes from "./oxlint-plugin-executor/rules/prefer-value-inferred-extension-types.js"; +import requireEffectFnName from "./oxlint-plugin-executor/rules/require-effect-fn-name.js"; import requireReactivityKeys from "./oxlint-plugin-executor/rules/require-reactivity-keys.js"; export default { @@ -44,6 +45,7 @@ export default { "no-cross-package-relative-imports": noCrossPackageRelativeImports, "no-direct-cloud-executor-schema-import": noDirectCloudExecutorSchemaImport, "require-reactivity-keys": requireReactivityKeys, + "require-effect-fn-name": requireEffectFnName, "no-effect-escape-hatch": noEffectEscapeHatch, "no-effect-internal-tags": noEffectInternalTags, "no-error-constructor": noErrorConstructor, diff --git a/scripts/oxlint-plugin-executor/rules/require-effect-fn-name.js b/scripts/oxlint-plugin-executor/rules/require-effect-fn-name.js new file mode 100644 index 000000000..4a34abe6c --- /dev/null +++ b/scripts/oxlint-plugin-executor/rules/require-effect-fn-name.js @@ -0,0 +1,32 @@ +import { getPropertyName, isIdentifier, isStringLiteral } from "../utils.js"; + +const message = + 'Effect.fn must be given a span name string, for example Effect.fn("Domain.method")(function* () { ... }). The name becomes the trace span and fiber name. Use Effect.fnUntraced for internal helpers that should not open a span. Skill: effect-source-of-truth.'; + +// A bare string-literal name (including a no-substitution template) is the +// traced form. Anything else as the first argument — a generator, an arrow, a +// config object, or a missing argument — is an un-named span and is rejected. +const isSpanName = (node) => + isStringLiteral(node) || (node?.type === "TemplateLiteral" && node.expressions.length === 0); + +export default { + meta: { + type: "problem", + docs: { + description: "Require a string span-name as the first argument to Effect.fn.", + }, + }, + create(context) { + return { + CallExpression(node) { + const callee = node.callee; + if (callee?.type !== "MemberExpression") return; + if (!isIdentifier(callee.object, "Effect")) return; + if (getPropertyName(callee.property) !== "fn") return; + if (!isSpanName(node.arguments?.[0])) { + context.report({ node, message }); + } + }, + }; + }, +};