fix(observability): read the standard OTEL_* variables above stored endpoints

apps/desktop/src/app/DesktopObservability.ts

@@ -365,9 +365,14 @@ const resolveOtlpExport = Effect.gen(function* () {
   return resolveDesktopOtlpExport({
     otel: environment.otelEnvironment,
     named: {
-      traces: Option.getOrUndefined(environment.otlpTracesUrl) ?? persisted.otlpTracesUrl,
-      metrics: Option.getOrUndefined(environment.otlpMetricsUrl) ?? persisted.otlpMetricsUrl,
-      logs: Option.getOrUndefined(environment.otlpLogsUrl) ?? persisted.otlpLogsUrl,
+      traces: Option.getOrUndefined(environment.otlpTracesUrl),
+      metrics: Option.getOrUndefined(environment.otlpMetricsUrl),
+      logs: Option.getOrUndefined(environment.otlpLogsUrl),
+    },
+    persisted: {
+      traces: persisted.otlpTracesUrl,
+      metrics: persisted.otlpMetricsUrl,
+      logs: persisted.otlpLogsUrl,
     },
     namedExportIntervalMs: Option.getOrUndefined(environment.otlpExportIntervalMs),
     namedHeaders: Option.getOrUndefined(environment.otlpHeaders),

apps/desktop/src/app/DesktopOtlpExport.test.ts

@@ -6,11 +6,11 @@ import * as Layer from "effect/Layer";
 
 import {
   DEFAULT_DESKTOP_EXPORT_INTERVAL_MS,
-  type DesktopNamedOtlpEndpoints,
+  type DesktopOtlpEndpoints,
   resolveDesktopOtlpExport,
 } from "./DesktopOtlpExport.ts";
 
-const noNamedEndpoints: DesktopNamedOtlpEndpoints = {
+const noEndpoints: DesktopOtlpEndpoints = {
   traces: undefined,
   metrics: undefined,
   logs: undefined,
@@ -19,7 +19,8 @@ const noNamedEndpoints: DesktopNamedOtlpEndpoints = {
 const resolve = (
   env: Record<string, string>,
   overrides: {
-    readonly named?: Partial<DesktopNamedOtlpEndpoints>;
+    readonly named?: Partial<DesktopOtlpEndpoints>;
+    readonly persisted?: Partial<DesktopOtlpEndpoints>;
     readonly namedExportIntervalMs?: number;
     readonly namedHeaders?: Readonly<Record<string, string>>;
     readonly namedProtocol?: "http/json" | "http/protobuf";
@@ -30,7 +31,8 @@ const resolve = (
     Effect.map((otel) =>
       resolveDesktopOtlpExport({
         otel,
-        named: { ...noNamedEndpoints, ...overrides.named },
+        named: { ...noEndpoints, ...overrides.named },
+        persisted: { ...noEndpoints, ...overrides.persisted },
         namedExportIntervalMs: overrides.namedExportIntervalMs,
         namedHeaders: overrides.namedHeaders,
         namedProtocol: overrides.namedProtocol,
@@ -63,6 +65,45 @@ describe("resolveDesktopOtlpExport", () => {
     }),
   );
 
+  it.effect("reads an exported endpoint before a stored one", () =>
+    Effect.gen(function* () {
+      // Same order the server uses, because a machine's variables must not
+      // resolve differently in the two processes reading them.
+      const resolved = yield* resolve(
+        { OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com" },
+        {
+          named: { logs: "http://localhost:4318/v1/logs" },
+          persisted: {
+            traces: "http://stored.example.com/v1/traces",
+            metrics: "http://stored.example.com/v1/metrics",
+            logs: "http://stored.example.com/v1/logs",
+          },
+        },
+      );
+      assert.strictEqual(resolved.traces.url, "https://collector.example.com/v1/traces");
+      assert.strictEqual(resolved.traces.protocol, "http/protobuf");
+      assert.strictEqual(resolved.metrics.url, "https://collector.example.com/v1/metrics");
+      // T3 Code's own name outranks both, and keeps T3 Code's wire format.
+      assert.strictEqual(resolved.logs.url, "http://localhost:4318/v1/logs");
+      assert.strictEqual(resolved.logs.protocol, "http/json");
+    }),
+  );
+
+  it.effect("falls back to a stored endpoint for the signals nothing exported", () =>
+    Effect.gen(function* () {
+      const resolved = yield* resolve(
+        { OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: "https://collector.example.com/v1/traces" },
+        { persisted: { metrics: "http://stored.example.com/v1/metrics" } },
+      );
+      assert.strictEqual(resolved.traces.url, "https://collector.example.com/v1/traces");
+      assert.strictEqual(resolved.metrics.url, "http://stored.example.com/v1/metrics");
+      // A stored endpoint keeps the interval T3 Code has always used, because
+      // the variables that name one did not name this endpoint.
+      assert.strictEqual(resolved.metrics.exportIntervalMs, DEFAULT_DESKTOP_EXPORT_INTERVAL_MS);
+      assert.strictEqual(resolved.logs.url, undefined);
+    }),
+  );
+
   it.effect("lets a named endpoint take the whole signal, not only its url", () =>
     Effect.gen(function* () {
       const resolved = yield* resolve(
@@ -167,21 +208,61 @@ describe("resolveDesktopOtlpExport", () => {
     }),
   );
 
-  it.effect("honors one T3 Code interval across every signal", () =>
+  it.effect("honors one T3 Code interval across every signal it named", () =>
     Effect.gen(function* () {
       const resolved = yield* resolve(
+        {},
         {
-          OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com",
-          OTEL_METRIC_EXPORT_INTERVAL: "30000",
+          named: {
+            traces: "http://127.0.0.1:4318/v1/traces",
+            metrics: "http://127.0.0.1:4318/v1/metrics",
+            logs: "http://127.0.0.1:4318/v1/logs",
+          },
+          namedExportIntervalMs: 2500,
         },
-        { namedExportIntervalMs: 2500 },
       );
       assert.strictEqual(resolved.traces.exportIntervalMs, 2500);
       assert.strictEqual(resolved.metrics.exportIntervalMs, 2500);
       assert.strictEqual(resolved.logs.exportIntervalMs, 2500);
     }),
   );
 
+  it.effect("keeps a stored endpoint from re-enabling a signal turned off by name", () =>
+    Effect.gen(function* () {
+      // The main process reads the same sources in the same order as the
+      // server, so a signal an exported variable switched off must not come
+      // back here from the endpoint Settings remembers.
+      const resolved = yield* resolve(
+        {
+          OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com",
+          OTEL_LOGS_EXPORTER: "none",
+        },
+        { persisted: { logs: "https://stored.example.com/v1/logs" } },
+      );
+      assert.strictEqual(resolved.logs.url, undefined);
+      assert.strictEqual(resolved.traces.url, "https://collector.example.com/v1/traces");
+    }),
+  );
+
+  it.effect("keeps one T3 Code interval off the endpoints it did not name", () =>
+    Effect.gen(function* () {
+      // The source that named an endpoint sets the cadence of the export it
+      // configured. Letting `T3CODE_OTLP_EXPORT_INTERVAL_MS` reach across
+      // would have it pace an export it knows nothing about, and would
+      // discard the per-signal number standing right next to that endpoint.
+      const resolved = yield* resolve(
+        {
+          OTEL_EXPORTER_OTLP_ENDPOINT: "https://collector.example.com",
+          OTEL_METRIC_EXPORT_INTERVAL: "30000",
+        },
+        { namedExportIntervalMs: 2500 },
+      );
+      assert.strictEqual(resolved.metrics.exportIntervalMs, 30_000);
+      assert.strictEqual(resolved.traces.exportIntervalMs, 5_000);
+      assert.strictEqual(resolved.logs.exportIntervalMs, 1_000);
+    }),
+  );
+
   it.effect("falls back to the interval the specification defines for each signal", () =>
     Effect.gen(function* () {
       const resolved = yield* resolve({

apps/desktop/src/app/DesktopOtlpExport.ts

@@ -29,7 +29,7 @@ export interface DesktopOtlpSignal {
   readonly protocol: OtelEnvironment.OtlpProtocol;
   readonly headers: Readonly<Record<string, string>> | undefined;
   readonly maxBatchSize: number | undefined;
-  readonly temporality: OtelEnvironment.MetricsTemporality | undefined;
+  readonly temporality: OtelEnvironment.MetricsTemporality;
 }
 
 export interface DesktopOtlpResource {
@@ -47,16 +47,19 @@ export interface DesktopOtlpExport {
   readonly warnings: ReadonlyArray<string>;
 }
 
-/** An endpoint named outside the OpenTelemetry variables, per signal. */
-export interface DesktopNamedOtlpEndpoints {
+/** One endpoint per signal, from a single source. */
+export interface DesktopOtlpEndpoints {
   readonly traces: string | undefined;
   readonly metrics: string | undefined;
   readonly logs: string | undefined;
 }
 
 export interface DesktopOtlpExportInput {
   readonly otel: OtelEnvironment.OtelEnvironment;
-  readonly named: DesktopNamedOtlpEndpoints;
+  /** `T3CODE_OTLP_*`, which outranks everything. */
+  readonly named: DesktopOtlpEndpoints;
+  /** Settings, which answers under both sets of variables. */
+  readonly persisted: DesktopOtlpEndpoints;
   /** `T3CODE_OTLP_EXPORT_INTERVAL_MS`, which deliberately covers every signal. */
   readonly namedExportIntervalMs: number | undefined;
   /** `T3CODE_OTLP_HEADERS`, which deliberately covers every signal. */
@@ -79,41 +82,35 @@ const offSignal: DesktopOtlpSignal = {
   protocol: DEFAULT_DESKTOP_PROTOCOL,
   headers: undefined,
   maxBatchSize: undefined,
-  temporality: undefined,
+  temporality: OtelEnvironment.DEFAULT_METRICS_TEMPORALITY,
 };
 
 /**
- * A signal whose endpoint came from somewhere else is not the OpenTelemetry
- * variables' to configure. Dropping the whole signal, rather than the endpoint
- * alone, is what stops an ambient `OTEL_EXPORTER_OTLP_ENDPOINT` from changing
- * the wire format, headers, or batching of an export a `T3CODE_OTLP_*` name or
- * Settings already answered.
+ * Turns the source that won one signal into what the exporter needs.
+ * `OtelEnvironment.resolveSignalSource` has already decided which source that
+ * is, and hands back settings only when the `OTEL_*` variables are the ones
+ * that named the endpoint.
  */
 const resolveSignal = (
-  named: string | undefined,
-  signal: OtelEnvironment.OtlpSignal,
+  resolved: { readonly url: string | undefined; readonly signal: OtelEnvironment.OtlpSignal },
   input: DesktopOtlpExportInput,
 ): DesktopOtlpSignal => {
-  const settings = named === undefined ? signal.settings : undefined;
-  const url = named ?? settings?.url;
-  if (url === undefined) {
+  if (resolved.url === undefined) {
     return offSignal;
   }
   return {
-    url,
-    exportIntervalMs:
-      input.namedExportIntervalMs ??
-      settings?.exportIntervalMs ??
-      DEFAULT_DESKTOP_EXPORT_INTERVAL_MS,
-    protocol: settings?.protocol ?? input.namedProtocol ?? DEFAULT_DESKTOP_PROTOCOL,
-    headers: settings?.headers ?? input.namedHeaders,
-    maxBatchSize: settings?.maxBatchSize,
-    temporality: settings?.temporality,
+    url: resolved.url,
+    ...OtelEnvironment.resolveSignalExport({
+      settings: resolved.signal.settings,
+      t3Protocol: input.namedProtocol ?? DEFAULT_DESKTOP_PROTOCOL,
+      t3Headers: input.namedHeaders,
+      t3ExportIntervalMs: input.namedExportIntervalMs ?? DEFAULT_DESKTOP_EXPORT_INTERVAL_MS,
+    }),
   };
 };
 
 export const resolveDesktopOtlpExport = (input: DesktopOtlpExportInput): DesktopOtlpExport => {
-  const { otel, named } = input;
+  const { otel } = input;
   const resource: DesktopOtlpResource = {
     serviceName: input.serviceName,
     serviceVersion: otel.resource.serviceVersion,
@@ -130,23 +127,33 @@ export const resolveDesktopOtlpExport = (input: DesktopOtlpExportInput): Desktop
     };
   }
 
-  const signals = {
-    traces: named.traces === undefined ? otel.traces : OtelEnvironment.noSignal,
-    metrics: named.metrics === undefined ? otel.metrics : OtelEnvironment.noSignal,
-    logs: named.logs === undefined ? otel.logs : OtelEnvironment.noSignal,
-  };
+  const traces = OtelEnvironment.resolveSignalSource({
+    t3Url: input.named.traces,
+    signal: otel.traces,
+    persistedUrl: input.persisted.traces,
+  });
+  const metrics = OtelEnvironment.resolveSignalSource({
+    t3Url: input.named.metrics,
+    signal: otel.metrics,
+    persistedUrl: input.persisted.metrics,
+  });
+  const logs = OtelEnvironment.resolveSignalSource({
+    t3Url: input.named.logs,
+    signal: otel.logs,
+    persistedUrl: input.persisted.logs,
+  });
 
   // One variable can decline every signal, and saying so three times reads
   // like three separate problems.
   return {
-    traces: resolveSignal(named.traces, signals.traces, input),
-    metrics: resolveSignal(named.metrics, signals.metrics, input),
-    logs: resolveSignal(named.logs, signals.logs, input),
+    traces: resolveSignal(traces, input),
+    metrics: resolveSignal(metrics, input),
+    logs: resolveSignal(logs, input),
     resource,
     warnings: [
       ...new Set([
         ...otel.warnings,
-        ...[signals.traces.declined, signals.metrics.declined, signals.logs.declined].filter(
+        ...[traces.signal.declined, metrics.signal.declined, logs.signal.declined].filter(
           (reason): reason is string => reason !== undefined,
         ),
       ]),