fix: do not assume a default value for DASH0_OTEL_COLLECTOR_BASE_URL

Author: basti1302

src/index.ts

@@ -37,10 +37,12 @@ function init() {
   }
 }
 
-if (process.env.DASH0_DISABLE == null || process.env.DASH0_DISABLE.toLowerCase() !== 'true') {
-  init();
-} else {
+if (process.env.DASH0_DISABLE != null && process.env.DASH0_DISABLE.toLowerCase() === 'true') {
   logProhibitiveError(`The distribution has been disabled by setting DASH0_DISABLE=${process.env.DASH0_DISABLE}.`);
+} else if (process.env.DASH0_OTEL_COLLECTOR_BASE_URL == null) {
+  logProhibitiveError(`DASH0_OTEL_COLLECTOR_BASE_URL is not set.`);
+} else {
+  init();
 }
 
 function logProhibitiveError(message: string) {

src/init.ts

@@ -57,10 +57,8 @@ printDebugStdout('Starting NodeSDK.');
 
 let sdkShutdownHasBeenCalled = false;
 
-let baseUrl = 'http://dash0-operator-opentelemetry-collector.dash0-operator-system.svc.cluster.local:4318';
-if (process.env.DASH0_OTEL_COLLECTOR_BASE_URL) {
-  baseUrl = process.env.DASH0_OTEL_COLLECTOR_BASE_URL;
-}
+// Note: There is a check in index.ts that this env var is set.
+const baseUrl = process.env.DASH0_OTEL_COLLECTOR_BASE_URL;
 
 const configuration: Partial<NodeSDKConfiguration> = {
   spanProcessors: spanProcessors(),

test/integration/ChildProcessWrapper.ts

@@ -26,6 +26,7 @@ const emulateKubernetesPath = path.join(repoRoot, 'test', 'integration', 'emulat
 export default class ChildProcessWrapper {
   private childProcess?: ChildProcess;
   private ready: boolean;
+  private terminated: boolean;
   private options: ChildProcessWrapperOptions;
   private nextIpcRequestId: number;
   private responseEmitter: ResponseEmitter;
@@ -36,6 +37,7 @@ export default class ChildProcessWrapper {
       this.options.label = this.options.path;
     }
     this.ready = false;
+    this.terminated = false;
     this.nextIpcRequestId = 0;
     this.responseEmitter = new ResponseEmitter();
   }
@@ -44,6 +46,9 @@ export default class ChildProcessWrapper {
     const { modulePath, forkOptions } = await this.createForkOptions();
     this.childProcess = fork(modulePath, this.options.args ?? [], forkOptions);
     this.listenToIpcMessages();
+    this.childProcess.on('exit', () => {
+      this.terminated = true;
+    });
     this.echoOutputStreams();
     await this.waitUntilReady();
   }
@@ -119,6 +124,14 @@ export default class ChildProcessWrapper {
     }, this.options.waitForReadyRetryOptions);
   }
 
+  async waitUntilTerminated() {
+    await waitUntil(() => {
+      if (!this.terminated) {
+        throw new Error('Child process has not terminated yet.');
+      }
+    }, this.options.waitForReadyRetryOptions);
+  }
+
   async stop(signal?: number | NodeJS.Signals): Promise<void> {
     if (!this.childProcess) {
       return;

test/integration/test.ts

@@ -314,6 +314,36 @@ describe('attach', () => {
     });
   });
 
+  describe('timeout for flushing telemetry', () => {
+    let appUnderTest: ChildProcessWrapper;
+
+    beforeEach(async () => {
+      const appConfiguration = {
+        path: 'test/apps/empty-event-loop',
+        label: 'app',
+        useTsNode: true,
+        useDistro: true,
+        env: {
+          ...process.env,
+          // Weird finding: When setting DASH0_OTEL_COLLECTOR_BASE_URL to any url where the host name ends in .local
+          // (like http://non-reachable-dns-name.local:4318), the timeout of 500 ms for flushing telemetry is ignored
+          // because the DNS lookup blocks for 5 seconds. This apparently happens on Mac as well as on Linux/in Docker.
+          DASH0_OTEL_COLLECTOR_BASE_URL: 'http://non-reachable-host.url:4318',
+          DASH0_BOOTSTRAP_SPAN: 'Dash0 Test Bootstrap Span',
+        },
+      };
+      appUnderTest = new ChildProcessWrapper(appConfiguration);
+    });
+
+    it('should let process terminate after timeout', async () => {
+      await appUnderTest.start();
+      const startedAt = Date.now();
+      await appUnderTest.waitUntilTerminated();
+      const terminatedAt = Date.now();
+      expect(terminatedAt - startedAt).to.be.lessThan(1000);
+    });
+  });
+
   describe('print spans to file', () => {
     let appUnderTest: ChildProcessWrapper;
     const spanFilename = join(__dirname, 'spans.json');
@@ -406,6 +436,31 @@ describe('attach', () => {
     });
   });
 
+  describe('disable when export endpoint is not set', () => {
+    let appUnderTest: ChildProcessWrapper;
+
+    before(async () => {
+      const appConfiguration = defaultAppConfiguration(appPort);
+      delete appConfiguration.env!.DASH0_OTEL_COLLECTOR_BASE_URL;
+      appUnderTest = new ChildProcessWrapper(appConfiguration);
+      await appUnderTest.start();
+    });
+
+    after(async () => {
+      await appUnderTest.stop();
+    });
+
+    it('should do nothing if DASH0_OTEL_COLLECTOR_BASE_URL is not set', async () => {
+      await delay(1000);
+      await sendHttpRequestAndVerifyResponse();
+      await delay(2000);
+
+      if (await collector().hasTelemetry()) {
+        fail('The collector received telemetry data although it should not have received anything.');
+      }
+    });
+  });
+
   async function sendHttpRequestAndFetchTraceData() {
     await sendHttpRequestAndVerifyResponse();
     return collector().fetchTraces();