Skip to content

Fix Observability: trace quality, cost reduction, and new metrics #1133

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.

Sign up for GitHub

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

Open
nicacioliveira wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix Observability: trace quality, cost reduction, and new metrics #1133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
73729a6
feat(o11y): improve telemetry quality, reduce cost, add visibility
Mar 20, 2026
ae8a997
feat(o11y): Redis health metrics, startup duration, remove K_REVISION…
Mar 20, 2026
61ed616
fix(trace): FilteringSpanProcessor now works — use provider.getTracer…
Mar 20, 2026
a5060ad
fix(sampler): DebugSampler was forcing 100% sampling on all requests
Mar 20, 2026
3b6c792
fix lrucache counter evictions
Mar 20, 2026
36f17b8
fix(o11y): address code review — metrics accuracy and deno fmt
Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions blocks/loader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ import { HttpError } from "../engine/errors.ts";
import type { ResolverMiddlewareContext } from "../engine/middleware.ts";
import type { State } from "../mod.ts";
import { logger } from "../observability/otel/config.ts";
import {
meter,
OTEL_ENABLE_EXTRA_METRICS,
} from "../observability/otel/metrics.ts";
import { meter } from "../observability/otel/metrics.ts";
import { caches, ENABLE_LOADER_CACHE } from "../runtime/caches/mod.ts";
import { inFuture } from "../runtime/caches/utils.ts";
import type { DebugProperties } from "../utils/vary.ts";
Expand Down Expand Up @@ -184,6 +181,7 @@ const wrapLoader = (
}: LoaderModule,
resolveChain: FieldResolver[],
release: DecofileProvider,
loaderKey?: string,
) => {
const [cacheMaxAge, mode] = typeof cache === "string"
? [MAX_AGE_S, cache]
Expand All @@ -204,7 +202,9 @@ const wrapLoader = (
req: Request,
ctx: FnContext<State, any>,
): Promise<ReturnType<typeof handler>> => {
const loader = ctx.resolverId || "unknown";
const loader = (ctx.resolverId && ctx.resolverId !== "obj")
? ctx.resolverId
: (loaderKey ?? ctx.resolverId ?? "unknown");
const start = performance.now();
let status: "bypass" | "miss" | "stale" | "hit" | undefined;

Expand Down Expand Up @@ -267,8 +267,9 @@ const wrapLoader = (
(await release?.revision() ?? undefined);

if (!revisionID) {
logger.warn(`Could not get K_REVISION`);
timing?.end();
status = "bypass";
stats.cache.add(1, { status, loader });
return await handler(props, req, ctx);
}

Expand Down Expand Up @@ -339,9 +340,7 @@ const wrapLoader = (
return await flights.do(request.url, staleWhileRevalidate);
} finally {
const dimension = { loader, status };
if (OTEL_ENABLE_EXTRA_METRICS) {
stats.latency.record(performance.now() - start, dimension);
}
stats.latency.record(performance.now() - start, dimension);
ctx.monitoring?.currentSpan?.setDesc(status);
Comment on lines 341 to 344
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify all status assignments and latency emission context in blocks/loader.ts
rg -n -C3 'let status|status =|if \(!revisionID\)|stats\.latency\.record|setDesc\(status\)' blocks/loader.ts

Repository: deco-cx/deco

Length of output: 1907

🏁 Script executed:

rg -n -B 30 'let status:' blocks/loader.ts | head -50

Repository: deco-cx/deco

Length of output: 1136

🏁 Script executed:

rg -n -A 50 'let status:' blocks/loader.ts | tail -100

Repository: deco-cx/deco

Length of output: 1968

🏁 Script executed:

sed -n '223,343p' blocks/loader.ts

Repository: deco-cx/deco

Length of output: 4122

Avoid emitting resolver_latency with status: undefined.

The early return at line 271 (when !revisionID) does not assign a value to status before exiting, causing the finally block to unconditionally record latency metrics with status: undefined. Additionally, if any exception occurs before a status assignment, the same issue occurs since there is no catch handler.

Initialize status to a default value (e.g., "unknown") and explicitly set it before early returns and in a catch block to ensure all code paths emit consistent metric attributes.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@blocks/loader.ts` around lines 339 - 342, Initialize the local variable
status to a default like "unknown" and ensure it is explicitly set before any
early return (e.g., the !revisionID branch) and inside a catch handler, so the
finally block's stats.latency.record(performance.now() - start, { loader, status
}) and ctx.monitoring?.currentSpan?.setDesc(status) never receive undefined; add
a try/catch around the main logic to assign a failure status on exceptions and
update the early-return paths to set the appropriate status value before
returning.

}
},
Expand All @@ -356,7 +355,7 @@ const loaderBlock: Block<LoaderModule> = {
wrapCaughtErrors,
(props: TProps, ctx: HttpContext<{ global: any } & RequestState>) =>
applyProps(
wrapLoader(mod, ctx.resolveChain, ctx.context.state.release),
wrapLoader(mod, ctx.resolveChain, ctx.context.state.release, key),
)(
props,
ctx,
Expand Down
5 changes: 4 additions & 1 deletion deps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export type {
export * as weakcache from "npm:weak-lru-cache@1.0.0";
export type Handler = Deno.ServeHandler;

export { OTLPTraceExporter } from "npm:@opentelemetry/exporter-trace-otlp-proto@0.52.1";
export { OTLPTraceExporter } from "npm:@opentelemetry/exporter-trace-otlp-http@0.52.1";
export { Resource } from "npm:@opentelemetry/resources@1.25.1";
export {
BatchSpanProcessor,
Expand All @@ -69,8 +69,11 @@ export {
} from "npm:@opentelemetry/sdk-trace-base@1.25.1";

export type {
ReadableSpan,
Sampler,
SamplingResult,
SpanExporter,
SpanProcessor,
} from "npm:@opentelemetry/sdk-trace-base@1.25.1";
export { NodeTracerProvider } from "npm:@opentelemetry/sdk-trace-node@1.25.1";
export {
Expand Down
11 changes: 10 additions & 1 deletion engine/manifest/manifest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// deno-lint-ignore-file no-explicit-any
import { parse } from "@std/flags";
import { ValueType } from "../../deps.ts";
import { meter } from "../../observability/otel/metrics.ts";
import { gray, green, red } from "@std/fmt/colors";
import {
type AppManifest,
Expand Down Expand Up @@ -284,8 +286,15 @@ export const fulfillContext = async <
ctx.site = currentSite!;
const provider = release ?? await getProvider();
const runtimePromise = deferred<DecoRuntimeState<T>>();
const startedAt = ctx.instance.startedAt;
ctx.runtime = runtimePromise.finally(() => {
ctx.instance.readyAt = new Date();
const readyAt = new Date();
ctx.instance.readyAt = readyAt;
meter.createHistogram("instance_startup_duration_ms", {
description: "Time from instance start to first request readiness.",
unit: "ms",
valueType: ValueType.DOUBLE,
}).record(readyAt.getTime() - startedAt.getTime());
});

ctx.release = provider;
Expand Down
90 changes: 39 additions & 51 deletions observability/otel/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,68 +1,36 @@
import * as log from "@std/log";
import { Logger } from "@std/log/logger";
import { Context, context } from "../../deco.ts";
import denoJSON from "../../deno.json" with { type: "json" };
import {
BatchSpanProcessor,
diag,
DiagConsoleLogger,
DiagLogLevel,
FetchInstrumentation,
NodeTracerProvider,
opentelemetry,
OTLPTraceExporter,
ParentBasedSampler,
registerInstrumentations,
Resource,
SemanticResourceAttributes,
} from "../../deps.ts";
import { DenoRuntimeInstrumentation } from "./instrumentation/deno-runtime.ts";

if (Deno.env.has("OTEL_DIAG")) {
diag.setLogger(new DiagConsoleLogger(), DiagLogLevel.DEBUG);
}
import "./instrumentation/deno-runtime.ts";
import { DebugSampler } from "./samplers/debug.ts";
import { type SamplingOptions, URLBasedSampler } from "./samplers/urlBased.ts";

import { ENV_SITE_NAME } from "../../engine/decofile/constants.ts";
import { safeImportResolve } from "../../engine/importmap/builder.ts";
import { FilteringSpanProcessor } from "./processors/filtering.ts";
import { OTEL_IS_ENABLED, resource } from "./resource.ts";
import { OpenTelemetryHandler } from "./logger.ts";

const tryGetVersionOf = (pkg: string) => {
try {
const [_, ver] = safeImportResolve(pkg).split("@");
return ver.substring(0, ver.length - 1);
} catch {
return undefined;
}
};
const apps_ver = tryGetVersionOf("apps/") ??
tryGetVersionOf("deco-sites/std/") ?? "_";

export const resource = Resource.default().merge(
new Resource({
[SemanticResourceAttributes.SERVICE_NAME]: Deno.env.get(ENV_SITE_NAME) ??
"deco",
[SemanticResourceAttributes.SERVICE_VERSION]:
Context.active().deploymentId ??
Deno.hostname(),
[SemanticResourceAttributes.SERVICE_INSTANCE_ID]: crypto.randomUUID(),
[SemanticResourceAttributes.CLOUD_PROVIDER]: context.platform,
"deco.runtime.version": denoJSON.version,
"deco.apps.version": apps_ver,
[SemanticResourceAttributes.CLOUD_REGION]: Deno.env.get("DENO_REGION") ??
"unknown",
[SemanticResourceAttributes.DEPLOYMENT_ENVIRONMENT]: Deno.env.get(
"DECO_ENV_NAME",
)
? `env-${Deno.env.get("DECO_ENV_NAME")}`
: "production",
}),
);

const loggerName = "deco-logger";
export const OTEL_IS_ENABLED: boolean = Deno.env.has(
"OTEL_EXPORTER_OTLP_ENDPOINT",
);
export const logger: Logger = new Logger(loggerName, "INFO", {
handlers: [
...OTEL_IS_ENABLED
? [
new OpenTelemetryHandler("INFO", {
resourceAttributes: resource.attributes,
detectResources: false,
}),
]
: [new log.ConsoleHandler("INFO")],
Expand All @@ -82,6 +50,7 @@ registerInstrumentations({
// @ts-ignore: no idea why this is failing, but it should work
new FetchInstrumentation(
{
ignoreUrls: [/127\.0\.0\.1/, /localhost/],
applyCustomAttributesOnSpan: (
span,
_req,
Expand All @@ -101,7 +70,6 @@ registerInstrumentations({
},
},
),
new DenoRuntimeInstrumentation(),
],
});

Expand All @@ -125,8 +93,9 @@ const parseSamplingOptions = (): SamplingOptions | undefined => {
}
};

const samplingOptions = parseSamplingOptions();
const debugSampler = new DebugSampler(
new URLBasedSampler(parseSamplingOptions()),
new URLBasedSampler(samplingOptions),
);
const provider = new NodeTracerProvider({
resource: resource,
Expand All @@ -137,17 +106,36 @@ const provider = new NodeTracerProvider({
),
});

if (OTEL_IS_ENABLED) {
// Deno 2.2+ has built-in OTel support via OTEL_DENO=true.
// If enabled, it sets its own global TracerProvider and instruments fetch/console,
// which would conflict with our manual setup (double spans, double logs).
// Skip provider.register() when Deno's native OTel is active.
const DENO_OTEL_ACTIVE = Deno.env.get("OTEL_DENO") === "true";


if (OTEL_IS_ENABLED && !DENO_OTEL_ACTIVE) {
const traceExporter = new OTLPTraceExporter();
// @ts-ignore: no idea why this is failing, but it should work
provider.addSpanProcessor(new BatchSpanProcessor(traceExporter));
provider.addSpanProcessor(
new FilteringSpanProcessor(
// @ts-ignore: no idea why this is failing, but it should work
new BatchSpanProcessor(traceExporter),
samplingOptions,
),
);

provider.register();

addEventListener("unload", () => {
provider.shutdown().catch(() => {});
});
}
Comment on lines +109 to 131
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent DENO_OTEL_ACTIVE guard between config.ts and metrics.ts.

The tracer provider registration correctly skips when DENO_OTEL_ACTIVE is true, but metrics.ts (lines 45-60 per context snippet) registers its own unload listener unconditionally when OTEL_IS_ENABLED without checking DENO_OTEL_ACTIVE. This could cause conflicts when Deno's native OTel is active.

Consider applying the same guard in metrics.ts for consistency:

// In metrics.ts
if (OTEL_IS_ENABLED && !DENO_OTEL_ACTIVE) {
  // ... metric exporter setup and unload listener
}
#!/bin/bash
# Check if metrics.ts has DENO_OTEL_ACTIVE guard
rg -n "DENO_OTEL_ACTIVE|OTEL_DENO" observability/otel/metrics.ts
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@observability/otel/config.ts` around lines 109 - 131, Wrap the metrics
initialization and its unload listener in the same DENO_OTEL_ACTIVE guard used
for tracing: change the block that currently checks only OTEL_IS_ENABLED to
require OTEL_IS_ENABLED && !DENO_OTEL_ACTIVE, and ensure DENO_OTEL_ACTIVE is
available in that module (either by computing Deno.env.get("OTEL_DENO") ===
"true" or importing the constant from the tracing config). Specifically update
the metrics initialization block that registers the metric exporter / meter
provider and the addEventListener("unload", ...) so it runs only when
OTEL_IS_ENABLED && !DENO_OTEL_ACTIVE.


export const tracer = opentelemetry.trace.getTracer(
"deco-tracer",
);
// Use provider.getTracer directly (not via global API) to ensure spans
// always go through our FilteringSpanProcessor, even if another TracerProvider
// overrides the global after our provider.register() call.
export const tracer = OTEL_IS_ENABLED && !DENO_OTEL_ACTIVE
? provider.getTracer("deco-tracer")
: opentelemetry.trace.getTracer("deco-tracer");

export const tracerIsRecording = () =>
opentelemetry.trace.getActiveSpan()?.isRecording() ?? false;
103 changes: 44 additions & 59 deletions observability/otel/instrumentation/deno-runtime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,69 +1,54 @@
/**
* Heavily inspired from unlicensed code: https://github.com/cloudydeno/deno-observability/blob/main/instrumentation/deno-runtime.ts
*/
import {
type Attributes,
InstrumentationBase,
type InstrumentationConfig,
type ObservableCounter,
type ObservableGauge,
type ObservableResult,
type ObservableUpDownCounter,
ValueType,
} from "../../../deps.ts";

export class DenoRuntimeInstrumentation extends InstrumentationBase {
readonly component: string = "deno-runtime";
moduleName = this.component;

constructor(_config?: InstrumentationConfig) {
super("deno-runtime", "0.1.0", { enabled: false });
import { meter } from "../metrics.ts";

const memoryUsage: ObservableGauge<Attributes> = meter
.createObservableGauge("deno.memory_usage", {
unit: "By",
valueType: ValueType.DOUBLE,
description: "Deno process memory usage in bytes.",
});

const openResources: ObservableUpDownCounter<Attributes> = meter
.createObservableUpDownCounter("deno.open_resources", {
valueType: ValueType.DOUBLE,
description: "Number of open resources of a particular type.",
});

const gatherMemoryUsage = (x: ObservableResult<Attributes>) => {
const usage = Deno.memoryUsage();
x.observe(usage.rss, { "deno.memory.type": "rss" });
x.observe(usage.heapTotal, { "deno.memory.type": "heap_total" });
x.observe(usage.heapUsed, { "deno.memory.type": "heap_used" });
x.observe(usage.external, { "deno.memory.type": "external" });
};

const gatherOpenResources = (x: ObservableResult<Attributes>) => {
try {
// deno-lint-ignore no-explicit-any
const resources = (Deno as any).resources?.() as
| Record<string, string>
| undefined;
if (!resources) return;
const counts: Record<string, number> = {};
for (const type of Object.values(resources)) {
counts[type] = (counts[type] ?? 0) + 1;
}
for (const [type, count] of Object.entries(counts)) {
x.observe(count, { "deno.resource.type": type });
}
} catch {
// Deno.resources() may not be available in all environments
}
};

metrics!: {
openResources: ObservableUpDownCounter<Attributes>;
memoryUsage: ObservableGauge<Attributes>;
dispatchedCtr: ObservableCounter<Attributes>;
inflightCtr: ObservableUpDownCounter<Attributes>;
};

protected init() {}

private gatherMemoryUsage = (x: ObservableResult<Attributes>) => {
const usage = Deno.memoryUsage();
x.observe(usage.rss, { "deno.memory.type": "rss" });
x.observe(usage.heapTotal, { "deno.memory.type": "heap_total" });
x.observe(usage.heapUsed, { "deno.memory.type": "heap_used" });
x.observe(usage.external, { "deno.memory.type": "external" });
};
memoryUsage.addCallback(gatherMemoryUsage);
openResources.addCallback(gatherOpenResources);

override enable() {
this.metrics ??= {
openResources: this.meter
.createObservableUpDownCounter("deno.open_resources", {
valueType: ValueType.DOUBLE,
description: "Number of open resources of a particular type.",
}),
memoryUsage: this.meter
.createObservableGauge("deno.memory_usage", {
valueType: ValueType.DOUBLE,
}),
dispatchedCtr: this.meter
.createObservableCounter("deno.ops_dispatched", {
valueType: ValueType.DOUBLE,
description: "Total number of Deno op invocations.",
}),
inflightCtr: this.meter
.createObservableUpDownCounter("deno.ops_inflight", {
valueType: ValueType.DOUBLE,
description: "Number of currently-inflight Deno ops.",
}),
};

this.metrics.memoryUsage.addCallback(this.gatherMemoryUsage);
}

override disable() {
this.metrics.memoryUsage.removeCallback(this.gatherMemoryUsage);
}
}
// Kept for backward compatibility — no longer needed but exported to avoid import errors
export class DenoRuntimeInstrumentation {}
Loading
Loading