Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue #9165 express router entry duplicated #84

Merged
merged 4 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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