From 3decd1e8379408f6db2a8b49eb73beb6cd4de0c0 Mon Sep 17 00:00:00 2001
From: Naseem <naseem@transit.app>
Date: Fri, 3 Apr 2020 00:03:43 -0400
Subject: [PATCH] feat: check for JAEGER_AGENT_HOST to populate exporter's host
 if none defined

It still falls back to localhost, but if the env var is set, it will use that as the host to send spans to. This prevents end users from writing extra code when the jaeger agent is remote.

Signed-off-by: Naseem <naseem@transit.app>
---
 .../opentelemetry-exporter-jaeger/README.md   |  6 +++-
 .../src/jaeger.ts                             |  2 ++
 .../test/jaeger.test.ts                       | 31 ++++++++++++++++++-
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/packages/opentelemetry-exporter-jaeger/README.md b/packages/opentelemetry-exporter-jaeger/README.md
index de0cfac4148..09bfd7d3650 100644
--- a/packages/opentelemetry-exporter-jaeger/README.md
+++ b/packages/opentelemetry-exporter-jaeger/README.md
@@ -1,4 +1,5 @@
 # OpenTelemetry Jaeger Trace Exporter
+
 [![Gitter chat][gitter-image]][gitter-url]
 [![NPM Published Version][npm-img]][npm-url]
 [![dependencies][dependencies-image]][dependencies-url]
@@ -54,6 +55,9 @@ npm install --save @opentelemetry/exporter-jaeger
 
 Install the exporter on your application and pass the options, it must contain a service name.
 
+Furthermore, the `host` option (which defaults to `localhost`), can instead be set by the
+`JAEGER_AGENT_HOST` environment variable to reduce in-code config.
+
 ```js
 import { JaegerExporter } from '@opentelemetry/exporter-jaeger';
 
@@ -78,8 +82,8 @@ You can use built-in `SimpleSpanProcessor` or `BatchSpanProcessor` or write your
 - [SimpleSpanProcessor](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-tracing.md#simple-processor): The implementation of `SpanProcessor` that passes ended span directly to the configured `SpanExporter`.
 - [BatchSpanProcessor](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-tracing.md#batching-processor): The implementation of the `SpanProcessor` that batches ended spans and pushes them to the configured `SpanExporter`. It is recommended to use this `SpanProcessor` for better performance and optimization.
 
-
 ## Useful links
+
 - To know more about Jaeger, visit: https://www.jaegertracing.io/docs/latest/getting-started/
 - For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
 - For more about OpenTelemetry JavaScript: <https://github.com/open-telemetry/opentelemetry-js>
diff --git a/packages/opentelemetry-exporter-jaeger/src/jaeger.ts b/packages/opentelemetry-exporter-jaeger/src/jaeger.ts
index eae71a518e6..83f66584925 100644
--- a/packages/opentelemetry-exporter-jaeger/src/jaeger.ts
+++ b/packages/opentelemetry-exporter-jaeger/src/jaeger.ts
@@ -37,6 +37,8 @@ export class JaegerExporter implements SpanExporter {
     this._onShutdownFlushTimeout =
       typeof config.flushTimeout === 'number' ? config.flushTimeout : 2000;
 
+    config.host = config.host || process.env.JAEGER_AGENT_HOST;
+
     this._sender = new jaegerTypes.UDPSender(config);
     if (this._sender._client instanceof Socket) {
       // unref socket to prevent it from keeping the process running
diff --git a/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts b/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts
index 12b34f466bc..ebbd93d32aa 100644
--- a/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts
+++ b/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts
@@ -26,6 +26,10 @@ import { Resource } from '@opentelemetry/resources';
 
 describe('JaegerExporter', () => {
   describe('constructor', () => {
+    afterEach(() => {
+      delete process.env.JAEGER_AGENT_HOST;
+    });
+
     it('should construct an exporter', () => {
       const exporter = new JaegerExporter({ serviceName: 'opentelemetry' });
       assert.ok(typeof exporter.export === 'function');
@@ -38,7 +42,7 @@ describe('JaegerExporter', () => {
     it('should construct an exporter with host, port, logger and tags', () => {
       const exporter = new JaegerExporter({
         serviceName: 'opentelemetry',
-        host: 'localhost',
+        host: 'remotehost',
         port: 8080,
         logger: new NoopLogger(),
         tags: [{ key: 'opentelemetry-exporter-jaeger', value: '0.1.0' }],
@@ -47,6 +51,7 @@ describe('JaegerExporter', () => {
       assert.ok(typeof exporter.shutdown === 'function');
 
       const process: ThriftProcess = exporter['_sender']._process;
+      assert.strictEqual(exporter['_sender']._host, 'remotehost');
       assert.strictEqual(process.serviceName, 'opentelemetry');
       assert.strictEqual(process.tags.length, 1);
       assert.strictEqual(process.tags[0].key, 'opentelemetry-exporter-jaeger');
@@ -54,6 +59,30 @@ describe('JaegerExporter', () => {
       assert.strictEqual(process.tags[0].vStr, '0.1.0');
     });
 
+    it('should default to localhost if no host is configured', () => {
+      const exporter = new JaegerExporter({
+        serviceName: 'opentelemetry',
+      });
+      assert.strictEqual(exporter['_sender']._host, 'localhost');
+    });
+
+    it('should respect jaeger host env variable', () => {
+      process.env.JAEGER_AGENT_HOST = 'env-set-host';
+      const exporter = new JaegerExporter({
+        serviceName: 'test-service',
+      });
+      assert.strictEqual(exporter['_sender']._host, 'env-set-host');
+    });
+
+    it('should prioritize host option over env variable', () => {
+      process.env.JAEGER_AGENT_HOST = 'env-set-host';
+      const exporter = new JaegerExporter({
+        serviceName: 'test-service',
+        host: 'option-set-host'
+      });
+      assert.strictEqual(exporter['_sender']._host, 'option-set-host');
+    });
+
     it('should construct an exporter with flushTimeout', () => {
       const exporter = new JaegerExporter({
         serviceName: 'opentelemetry',