From ac3f2ddb5bfa30ed2bd46f7a4f921803d5e99b6e Mon Sep 17 00:00:00 2001 From: Naseem 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 --- .../opentelemetry-exporter-jaeger/README.md | 7 ++++- .../src/jaeger.ts | 2 ++ .../test/jaeger.test.ts | 31 ++++++++++++++++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-exporter-jaeger/README.md b/packages/opentelemetry-exporter-jaeger/README.md index de0cfac414..ad8c0a88e0 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,10 @@ 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. If both are +set, the value set by the option in code is authoritative. + ```js import { JaegerExporter } from '@opentelemetry/exporter-jaeger'; @@ -78,8 +83,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: - For more about OpenTelemetry JavaScript: diff --git a/packages/opentelemetry-exporter-jaeger/src/jaeger.ts b/packages/opentelemetry-exporter-jaeger/src/jaeger.ts index eae71a518e..83f6658492 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 12b34f466b..53105fd3ce 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',