Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions docs/api-reference/veryfront/observability.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,11 @@ import { captureApplicationError, flushApplicationErrors, initializeSentry } fro
|------|-------------|--------|
| `captureApplicationError` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/application-errors.ts#L223) |
| `flushApplicationErrors` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/application-errors.ts#L241) |
| `initializeSentry` | Initialize the process-wide Sentry reporter once. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L99) |
| `initializeSentryFromEnv` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L83) |
| `isSentryEnabled` | Resolve the compatibility-release Sentry flag. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L45) |
| `resetSentryForTests` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L144) |
| `resolveSentryConfigFromEnv` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L61) |
| `initializeSentry` | Initialize the process-wide Sentry reporter once. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L86) |
| `initializeSentryFromEnv` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L70) |
| `isSentryEnabled` | Return whether Sentry is explicitly enabled. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L39) |
| `resetSentryForTests` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L131) |
| `resolveSentryConfigFromEnv` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/observability/sentry.ts#L49) |

#### Types

Expand Down
8 changes: 4 additions & 4 deletions extensions/ext-observability-sentry/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ VERYFRONT_ERROR_REPORTER=sentry
SENTRY_DSN=https://public@example.ingest.sentry.io/1
```

`SENTRY_ENABLED=false` always disables reporting, even when the adapter and a
valid DSN are present. During the compatibility rollout, an unset flag keeps
the existing adapter-selection behavior. `SENTRY_DSN` selects the event
destination and may use a public HTTPS custom Sentry hostname; `SENTRY_URL` is
Only `SENTRY_ENABLED=true` or `SENTRY_ENABLED=1` enables reporting. An unset,
blank, `false`, `0`, or unrecognized value disables reporting even when the
adapter and a valid DSN are present. `SENTRY_DSN` selects the event destination
and may use a public HTTPS custom Sentry hostname; `SENTRY_URL` is
release-tooling configuration and is not read by the runtime adapter.

`SENTRY_DSN` alone does not activate the framework adapter. Official compiled Veryfront
Expand Down
2 changes: 2 additions & 0 deletions src/agent/hosted/veryfront-cloud-agent-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ Deno.test("startAgentService keeps application-error reporting active after read
.setInitializeApplicationErrorsForTests(async () => {
const lifecycle = await initializeNodeAgentServiceSentryApplicationErrors({
env: {
SENTRY_ENABLED: "true",
SENTRY_DSN: "https://public@example.ingest.sentry.io/1",
},
flushTimeoutMs: 5,
Expand Down Expand Up @@ -468,6 +469,7 @@ Deno.test("startAgentService captures, flushes, and resets terminal startup fail
.setInitializeApplicationErrorsForTests(async () => {
const lifecycle = await initializeNodeAgentServiceSentryApplicationErrors({
env: {
SENTRY_ENABLED: "true",
SENTRY_DSN: "https://public@example.ingest.sentry.io/1",
},
flushTimeoutMs: 5,
Expand Down
32 changes: 19 additions & 13 deletions src/agent/service/node-sentry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe("agent/service/node-sentry", () => {
it("resolves Agent Sentry config from the existing service env", () => {
assertEquals(
resolveNodeAgentServiceSentryConfig({
SENTRY_ENABLED: "true",
SENTRY_DSN: " https://public@example.ingest.sentry.io/1 ",
SENTRY_ENVIRONMENT: "staging",
SENTRY_RELEASE: "release-1",
Expand All @@ -73,7 +74,7 @@ describe("agent/service/node-sentry", () => {
assertStrictEquals(resolveNodeAgentServiceSentryConfig({}), undefined);
});

it("honors explicit Sentry enablement while preserving the legacy DSN behavior when unset", () => {
it("requires explicit Sentry enablement", () => {
const dsn = "https://public@errors.example.test/42";

assertEquals(
Expand All @@ -84,7 +85,7 @@ describe("agent/service/node-sentry", () => {
resolveNodeAgentServiceSentryConfig({ SENTRY_ENABLED: "false", SENTRY_DSN: dsn }),
undefined,
);
assertEquals(resolveNodeAgentServiceSentryConfig({ SENTRY_DSN: dsn })?.dsn, dsn);
assertStrictEquals(resolveNodeAgentServiceSentryConfig({ SENTRY_DSN: dsn }), undefined);
});

it("warns once without exposing secrets when Sentry is explicitly enabled without a DSN", async () => {
Expand Down Expand Up @@ -130,7 +131,7 @@ describe("agent/service/node-sentry", () => {
}
});

it("does not warn or load the SDK when disabled or compatibility-unset without a DSN", async () => {
it("does not warn or load the SDK when disabled or unset without a DSN", async () => {
resetNodeAgentServiceSentryForTests();
const originalWarn = console.warn;
const warnings: unknown[][] = [];
Expand All @@ -148,13 +149,13 @@ describe("agent/service/node-sentry", () => {
env: { SENTRY_ENABLED: "false", SENTRY_DSN: " " },
loadExtension,
});
const compatibilityUnset = await initializeNodeAgentServiceSentryApplicationErrors({
const unset = await initializeNodeAgentServiceSentryApplicationErrors({
env: { SENTRY_DSN: " " },
loadExtension,
});

assertEquals(disabled.enabled, false);
assertEquals(compatibilityUnset.enabled, false);
assertEquals(unset.enabled, false);
assertEquals(warnings, []);
assertEquals(loadCount, 0);
} finally {
Expand Down Expand Up @@ -272,6 +273,7 @@ describe("agent/service/node-sentry", () => {
const otelRecords: string[] = [];
const lifecycle = await initializeNodeAgentServiceSentryApplicationErrors({
env: {
SENTRY_ENABLED: "true",
SENTRY_DSN: "https://public@example.ingest.sentry.io/1",
SENTRY_ENVIRONMENT: "staging",
},
Expand Down Expand Up @@ -310,6 +312,7 @@ describe("agent/service/node-sentry", () => {
const otelRecords: string[] = [];
const lifecycle = await initializeNodeAgentServiceSentryApplicationErrors({
env: {
SENTRY_ENABLED: "true",
SENTRY_DSN: "https://public@example.ingest.sentry.io/1",
},
flushTimeoutMs: 5,
Expand Down Expand Up @@ -351,14 +354,14 @@ describe("agent/service/node-sentry", () => {
const secondReporter = createReporter();

const firstLifecycle = await initializeNodeAgentServiceSentryApplicationErrors({
env: { SENTRY_DSN: "https://public@example.ingest.sentry.io/1" },
env: { SENTRY_ENABLED: "true", SENTRY_DSN: "https://public@example.ingest.sentry.io/1" },
loadExtension: () =>
Promise.resolve({
createNodeSentryApplicationErrorReporter: () => firstReporter,
}),
});
const secondLifecycle = await initializeNodeAgentServiceSentryApplicationErrors({
env: { SENTRY_DSN: "https://public@example.ingest.sentry.io/2" },
env: { SENTRY_ENABLED: "true", SENTRY_DSN: "https://public@example.ingest.sentry.io/2" },
loadExtension: () =>
Promise.resolve({
createNodeSentryApplicationErrorReporter: () => secondReporter,
Expand Down Expand Up @@ -396,7 +399,7 @@ describe("agent/service/node-sentry", () => {
const firstReporter = createReporter();
const secondReporter = createReporter();
const firstLifecycle = await initializeNodeAgentServiceSentryApplicationErrors({
env: { SENTRY_DSN: "https://public@example.ingest.sentry.io/1" },
env: { SENTRY_ENABLED: "true", SENTRY_DSN: "https://public@example.ingest.sentry.io/1" },
loadExtension: () =>
Promise.resolve({
createNodeSentryApplicationErrorReporter: () => firstReporter,
Expand All @@ -408,7 +411,7 @@ describe("agent/service/node-sentry", () => {
) => void)
| undefined;
const replacement = initializeNodeAgentServiceSentryApplicationErrors({
env: { SENTRY_DSN: "https://public@example.ingest.sentry.io/2" },
env: { SENTRY_ENABLED: "true", SENTRY_DSN: "https://public@example.ingest.sentry.io/2" },
loadExtension: () =>
new Promise((resolve) => {
resolveReplacement = resolve;
Expand Down Expand Up @@ -451,7 +454,7 @@ describe("agent/service/node-sentry", () => {
it("preserves the current reporter when replacement loading or construction fails", async () => {
const reporter = createReporter();
const lifecycle = await initializeNodeAgentServiceSentryApplicationErrors({
env: { SENTRY_DSN: "https://public@example.ingest.sentry.io/1" },
env: { SENTRY_ENABLED: "true", SENTRY_DSN: "https://public@example.ingest.sentry.io/1" },
loadExtension: () =>
Promise.resolve({
createNodeSentryApplicationErrorReporter: () => reporter,
Expand All @@ -472,7 +475,10 @@ describe("agent/service/node-sentry", () => {
) {
try {
await initializeNodeAgentServiceSentryApplicationErrors({
env: { SENTRY_DSN: "https://public@example.ingest.sentry.io/2" },
env: {
SENTRY_ENABLED: "true",
SENTRY_DSN: "https://public@example.ingest.sentry.io/2",
},
loadExtension,
});
throw new Error("Expected replacement initialization to fail");
Expand Down Expand Up @@ -509,14 +515,14 @@ describe("agent/service/node-sentry", () => {
) => void)
| undefined;
const firstInit = initializeNodeAgentServiceSentryApplicationErrors({
env: { SENTRY_DSN: "https://public@example.ingest.sentry.io/1" },
env: { SENTRY_ENABLED: "true", SENTRY_DSN: "https://public@example.ingest.sentry.io/1" },
loadExtension: () =>
new Promise((resolve) => {
resolveFirst = resolve;
}),
});
const secondLifecycle = await initializeNodeAgentServiceSentryApplicationErrors({
env: { SENTRY_DSN: "https://public@example.ingest.sentry.io/2" },
env: { SENTRY_ENABLED: "true", SENTRY_DSN: "https://public@example.ingest.sentry.io/2" },
loadExtension: () =>
Promise.resolve({
createNodeSentryApplicationErrorReporter: () => secondReporter,
Expand Down
4 changes: 2 additions & 2 deletions src/agent/service/node-sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export function resolveNodeAgentServiceSentryConfig(
defaultServiceName = DEFAULT_SERVICE_NAME,
): NodeAgentServiceSentryConfig | undefined {
const dsn = readTrimmedEnv(env, "SENTRY_DSN");
if (!isSentryEnabled(env.SENTRY_ENABLED, true)) return undefined;
if (!isSentryEnabled(env.SENTRY_ENABLED)) return undefined;
if (!dsn) return undefined;

const serviceName = readTrimmedEnv(env, "SENTRY_SERVICE_NAME") ??
Expand Down Expand Up @@ -247,7 +247,7 @@ export async function initializeNodeAgentServiceSentryApplicationErrors(options:
if (!config) {
deactivateCurrentLifecycle();
if (
isSentryEnabled(options.env.SENTRY_ENABLED, false) &&
isSentryEnabled(options.env.SENTRY_ENABLED) &&
!readTrimmedEnv(options.env, "SENTRY_DSN")
) {
warnAboutMissingDsnOnce();
Expand Down
20 changes: 12 additions & 8 deletions src/observability/sentry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,13 @@ describe("observability/sentry", () => {
assertEquals(state.config, undefined);
});

it("Sentry enablement preserves legacy behavior while explicit false always wins", () => {
assertEquals(isSentryEnabled("true", false), true);
assertEquals(isSentryEnabled("false", true), false);
assertEquals(isSentryEnabled("0", true), false);
assertEquals(isSentryEnabled(undefined, true), true);
assertEquals(isSentryEnabled(undefined, false), false);
it("Sentry enablement requires an explicit true value", () => {
assertEquals(isSentryEnabled("true"), true);
assertEquals(isSentryEnabled(" 1 "), true);
assertEquals(isSentryEnabled("false"), false);
assertEquals(isSentryEnabled("0"), false);
assertEquals(isSentryEnabled("enabled"), false);
assertEquals(isSentryEnabled(undefined), false);
});

it("Sentry startup warns once without exposing secrets when explicitly enabled without a DSN", async () => {
Expand Down Expand Up @@ -103,7 +104,7 @@ describe("observability/sentry", () => {
}
});

it("Sentry startup does not warn or load the SDK when disabled or compatibility-unset without a DSN", async () => {
it("Sentry startup does not warn or load the SDK when disabled or unset without a DSN", async () => {
resetSentryForTests();
const previousEnabled = Deno.env.get("SENTRY_ENABLED");
const previousProvider = Deno.env.get("VERYFRONT_ERROR_REPORTER");
Expand Down Expand Up @@ -153,13 +154,16 @@ describe("observability/sentry", () => {
});

it("Sentry environment configuration requires explicit provider opt-in", () => {
const previousEnabled = Deno.env.get("SENTRY_ENABLED");
const previousProvider = Deno.env.get("VERYFRONT_ERROR_REPORTER");
const previousDsn = Deno.env.get("SENTRY_DSN");
try {
Deno.env.set("SENTRY_ENABLED", "true");
Deno.env.delete("VERYFRONT_ERROR_REPORTER");
Deno.env.set("SENTRY_DSN", "https://public@example.ingest.sentry.io/1");
assertEquals(resolveSentryConfigFromEnv(), undefined);
} finally {
restoreEnv("SENTRY_ENABLED", previousEnabled);
restoreEnv("VERYFRONT_ERROR_REPORTER", previousProvider);
restoreEnv("SENTRY_DSN", previousDsn);
}
Expand All @@ -172,7 +176,7 @@ describe("observability/sentry", () => {
const previousServiceName = Deno.env.get("SENTRY_SERVICE_NAME");
const previousOtelServiceName = Deno.env.get("OTEL_SERVICE_NAME");
try {
Deno.env.delete("SENTRY_ENABLED");
Deno.env.set("SENTRY_ENABLED", "true");
Deno.env.set("VERYFRONT_ERROR_REPORTER", "sentry");
Deno.env.set("SENTRY_DSN", "https://public@example.ingest.sentry.io/1");
Deno.env.set("SENTRY_SERVICE_NAME", " ");
Expand Down
23 changes: 5 additions & 18 deletions src/observability/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,14 @@ let initializingConfig: Required<SentryConfig> | undefined;
let installedConfig: Required<SentryConfig> | undefined;
let missingDsnWarningEmitted = false;

/**
* Resolve the compatibility-release Sentry flag.
*
* Explicit false always disables reporting. While the flag is unset (or is an
* unrecognized value), callers retain their pre-flag behavior until managed
* deployments have been migrated to explicit values.
*/
export function isSentryEnabled(
enabled: string | undefined,
legacyEnabled: boolean,
): boolean {
/** Return whether Sentry is explicitly enabled. */
export function isSentryEnabled(enabled: string | undefined): boolean {
switch (enabled?.trim().toLowerCase()) {
case "true":
case "1":
return true;
case "false":
case "0":
return false;
default:
return legacyEnabled;
return false;
}
}

Expand All @@ -63,8 +51,7 @@ export function resolveSentryConfigFromEnv(
): SentryConfig | undefined {
const reporterSelected = getEnv("VERYFRONT_ERROR_REPORTER")?.trim().toLowerCase() ===
SENTRY_ERROR_REPORTER;
const enabled = getEnv("SENTRY_ENABLED");
if (!reporterSelected || !isSentryEnabled(enabled, reporterSelected)) {
if (!reporterSelected || !isSentryEnabled(getEnv("SENTRY_ENABLED"))) {
return undefined;
}

Expand Down Expand Up @@ -162,7 +149,7 @@ function sentryConfigsEqual(
function shouldWarnAboutMissingDsn(): boolean {
const reporterSelected = getEnv("VERYFRONT_ERROR_REPORTER")?.trim().toLowerCase() ===
SENTRY_ERROR_REPORTER;
return reporterSelected && isSentryEnabled(getEnv("SENTRY_ENABLED"), false) &&
return reporterSelected && isSentryEnabled(getEnv("SENTRY_ENABLED")) &&
!getEnv("SENTRY_DSN")?.trim();
}

Expand Down
Loading