fix(exporter-collector): default endpoint for node and browser (#1197)

Co-authored-by: Mayur Kale <[email protected]>
srikanthccv · Jun 17, 2020 · deb3570 · deb3570
1 parent b20bbf9
commit deb3570
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 4 deletions.
diff --git a/packages/opentelemetry-exporter-collector/README.md b/packages/opentelemetry-exporter-collector/README.md
@@ -15,6 +15,7 @@ npm install --save @opentelemetry/exporter-collector
 ```
 
 ## Usage in Web
+The CollectorExporter in Web expects the endpoint to end in `/v1/trace`. 
 
 ```js
 import { SimpleSpanProcessor } from '@opentelemetry/tracing';
@@ -34,14 +35,15 @@ provider.register();
 ```
 
 ## Usage in Node
+The CollectorExporter in Node expects the URL to only be the hostname. It will not work with `/v1/trace`.
 
 ```js
 const { BasicTracerProvider, SimpleSpanProcessor } = require('@opentelemetry/tracing');
 const { CollectorExporter } =  require('@opentelemetry/exporter-collector');
 
 const collectorOptions = {
   serviceName: 'basic-service',
-  url: '<opentelemetry-collector-url>' // url is optional and can be omitted - default is http://localhost:55678/v1/trace
+  url: '<opentelemetry-collector-url>' // url is optional and can be omitted - default is localhost:55678
 };
 
 const provider = new BasicTracerProvider();
@@ -62,7 +64,7 @@ const { CollectorExporter } =  require('@opentelemetry/exporter-collector');
 
 const collectorOptions = {
   serviceName: 'basic-service',
-  url: '<opentelemetry-collector-url>', // url is optional and can be omitted - default is http://localhost:55678/v1/trace
+  url: '<opentelemetry-collector-url>', // url is optional and can be omitted - default is localhost:55678
   credentials: grpc.credentials.createSsl(
     fs.readFileSync('./ca.crt'),
     fs.readFileSync('./client.key'),

diff --git a/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts b/packages/opentelemetry-exporter-collector/src/CollectorExporterBase.ts
@@ -31,7 +31,6 @@ export interface CollectorExporterConfigBase {
 }
 
 const DEFAULT_SERVICE_NAME = 'collector-exporter';
-const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace';
 
 /**
  * Collector Exporter abstract base class
@@ -51,7 +50,7 @@ export abstract class CollectorExporterBase<
    */
   constructor(config: T = {} as T) {
     this.serviceName = config.serviceName || DEFAULT_SERVICE_NAME;
-    this.url = config.url || DEFAULT_COLLECTOR_URL;
+    this.url = this.getDefaultUrl(config.url);
     if (typeof config.hostName === 'string') {
       this.hostName = config.hostName;
     }
@@ -135,4 +134,5 @@ export abstract class CollectorExporterBase<
     onSuccess: () => void,
     onError: (error: CollectorExporterError) => void
   ): void;
+  abstract getDefaultUrl(url: string | undefined): string;
 }
diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/CollectorExporter.ts
@@ -24,6 +24,8 @@ import * as collectorTypes from '../../types';
 
 export type CollectorExporterConfig = CollectorExporterConfigBase;
 
+const DEFAULT_COLLECTOR_URL = 'http://localhost:55678/v1/trace';
+
 /**
  * Collector Exporter for Web
  */
@@ -38,6 +40,10 @@ export class CollectorExporter extends CollectorExporterBase<
     window.removeEventListener('unload', this.shutdown);
   }
 
+  getDefaultUrl(url: string | undefined) {
+    return url || DEFAULT_COLLECTOR_URL;
+  }
+
   sendSpans(
     spans: ReadableSpan[],
     onSuccess: () => void,

diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporter.ts
@@ -29,6 +29,8 @@ import { toCollectorExportTraceServiceRequest } from '../../transform';
 import { GRPCQueueItem, TraceServiceClient } from './types';
 import { removeProtocol } from './util';
 
+const DEFAULT_COLLECTOR_URL = 'localhost:55678';
+
 /**
  * Collector Exporter Config for Node
  */
@@ -135,4 +137,8 @@ export class CollectorExporter extends CollectorExporterBase<
       });
     }
   }
+
+  getDefaultUrl(url: string | undefined): string {
+    return url || DEFAULT_COLLECTOR_URL;
+  }
 }
diff --git a/packages/opentelemetry-exporter-collector/test/browser/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/browser/CollectorExporter.test.ts
@@ -219,3 +219,24 @@ describe('CollectorExporter - web', () => {
     });
   });
 });
+
+describe('CollectorExporter - browser (getDefaultUrl)', () => {
+  it('should default to v1/trace', done => {
+    const collectorExporter = new CollectorExporter({});
+    setTimeout(() => {
+      assert.strictEqual(
+        collectorExporter['url'],
+        'http://localhost:55678/v1/trace'
+      );
+      done();
+    });
+  });
+  it('should keep the URL if included', done => {
+    const url = 'http://foo.bar.com';
+    const collectorExporter = new CollectorExporter({ url });
+    setTimeout(() => {
+      assert.strictEqual(collectorExporter['url'], url);
+      done();
+    });
+  });
+});
diff --git a/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/common/CollectorExporter.test.ts
@@ -30,6 +30,9 @@ class CollectorExporter extends CollectorExporterBase<CollectorExporterConfig> {
   onInit() {}
   onShutdown() {}
   sendSpans() {}
+  getDefaultUrl(url: string | undefined) {
+    return url || '';
+  }
 }
 
 describe('CollectorExporter - common', () => {

diff --git a/packages/opentelemetry-exporter-collector/test/node/CollectorExporter.test.ts b/packages/opentelemetry-exporter-collector/test/node/CollectorExporter.test.ts
@@ -159,5 +159,23 @@ const testCollectorExporter = (params: TestParams) =>
     });
   });
 
+describe('CollectorExporter - node (getDefaultUrl)', () => {
+  it('should default to localhost', done => {
+    const collectorExporter = new CollectorExporter({});
+    setTimeout(() => {
+      assert.strictEqual(collectorExporter['url'], 'localhost:55678');
+      done();
+    });
+  });
+  it('should keep the URL if included', done => {
+    const url = 'http://foo.bar.com';
+    const collectorExporter = new CollectorExporter({ url });
+    setTimeout(() => {
+      assert.strictEqual(collectorExporter['url'], url);
+      done();
+    });
+  });
+});
+
 testCollectorExporter({ useTLS: true });
 testCollectorExporter({ useTLS: false });