Skip to content

Commit

Permalink
feat(http-plugin): add options to disable new spans if no parent open…
Browse files Browse the repository at this point in the history
  • Loading branch information
vmarchaud committed May 6, 2020
1 parent 2b14af3 commit e5a31d7
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 5 deletions.
2 changes: 2 additions & 0 deletions packages/opentelemetry-plugin-http/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ Http plugin has few options available to choose from. You can set the following:
| [`ignoreIncomingPaths`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `IgnoreMatcher[]` | Http plugin will not trace all incoming requests that match paths |
| [`ignoreOutgoingUrls`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `IgnoreMatcher[]` | Http plugin will not trace all outgoing requests that match urls |
| [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-plugin-http/src/types.ts#L28) | `string` | The primary server name of the matched virtual host. |
| `requireParentforOutgoingSpans` | Boolean | Require that is a parent span to create new span for outgoing requests. |
| `requireParentforIncomingSpans` | Boolean | Require that is a parent span to create new span for incoming requests. |

## Useful links
- For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
Expand Down
38 changes: 33 additions & 5 deletions packages/opentelemetry-plugin-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,14 @@ import {
SpanKind,
SpanOptions,
Status,
SpanContext,
TraceFlags,
} from '@opentelemetry/api';
import { BasePlugin } from '@opentelemetry/core';
import {
BasePlugin,
NoRecordingSpan,
getExtractedSpanContext,
} from '@opentelemetry/core';
import {
ClientRequest,
IncomingMessage,
Expand Down Expand Up @@ -57,6 +63,12 @@ export class HttpPlugin extends BasePlugin<Http> {
/** keep track on spans not ended */
private readonly _spanNotEnded: WeakSet<Span>;

private readonly _emptySpanContext: SpanContext = {
traceId: '',
spanId: '',
traceFlags: TraceFlags.NONE,
};

constructor(readonly moduleName: string, readonly version: string) {
super(`@opentelemetry/plugin-${moduleName}`, VERSION);
// For now component is equal to moduleName but it can change in the future.
Expand Down Expand Up @@ -396,7 +408,6 @@ export class HttpPlugin extends BasePlugin<Http> {
const spanOptions: SpanOptions = {
kind: SpanKind.CLIENT,
};

const span = plugin._startHttpSpan(operationName, spanOptions);

return plugin._tracer.withSpan(span, () => {
Expand All @@ -417,9 +428,26 @@ export class HttpPlugin extends BasePlugin<Http> {
}

private _startHttpSpan(name: string, options: SpanOptions) {
const span = this._tracer
.startSpan(name, options)
.setAttribute(AttributeNames.COMPONENT, this.component);
/*
* If a parent is required but not present, we use a `NoRecordingSpan` to still
* propagate context without recording it.
*/
const requireParent =
options.kind === SpanKind.CLIENT
? this._config.requireParentforOutgoingSpans
: this._config.requireParentforIncomingSpans;
let span: Span;
if (requireParent === true && this._tracer.getCurrentSpan() === undefined) {
const spanContext =
getExtractedSpanContext(context.active()) ?? plugin._emptySpanContext;
// TODO: Refactor this when a solution is found in
// https://github.com/open-telemetry/opentelemetry-specification/issues/530
span = new NoRecordingSpan(spanContext);
} else {
span = this._tracer
.startSpan(name, options)
.setAttribute(AttributeNames.COMPONENT, this.component);
}
this._spanNotEnded.add(span);
return span;
}
Expand Down
4 changes: 4 additions & 0 deletions packages/opentelemetry-plugin-http/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ export interface HttpPluginConfig extends PluginConfig {
applyCustomAttributesOnSpan?: HttpCustomAttributeFunction;
/** The primary server name of the matched virtual host. */
serverName?: string;
/** Require parent to create span for outgoing requests */
requireParentforOutgoingSpans?: boolean;
/** Require parent to create span for incoming requests */
requireParentforIncomingSpans?: boolean;
}

export interface Err extends Error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -696,5 +696,125 @@ describe('HttpPlugin', () => {
req.end();
});
});

describe('with require parent span', () => {
beforeEach(done => {
memoryExporter.reset();
plugin.enable(http, provider, provider.logger, {});
server = http.createServer((request, response) => {
response.end('Test Server Response');
});
server.listen(serverPort, done);
});

afterEach(() => {
server.close();
plugin.disable();
});

it(`should not trace without parent with options enabled (both client & server)`, async () => {
plugin.disable();
const config: HttpPluginConfig = {
requireParentforIncomingSpans: true,
requireParentforOutgoingSpans: true,
};
plugin.enable(http, provider, provider.logger, config);
const testPath = `/test/test`;
await httpRequest.get(
`${protocol}://${hostname}:${serverPort}${testPath}`
);
const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 0);
});

it(`should not trace without parent with options enabled (client only)`, async () => {
plugin.disable();
const config: HttpPluginConfig = {
requireParentforOutgoingSpans: true,
};
plugin.enable(http, provider, provider.logger, config);
const testPath = `/test/test`;
const result = await httpRequest.get(
`${protocol}://${hostname}:${serverPort}${testPath}`
);
assert(
result.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY] !== undefined
);
assert(
result.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY] !== undefined
);
const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1);
assert.strictEqual(
spans.every(span => span.kind === SpanKind.SERVER),
true
);
});

it(`should not trace without parent with options enabled (server only)`, async () => {
plugin.disable();
const config: HttpPluginConfig = {
requireParentforIncomingSpans: true,
};
plugin.enable(http, provider, provider.logger, config);
const testPath = `/test/test`;
const result = await httpRequest.get(
`${protocol}://${hostname}:${serverPort}${testPath}`
);
assert(
result.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY] !== undefined
);
assert(
result.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY] !== undefined
);
const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1);
assert.strictEqual(
spans.every(span => span.kind === SpanKind.CLIENT),
true
);
});

it(`should trace with parent with both requireParent options enabled`, done => {
plugin.disable();
const config: HttpPluginConfig = {
requireParentforIncomingSpans: true,
requireParentforOutgoingSpans: true,
};
plugin.enable(http, provider, provider.logger, config);
const testPath = `/test/test`;
const tracer = provider.getTracer('default');
const span = tracer.startSpan('parentSpan', {
kind: SpanKind.INTERNAL,
});
tracer.withSpan(span, () => {
httpRequest
.get(`${protocol}://${hostname}:${serverPort}${testPath}`)
.then(result => {
span.end();
assert(
result.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY] !==
undefined
);
assert(
result.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY] !==
undefined
);
const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 2);
assert.strictEqual(
spans.filter(span => span.kind === SpanKind.CLIENT).length,
1
);
assert.strictEqual(
spans.filter(span => span.kind === SpanKind.INTERNAL).length,
1
);
return done();
})
.catch(done);
});
});
});
});
});

0 comments on commit e5a31d7

Please sign in to comment.