Skip to content

Commit

Permalink
Fix issue #9165 express router entry duplicated (#84)
Browse files Browse the repository at this point in the history
  • Loading branch information
tom-pytel authored Jun 7, 2022
1 parent 92fe2ad commit d26f01b
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 38 deletions.
2 changes: 1 addition & 1 deletion src/plugins/ExpressPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion src/trace/context/Context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 4 additions & 6 deletions src/trace/context/DummyContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);

Expand Down
53 changes: 23 additions & 30 deletions src/trace/context/SpanContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,34 +37,31 @@ 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;
}

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];
}
Expand All @@ -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,
Expand All @@ -100,28 +98,30 @@ 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', {
parent,
});
}

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;
Expand All @@ -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', {
Expand All @@ -141,22 +140,18 @@ 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;
}

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', {
Expand All @@ -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;
}
Expand Down

0 comments on commit d26f01b

Please sign in to comment.