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
14 changes: 14 additions & 0 deletions apps/desktop/src/app/DesktopOtlpExport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,20 @@ describe("resolveDesktopOtlpExport", () => {
}),
);

it.effect("keeps exporting when T3 Code's own name says to", () =>
Effect.gen(function* () {
const resolved = yield* resolve(
{
T3CODE_OTEL_SDK_DISABLED: "false",
OTEL_SDK_DISABLED: "true",
},
{ named: { traces: "http://127.0.0.1:4318/v1/traces" } },
);
assert.strictEqual(resolved.traces.url, "http://127.0.0.1:4318/v1/traces");
assert.deepStrictEqual(resolved.warnings, []);
}),
);

it.effect("declines only the signal that asked for a protocol T3 Code cannot speak", () =>
Effect.gen(function* () {
const resolved = yield* resolve({
Expand Down
5 changes: 1 addition & 4 deletions apps/desktop/src/app/DesktopOtlpExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,7 @@ export const resolveDesktopOtlpExport = (input: DesktopOtlpExportInput): Desktop
metrics: offSignal,
logs: offSignal,
resource,
warnings: [
...otel.warnings,
"OTEL_SDK_DISABLED is set, so the desktop app exports no telemetry; this overrides T3CODE_OTLP_* and Settings too",
],
warnings: [...otel.warnings],
};
}

Expand Down
12 changes: 12 additions & 0 deletions apps/server/src/cli/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,18 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => {
}),
);

it.effect("keeps exporting when T3 Code's own name says to, whatever the standard one says", () =>
Effect.gen(function* () {
const resolved = yield* resolveWithEnv({
T3CODE_OTEL_SDK_DISABLED: "false",
OTEL_SDK_DISABLED: "true",
T3CODE_OTLP_TRACES_URL: "http://localhost:4318/v1/traces",
});

expect(resolved.otlpTracesUrl).toBe("http://localhost:4318/v1/traces");
}),
);

it.effect("falls back to persisted observability settings when env vars are absent", () =>
Effect.gen(function* () {
const fs = yield* FileSystem.FileSystem;
Expand Down
6 changes: 0 additions & 6 deletions apps/server/src/observability/Layers/Observability.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ export const ObservabilityLive = Layer.unwrap(
yield* Effect.logWarning(warning);
}

if (otel.disabled) {
yield* Effect.logWarning(
"OTEL_SDK_DISABLED is set, so no telemetry is exported; this overrides T3CODE_OTLP_* and Settings too",
);
}

// One variable can decline every signal, and saying so three times reads
// like three separate problems.
const declined = new Set(
Expand Down
15 changes: 12 additions & 3 deletions docs/fork/0018-the-standard-otel-variables-are-honored.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
log record arrives.
- Turn export off from the environment. `OTEL_SDK_DISABLED=true` stops every
export, including one configured in Settings, which is the one switch a
shared machine needs.
shared machine needs. `T3CODE_OTEL_SDK_DISABLED` is the same setting asked of
T3 Code's own name first, so `false` there keeps T3 Code exporting on a
machine whose profile disables every other SDK.
- Keep whatever you have. The `T3CODE_OTLP_*` names, the desktop bootstrap
envelope, and Settings all still win over the environment, and a setup that
never mentioned OpenTelemetry keeps the wire format it always used.
Expand Down Expand Up @@ -54,8 +56,15 @@ people can actually reach.

Auto-enabling from an ambient endpoint is the deliberate part. Every other
OpenTelemetry SDK behaves this way, and a telemetry variable that some processes
honor and others quietly ignore is worse than either answer, so `OTEL_SDK_DISABLED`
is the way out rather than a requirement to opt in.
honor and others quietly ignore is worse than either answer, so
`OTEL_SDK_DISABLED` is the way out rather than a requirement to opt in.

Turning export off is one setting with two names, not two switches, and it is
read in the same order as everything else here: ours, then the standard one.
The ordering is the whole point. Inheriting `OTEL_SDK_DISABLED` from a shell
profile is common, and without a name of our own the only way to get T3 Code's
telemetry back would be to unset a variable the rest of the machine depends
on.

## Upstream considerations

Expand Down
3 changes: 2 additions & 1 deletion docs/fork/0022-the-desktop-app-reports-its-own-work.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
Metrics stay off while the main process records none, so a configured metrics
endpoint hears from the server and nobody else rather than receiving an empty
payload every interval.
- Turn it off the same way. `OTEL_SDK_DISABLED=true` stops both processes.
- Turn it off the same way. `OTEL_SDK_DISABLED=true` stops both processes, and
so does `T3CODE_OTEL_SDK_DISABLED=true`, which is read first.
- Tell the two apart without trusting the environment. The main process reports
as `t3-desktop`, joining `t3-server` and `t3-web`, and `service.runtime` on it
is always `desktop`, so an ambient
Expand Down
21 changes: 16 additions & 5 deletions docs/operations/observability.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ The base endpoint is a base, not a full URL: traces go to `<endpoint>/v1/traces`
Set `OTEL_EXPORTER_OTLP_{TRACES,METRICS,LOGS}_ENDPOINT` when a signal needs a full URL of its own.

Ambient `OTEL_*` variables turn export on by themselves. A work collector in your shell profile means
T3 Code exports to it, so use `OTEL_SDK_DISABLED=true` if that is not what you want.
T3 Code exports to it, so use `OTEL_SDK_DISABLED=true` if that is not what you want. If the reverse is
your problem, a profile that disables every other SDK on the machine, `T3CODE_OTEL_SDK_DISABLED=false`
keeps T3 Code exporting.

#### Which Processes Export

Expand Down Expand Up @@ -263,14 +265,18 @@ goes.
The three signals are resolved separately, so traces can come from one source and metrics or logs
from another.

`OTEL_SDK_DISABLED=true` outranks all four and stops every export, including one configured through
Settings.
Whether anything is exported at all is one setting, read in that same order: `T3CODE_OTEL_SDK_DISABLED`
answers it, and `OTEL_SDK_DISABLED` answers it only when T3 Code's own name is unset. Either way the
answer stops every export, including one configured through Settings, which is the one switch a shared
machine needs. Reading ours first is what lets `T3CODE_OTEL_SDK_DISABLED=false` override an ambient
`OTEL_SDK_DISABLED=true`, so a machine can disable every other SDK and still ask for T3 Code's
telemetry.

#### What Is Read

| Variable | Effect |
| ------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------- |
| `OTEL_SDK_DISABLED` | Stops all export |
| `OTEL_SDK_DISABLED` | Stops all export, unless `T3CODE_OTEL_SDK_DISABLED` answered first |
| `OTEL_EXPORTER_OTLP_ENDPOINT` | Base URL for every signal |
| `OTEL_EXPORTER_OTLP_{TRACES,METRICS,LOGS}_ENDPOINT` | Full URL for one signal |
| `OTEL_EXPORTER_OTLP_HEADERS`, `OTEL_EXPORTER_OTLP_{TRACES,METRICS,LOGS}_HEADERS` | Export headers, per signal overriding the shared ones |
Expand Down Expand Up @@ -341,7 +347,9 @@ An empty value means the same thing as an unset one, so `OTEL_SERVICE_VERSION=`
variable were not there at all. An empty `OTEL_SERVICE_NAME` is not an attempt to rename anything,
so it is not warned about either. `OTEL_SDK_DISABLED` follows the specification's one rule for
booleans: the case-insensitive string `true` is the only value that switches export off, and
anything else, including `yes` and `1`, leaves it on.
anything else, including `yes` and `1`, leaves it on. `T3CODE_OTEL_SDK_DISABLED` is T3 Code's own
name, so it takes `true`, `1`, `yes`, `on` and their negatives, and a value it cannot read is reported
and then left to `OTEL_SDK_DISABLED` to answer rather than treated as either answer itself.

A `OTEL_EXPORTER_OTLP_HEADERS` or `OTEL_RESOURCE_ATTRIBUTES` value that fails to decode is discarded
whole rather than partly. A half-parsed credential reaches the collector as the same authentication
Expand Down Expand Up @@ -725,6 +733,9 @@ OTLP export:
- `T3CODE_OTLP_HEADERS`: extra headers for all three exporters, same format as
`OTEL_EXPORTER_OTLP_HEADERS`: comma-separated `key=value` pairs with percent-encoded values.
- `T3CODE_OTLP_PROTOCOL`: `http/json` (default) or `http/protobuf`
- `T3CODE_OTEL_SDK_DISABLED`: stops every export, whatever configured it, including Settings. Read
before `OTEL_SDK_DISABLED`, so `false` here keeps T3 Code exporting on a machine that sets the
standard name.

If the OTLP URLs are unset, local tracing still works, metrics stay in-process only, and logs stay
on stdout only.
Expand Down
51 changes: 51 additions & 0 deletions packages/shared/src/otelEnvironment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,57 @@ describe("OtelEnvironment", () => {
assert.strictEqual(resolved.disabled, true);
assert.strictEqual(resolved.traces.settings, undefined);
assert.strictEqual(resolved.metrics.settings, undefined);
// Someone who inherited this from a shell profile has somewhere to go.
assert.include(resolved.warnings.join("\n"), "T3CODE_OTEL_SDK_DISABLED=false");
}),
);

it.effect("lets T3 Code's own name answer before the standard one", () =>
Effect.gen(function* () {
const off = yield* OtelEnvironment.load.pipe(
withEnv({
T3CODE_OTEL_SDK_DISABLED: "true",
OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com",
}),
);
assert.isTrue(off.disabled);
assert.strictEqual(off.traces.settings, undefined);
assert.deepStrictEqual(off.warnings, [
"T3CODE_OTEL_SDK_DISABLED is set, so no telemetry is exported, whatever configured it",
]);

// The point of reading ours first: a machine that disables every other
// SDK in its shell profile can still ask for T3 Code's telemetry.
const on = yield* OtelEnvironment.load.pipe(
withEnv({
T3CODE_OTEL_SDK_DISABLED: "false",
OTEL_SDK_DISABLED: "true",
OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com",
}),
);
assert.isFalse(on.disabled);
assert.isDefined(on.traces.settings);
assert.deepStrictEqual(on.warnings, []);
}),
);

it.effect("reads T3 Code's own name the way T3 Code reads a boolean", () =>
Effect.gen(function* () {
// Ours to define, so it takes the affirmatives people type. The
// specification's single-value rule stays with the OTEL_* name.
const numeric = yield* OtelEnvironment.load.pipe(withEnv({ T3CODE_OTEL_SDK_DISABLED: "1" }));
assert.isTrue(numeric.disabled);

// A value that answers nothing leaves the source under it to answer.
const nonsense = yield* OtelEnvironment.load.pipe(
withEnv({
T3CODE_OTEL_SDK_DISABLED: "maybe",
OTEL_SDK_DISABLED: "true",
OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com",
}),
);
assert.isTrue(nonsense.disabled);
assert.include(nonsense.warnings.join("\n"), "T3CODE_OTEL_SDK_DISABLED=maybe");
}),
);

Expand Down
62 changes: 59 additions & 3 deletions packages/shared/src/otelEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
* told twice.
*
* Read by every T3 Code process that exports telemetry, so the server and the
* desktop app cannot disagree about what a variable means.
* desktop app cannot disagree about what a variable means. That is also why
* `T3CODE_OTEL_SDK_DISABLED` is read here: it is the same setting as
* `OTEL_SDK_DISABLED`, asked of T3 Code's own name first.
*
* Only the variables T3 Code can act on are read. The exporter speaks
* OTLP over HTTP, so `grpc` is declined loudly rather than answered with a
Expand Down Expand Up @@ -85,7 +87,13 @@ export interface OtlpResourceSettings {
}

export interface OtelEnvironment {
/** `OTEL_SDK_DISABLED`. When set, nothing is exported by any route. */
/**
* Whether anything is exported at all, by any route. `T3CODE_OTEL_SDK_DISABLED`
* answers it, and `OTEL_SDK_DISABLED` answers it only when T3 Code's own name
* is unset, which is the source order every other setting here follows. So
* `T3CODE_OTEL_SDK_DISABLED=false` is how a machine that exports
* `OTEL_SDK_DISABLED` for everything else keeps T3 Code exporting.
*/
readonly disabled: boolean;
/**
* Settings that were named but could not be used, each already phrased for a
Expand Down Expand Up @@ -125,6 +133,35 @@ const optionalString = (name: string) =>
const specBoolean = (name: string) =>
optionalString(name).pipe(Effect.map((raw) => raw?.toLowerCase() === "true"));

/**
* A `T3CODE_*` name is ours, so it answers to the affirmatives people actually
* type rather than the single value the specification allows. `undefined` means
* the name did not answer, either because it is unset or because its value was
* unreadable, and the source under it decides instead. A typo therefore costs
* that variable and nothing else, the same as everywhere else here.
*/
const t3Boolean = (name: string) =>
optionalString(name).pipe(
Effect.map(
(raw): { readonly value: boolean | undefined; readonly warnings: ReadonlyArray<string> } => {
if (raw === undefined) {
return { value: undefined, warnings: [] };
}
const value = raw.toLowerCase();
if (["true", "1", "yes", "on"].includes(value)) {
return { value: true, warnings: [] };
}
if (["false", "0", "no", "off"].includes(value)) {
return { value: false, warnings: [] };
}
return {
value: undefined,
warnings: [`${name}=${raw} is not a yes or a no and was ignored`],
};
},
),
);

/**
* A number that is not a number is warned about and dropped, which is what the
* specification asks for anywhere a value is unrecognized. Letting the read
Expand Down Expand Up @@ -446,14 +483,29 @@ const resolveResource = Effect.gen(function* () {

const UNREADABLE = "the OpenTelemetry environment could not be read";

/**
* Whichever name switched export off is the one worth naming, because it is
* the one the reader has to go and unset. An ambient `OTEL_SDK_DISABLED` is
* the case where that is not obvious and where the answer is not to unset
* anything, so the message carries the override with it.
*/
const disabledBy = (name: string) =>
name === "OTEL_SDK_DISABLED"
? "OTEL_SDK_DISABLED is set, so no telemetry is exported, whatever configured it; set T3CODE_OTEL_SDK_DISABLED=false to export anyway"
: `${name} is set, so no telemetry is exported, whatever configured it`;

/**
* Read the environment. Never fails: a variable T3 Code cannot honor
* leaves the corresponding setting unset and is reported through the signal's
* `declined`, because an unparseable telemetry knob is not a reason to refuse
* to start.
*/
export const load: Effect.Effect<OtelEnvironment> = Effect.gen(function* () {
const disabled = yield* specBoolean("OTEL_SDK_DISABLED");
const t3 = yield* t3Boolean("T3CODE_OTEL_SDK_DISABLED");
const spec = yield* specBoolean("OTEL_SDK_DISABLED");
// One setting, read the way every other setting here is read: T3 Code's own
// name answers it, and the standard name answers it only when ours is unset.
const disabled = t3.value ?? spec;
const protocolDecision = yield* resolveProtocol;
const resource = yield* resolveResource;
const temporality = yield* resolveMetricsTemporality;
Expand All @@ -472,6 +524,10 @@ export const load: Effect.Effect<OtelEnvironment> = Effect.gen(function* () {
// bad value arrives here once per signal and would be logged that often.
warnings: [
...new Set([
...t3.warnings,
...(disabled
? [disabledBy(t3.value === true ? "T3CODE_OTEL_SDK_DISABLED" : "OTEL_SDK_DISABLED")]
: []),
...protocolDecision.warnings,
...resource.warnings,
...temporality.warnings,
Expand Down
Loading