From d26f01b46571c106cb54afeb5c98ebc1582d340c Mon Sep 17 00:00:00 2001 From: Tomasz Pytel Date: Mon, 6 Jun 2022 22:41:49 -0300 Subject: [PATCH] Fix issue #9165 express router entry duplicated (#84) --- src/plugins/ExpressPlugin.ts | 2 +- src/trace/context/Context.ts | 2 +- src/trace/context/DummyContext.ts | 10 +++--- src/trace/context/SpanContext.ts | 53 ++++++++++++++----------------- 4 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/plugins/ExpressPlugin.ts b/src/plugins/ExpressPlugin.ts index 23de86c..8382f2f 100644 --- a/src/plugins/ExpressPlugin.ts +++ b/src/plugins/ExpressPlugin.ts @@ -45,7 +45,7 @@ class ExpressPlugin implements SwPlugin { const operation = (req.url || '/').replace(/\?.*/g, ''); const span = ignoreHttpMethodCheck(req.method ?? 'GET') ? DummySpan.create() - : ContextManager.current.newEntrySpan(operation, carrier, Component.HTTP_SERVER); + : ContextManager.current.newEntrySpan(operation, carrier, [Component.HTTP_SERVER, Component.EXPRESS]); span.component = Component.EXPRESS; diff --git a/src/trace/context/Context.ts b/src/trace/context/Context.ts index c8ab2d4..3fe22fc 100644 --- a/src/trace/context/Context.ts +++ b/src/trace/context/Context.ts @@ -32,7 +32,7 @@ export default interface Context { /* If 'inherit' is specified then if the span at the top of the stack is an Entry span of this component type then the span is reused instead of a new child span being created. This is intended for situations like an express handler inheriting an opened incoming http connection to present a single span. */ - newEntrySpan(operation: string, carrier?: ContextCarrier, inherit?: Component): Span; + newEntrySpan(operation: string, carrier?: ContextCarrier, inherit?: Component | Component[]): Span; /* if 'inherit' is specified then the span returned is marked for inheritance by an Exit span component which is created later and calls this function with a matching 'component' value. For example Axios using an Http exit diff --git a/src/trace/context/DummyContext.ts b/src/trace/context/DummyContext.ts index 509d3e2..62389d5 100644 --- a/src/trace/context/DummyContext.ts +++ b/src/trace/context/DummyContext.ts @@ -30,7 +30,7 @@ export default class DummyContext implements Context { nSpans = 0; finished = false; - newEntrySpan(operation: string, carrier?: ContextCarrier, inherit?: Component): Span { + newEntrySpan(operation: string, carrier?: ContextCarrier, inherit?: Component | Component[]): Span { return DummySpan.create(this); } @@ -46,18 +46,16 @@ export default class DummyContext implements Context { const spans = ContextManager.spansDup(); if (!this.nSpans++) { - ContextManager.checkCold(); // set cold to false + ContextManager.checkCold(); // set cold to false - if (spans.indexOf(span) === -1) - spans.push(span); + if (spans.indexOf(span) === -1) spans.push(span); } return this; } stop(span: DummySpan): boolean { - if (--this.nSpans) - return false; + if (--this.nSpans) return false; ContextManager.clear(span); diff --git a/src/trace/context/SpanContext.ts b/src/trace/context/SpanContext.ts index a02a699..7d9f9bf 100644 --- a/src/trace/context/SpanContext.ts +++ b/src/trace/context/SpanContext.ts @@ -37,19 +37,18 @@ import { emitter } from '../../lib/EventEmitter'; const logger = createLogger(__filename); emitter.on('segments-sent', () => { - SpanContext.nActiveSegments = 0; // reset limiter + SpanContext.nActiveSegments = 0; // reset limiter }); export default class SpanContext implements Context { - static nActiveSegments = 0; // counter to allow only config.maxBufferSize active (non-dummy) segments per reporting frame + static nActiveSegments = 0; // counter to allow only config.maxBufferSize active (non-dummy) segments per reporting frame spanId = 0; nSpans = 0; finished = false; segment: Segment = new Segment(); ignoreCheck(operation: string, type: SpanType, carrier?: ContextCarrier): Span | undefined { - if (operation.match(config.reIgnoreOperation) || (carrier && !carrier.isValid())) - return DummySpan.create(); + if (operation.match(config.reIgnoreOperation) || (carrier && !carrier.isValid())) return DummySpan.create(); return undefined; } @@ -57,14 +56,12 @@ export default class SpanContext implements Context { spanCheck(spanType: SpanType, operation: string, carrier?: ContextCarrier): [Span | null, Span?] { const span = this.ignoreCheck(operation, SpanType.ENTRY, carrier); - if (span) - return [span]; + if (span) return [span]; const spans = ContextManager.spans; const parent = spans[spans.length - 1]; - if (parent instanceof DummySpan) - return [parent]; + if (parent instanceof DummySpan) return [parent]; return [null, parent]; } @@ -79,7 +76,8 @@ export default class SpanContext implements Context { operation, }); - if (this.finished && parent) { // segment has already been closed and sent to server, if there is a parent span then need new segment to reference + if (this.finished && parent) { + // segment has already been closed and sent to server, if there is a parent span then need new segment to reference const carrier = new ContextCarrier( parent.context.segment.relatedTraces[0], parent.context.segment.segmentId, @@ -100,12 +98,11 @@ export default class SpanContext implements Context { return span; } - newEntrySpan(operation: string, carrier?: ContextCarrier, inherit?: Component): Span { + newEntrySpan(operation: string, carrier?: ContextCarrier, inherit?: Component | Component[]): Span { // tslint:disable-next-line:prefer-const let [span, parent] = this.spanCheck(SpanType.ENTRY, operation, carrier); - if (span) - return span; + if (span) return span; if (logger._isDebugEnabled) { logger.debug('Creating entry span', { @@ -113,15 +110,18 @@ export default class SpanContext implements Context { }); } - if (!this.finished && parent?.type === SpanType.ENTRY && inherit && inherit === parent.component) { + if ( + !this.finished && + parent?.type === SpanType.ENTRY && + inherit && + (inherit instanceof Component ? inherit === parent.component : inherit.indexOf(parent.component) != -1) + ) { span = parent; parent.operation = operation; - } else { span = this.newSpan(EntrySpan, parent!, operation); - if (carrier && carrier.isValid()) - span.extract(carrier); + if (carrier && carrier.isValid()) span.extract(carrier); } return span; @@ -131,8 +131,7 @@ export default class SpanContext implements Context { // tslint:disable-next-line:prefer-const let [span, parent] = this.spanCheck(SpanType.EXIT, operation); - if (span) - return span; + if (span) return span; if (logger._isDebugEnabled) { logger.debug('Creating exit span', { @@ -141,13 +140,10 @@ export default class SpanContext implements Context { }); } - if (!this.finished && parent?.type === SpanType.EXIT && component === parent.inherit) - span = parent; - else - span = this.newSpan(ExitSpan, parent!, operation); + if (!this.finished && parent?.type === SpanType.EXIT && component === parent.inherit) span = parent; + else span = this.newSpan(ExitSpan, parent!, operation); - if (inherit) - span.inherit = inherit; + if (inherit) span.inherit = inherit; return span; } @@ -155,8 +151,7 @@ export default class SpanContext implements Context { newLocalSpan(operation: string): Span { const [span, parent] = this.spanCheck(SpanType.LOCAL, operation); - if (span) - return span; + if (span) return span; if (logger._isDebugEnabled) { logger.debug('Creating local span', { @@ -180,12 +175,10 @@ export default class SpanContext implements Context { SpanContext.nActiveSegments += 1; span.isCold = ContextManager.checkCold(); - if (span.isCold) - span.tag(Tag.coldStart(), true); + if (span.isCold) span.tag(Tag.coldStart(), true); } - if (spans.indexOf(span) === -1) - spans.push(span); + if (spans.indexOf(span) === -1) spans.push(span); return this; }