Skip to content

Commit

Permalink
PROF-8545: Restore eager release of tags, adapt endpoint profiling co…
Browse files Browse the repository at this point in the history
…de (#3759)

* Restores eager release of tags for exported spans in `process_spans.js`.
* Moves tag fetching in `wall.js` from `_updateContext` to `_enter` to compensate for above.
* Fixes some tests that relied on tags being lazily released.
  • Loading branch information
szegedi committed Nov 8, 2023
1 parent 17fa54d commit 0999ce6
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 42 deletions.
45 changes: 29 additions & 16 deletions packages/dd-trace/src/profiling/profilers/wall.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ class NativeWallProfiler {
this._pprof.time.setContext(this._currentContext)
this._lastSpan = undefined
this._lastStartedSpans = undefined
this._lastWebTags = undefined
this._lastSampleCount = 0

beforeCh.subscribe(this._enter)
Expand All @@ -145,10 +146,29 @@ class NativeWallProfiler {
const span = getActiveSpan()
if (span) {
this._lastSpan = span
this._lastStartedSpans = getStartedSpans(span.context())
const startedSpans = getStartedSpans(span.context())
this._lastStartedSpans = startedSpans
if (this._endpointCollectionEnabled) {
let found = false
// Find the first webspan starting from the end:
// There might be several webspans, for example with next.js, http plugin creates a first span
// and then next.js plugin creates a child span, and this child span haves the correct endpoint information.
for (let i = startedSpans.length - 1; i >= 0; i--) {
const tags = getSpanContextTags(startedSpans[i])
if (isWebServerSpan(tags)) {
this._lastWebTags = tags
found = true
break
}
}
if (!found) {
this._lastWebTags = undefined
}
}
} else {
this._lastStartedSpans = undefined
this._lastSpan = undefined
this._lastWebTags = undefined
}
}

Expand All @@ -163,21 +183,11 @@ class NativeWallProfiler {
context.rootSpanId = rootSpan.context().toSpanId()
}
}
if (this._endpointCollectionEnabled) {
const startedSpans = this._lastStartedSpans
// Find the first webspan starting from the end:
// There might be several webspans, for example with next.js, http plugin creates a first span
// and then next.js plugin creates a child span, and this child span haves the correct endpoint information.
for (let i = startedSpans.length - 1; i >= 0; i--) {
const tags = getSpanContextTags(startedSpans[i])
if (isWebServerSpan(tags)) {
context.webTags = tags
// endpoint may not be determined yet, but keep it as fallback
// if tags are not available anymore during serialization
context.endpoint = endpointNameFromTags(tags)
break
}
}
if (this._lastWebTags) {
context.webTags = this._lastWebTags
// endpoint may not be determined yet, but keep it as fallback
// if tags are not available anymore during serialization
context.endpoint = endpointNameFromTags(this._lastWebTags)
}
}

Expand Down Expand Up @@ -224,6 +234,9 @@ class NativeWallProfiler {
beforeCh.unsubscribe(this._enter)
enterCh.unsubscribe(this._enter)
this._profilerState = undefined
this._lastSpan = undefined
this._lastStartedSpans = undefined
this._lastWebTags = undefined
}

this._started = false
Expand Down
4 changes: 4 additions & 0 deletions packages/dd-trace/src/span_processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ class SpanProcessor {
}
}

for (const span of trace.finished) {
span.context()._tags = {}
}

trace.started = active
trace.finished = []
}
Expand Down
41 changes: 15 additions & 26 deletions packages/dd-trace/test/tracer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const { DD_MAJOR } = require('../../../version')
const SPAN_TYPE = tags.SPAN_TYPE
const RESOURCE_NAME = tags.RESOURCE_NAME
const SERVICE_NAME = tags.SERVICE_NAME
const EXPORT_SERVICE_NAME = 'service'
const BASE_SERVICE = tags.BASE_SERVICE

const describeOrphanable = DD_MAJOR < 4 ? describe : describe.skip
Expand All @@ -39,6 +40,7 @@ describe('Tracer', () => {

tracer = new Tracer(config)
tracer._exporter.setUrl = sinon.stub()
tracer._exporter.export = sinon.stub()
tracer._prioritySampler.configure = sinon.stub()
})

Expand Down Expand Up @@ -89,38 +91,25 @@ describe('Tracer', () => {
})

describe('_dd.base_service', () => {
let genSpan

it('should be set when tracer.trace service mismatches configured service', () => {
tracer.trace('name', { service: 'custom' }, span => {
genSpan = span
})
const tags = genSpan.context()._tags
expect(genSpan).to.be.instanceof(Span)
expect(tags).to.include({
[BASE_SERVICE]: 'service',
[SERVICE_NAME]: 'custom'
})
tracer.trace('name', { service: 'custom' }, () => {})
const trace = tracer._exporter.export.getCall(0).args[0][0]
expect(trace).to.have.property(EXPORT_SERVICE_NAME, 'custom')
expect(trace.meta).to.have.property(BASE_SERVICE, 'service')
})

it('should not be set when tracer.trace service is not supplied', () => {
tracer.trace('name', {}, span => {
genSpan = span
})
const tags = genSpan.context()._tags
expect(genSpan).to.be.instanceof(Span)
expect(tags).to.have.property(SERVICE_NAME, 'service')
expect(tags).to.not.have.property(BASE_SERVICE)
tracer.trace('name', {}, () => {})
const trace = tracer._exporter.export.getCall(0).args[0][0]
expect(trace).to.have.property(EXPORT_SERVICE_NAME, 'service')
expect(trace.meta).to.not.have.property(BASE_SERVICE)
})

it('should be set when tracer.trace service matched configured service', () => {
tracer.trace('name', { service: 'service' }, span => {
genSpan = span
})
const tags = genSpan.context()._tags
expect(genSpan).to.be.instanceof(Span)
expect(tags).to.have.property(SERVICE_NAME, 'service')
expect(tags).to.not.have.property(BASE_SERVICE)
it('should not be set when tracer.trace service matched configured service', () => {
tracer.trace('name', { service: 'service' }, () => {})
const trace = tracer._exporter.export.getCall(0).args[0][0]
expect(trace).to.have.property(EXPORT_SERVICE_NAME, 'service')
expect(trace.meta).to.not.have.property(BASE_SERVICE)
})
})

Expand Down

0 comments on commit 0999ce6

Please sign in to comment.