Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
40 changes: 22 additions & 18 deletions integration-tests/esbuild.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,28 @@ const path = require('path')
const CWD = process.cwd()
const TEST_DIR = path.join(__dirname, 'esbuild')

// eslint-disable-next-line no-console
console.log(`cd ${TEST_DIR}`)
process.chdir(TEST_DIR)
describe('esbuild', () => {
it('works', () => {
// eslint-disable-next-line no-console
console.log(`cd ${TEST_DIR}`)
process.chdir(TEST_DIR)

// eslint-disable-next-line no-console
console.log('npm run build')
chproc.execSync('npm run build')
// eslint-disable-next-line no-console
console.log('npm run build')
chproc.execSync('npm run build')

// eslint-disable-next-line no-console
console.log('npm run built')
try {
chproc.execSync('npm run built', {
timeout: 1000 * 30
// eslint-disable-next-line no-console
console.log('npm run built')
try {
chproc.execSync('npm run built', {
timeout: 1000 * 30
})
} catch (err) {
// eslint-disable-next-line no-console
console.error(err)
process.exit(1)
} finally {
process.chdir(CWD)
}
})
} catch (err) {
// eslint-disable-next-line no-console
console.error(err)
process.exit(1)
} finally {
process.chdir(CWD)
}
})
83 changes: 83 additions & 0 deletions integration-tests/sci-embedding.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
'use strict'

const {
FakeAgent,
spawnProc,
createSandbox,
curlAndAssertMessage
} = require('./helpers')
const path = require('path')
const { assert } = require('chai')
const { SCI_COMMIT_SHA, SCI_REPOSITORY_URL } = require('../packages/dd-trace/src/constants')

const DUMMY_GIT_SHA = '13851f2b092e97acebab1b73f6c0e7818e795b50'
const DUMMY_REPOSITORY_URL = '[email protected]:DataDog/sci_git_example.git'

describe('sci embedding', function () {
let agent
let proc
let sandbox
let cwd

before(async () => {
sandbox = await createSandbox(['express'])
cwd = sandbox.folder
})

after(async () => {
await sandbox.remove()
})

beforeEach(async () => {
agent = await new FakeAgent().start()
})

afterEach(async () => {
proc && proc.kill()
await agent.stop()
})

context('via DD_GIT_* tags', () => {
it('shows in the first span', async () => {
proc = await spawnProc(path.join(cwd, 'sci-embedding/index.js'), {
cwd,
env: {
AGENT_PORT: agent.port,
DD_GIT_REPOSITORY_URL: DUMMY_REPOSITORY_URL,
DD_GIT_COMMIT_SHA: DUMMY_GIT_SHA
},
stdio: 'inherit'
})
return curlAndAssertMessage(agent, proc, ({ payload }) => {
const firstSpan = payload[0][0]
assert.equal(firstSpan.meta[SCI_COMMIT_SHA], DUMMY_GIT_SHA)
assert.equal(firstSpan.meta[SCI_REPOSITORY_URL], DUMMY_REPOSITORY_URL)

const secondSpan = payload[0][1]
assert.notExists(secondSpan.meta[SCI_COMMIT_SHA])
assert.notExists(secondSpan.meta[SCI_REPOSITORY_URL])
})
})
})
context('via DD_TAGS', () => {
it('shows in the first span', async () => {
proc = await spawnProc(path.join(cwd, 'sci-embedding/index.js'), {
cwd,
env: {
AGENT_PORT: agent.port,
DD_TAGS: `git.repository_url:${DUMMY_REPOSITORY_URL},git.commit.sha:${DUMMY_GIT_SHA}`
},
stdio: 'inherit'
})
return curlAndAssertMessage(agent, proc, ({ payload }) => {
const firstSpan = payload[0][0]
assert.equal(firstSpan.meta[SCI_COMMIT_SHA], DUMMY_GIT_SHA)
assert.equal(firstSpan.meta[SCI_REPOSITORY_URL], DUMMY_REPOSITORY_URL)

const secondSpan = payload[0][1]
assert.notExists(secondSpan.meta[SCI_COMMIT_SHA])
assert.notExists(secondSpan.meta[SCI_REPOSITORY_URL])
})
})
})
})
15 changes: 15 additions & 0 deletions integration-tests/sci-embedding/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const tracer = require('dd-trace')
const express = require('express')

tracer.init({ port: process.env.AGENT_PORT })

const app = express()

app.use((req, res) => {
res.end('hello, world\n')
})

const server = app.listen(0, () => {
const port = server.address().port
process.send({ port })
})
19 changes: 19 additions & 0 deletions packages/dd-trace/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const coalesce = require('koalas')
const tagger = require('./tagger')
const { isTrue, isFalse } = require('./util')
const uuid = require('crypto-randomuuid')
const { GIT_REPOSITORY_URL, GIT_COMMIT_SHA } = require('./plugins/util/tags')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like these are expected to be set by users? If that's the case they should probably be moved to ext/tags.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that makes sense. I'll do this in another PR though, because there are multiple files to change and I don't want to make this harder to review


const fromEntries = Object.fromEntries || (entries =>
entries.reduce((obj, [k, v]) => Object.assign(obj, { [k]: v }), {}))
Expand Down Expand Up @@ -393,6 +394,11 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)
true
)

const DD_TRACE_GIT_METADATA_ENABLED = coalesce(
process.env.DD_TRACE_GIT_METADATA_ENABLED,
true
)

const ingestion = options.ingestion || {}
const dogstatsd = coalesce(options.dogstatsd, {})
const sampler = {
Expand Down Expand Up @@ -506,6 +512,19 @@ ken|consumer_?(?:id|key|secret)|sign(?:ed|ature)?|auth(?:entication|orization)?)
this.isGitUploadEnabled = this.isCiVisibility &&
(this.isIntelligentTestRunnerEnabled && !isFalse(DD_CIVISIBILITY_GIT_UPLOAD_ENABLED))

this.isTraceGitMetadataEnabled = isTrue(DD_TRACE_GIT_METADATA_ENABLED)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it consistent, so gitMetadataEnabled.


if (this.isTraceGitMetadataEnabled) {
this.repositoryUrl = coalesce(
process.env.DD_GIT_REPOSITORY_URL,
this.tags[GIT_REPOSITORY_URL]
)
this.commitSHA = coalesce(
process.env.DD_GIT_COMMIT_SHA,
this.tags[GIT_COMMIT_SHA]
)
}

this.stats = {
enabled: isTrue(DD_TRACE_STATS_COMPUTATION_ENABLED)
}
Expand Down
4 changes: 3 additions & 1 deletion packages/dd-trace/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@ module.exports = {
ERROR_MESSAGE: 'error.message',
ERROR_STACK: 'error.stack',
COMPONENT: 'component',
CLIENT_PORT_KEY: 'network.destination.port'
CLIENT_PORT_KEY: 'network.destination.port',
SCI_REPOSITORY_URL: '_dd.git.repository_url',
SCI_COMMIT_SHA: '_dd.git.commit.sha'
}
14 changes: 13 additions & 1 deletion packages/dd-trace/src/span_processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const format = require('./format')
const SpanSampler = require('./span_sampler')

const { SpanStatsProcessor } = require('./span_stats')
const { SCI_COMMIT_SHA, SCI_REPOSITORY_URL } = require('./constants')

const startedSpans = new WeakSet()
const finishedSpans = new WeakSet()
Expand All @@ -25,14 +26,18 @@ class SpanProcessor {
const active = []
const formatted = []
const trace = spanContext._trace
const { flushMinSpans } = this._config
const { flushMinSpans, isTraceGitMetadataEnabled } = this._config
const { started, finished } = trace

if (trace.record === false) return
if (started.length === finished.length || finished.length >= flushMinSpans) {
this._prioritySampler.sample(spanContext)
this._spanSampler.sample(spanContext)

if (isTraceGitMetadataEnabled) {
this.addRepositoryMetadata(started)
}

for (const span of started) {
if (span._duration !== undefined) {
const formattedSpan = format(span)
Expand All @@ -59,6 +64,13 @@ class SpanProcessor {
}
}

addRepositoryMetadata (spans) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like something the formatter should do since it's responsible for knowing how to use the trace from the span. It's also already handling similar other tags.

const { repositoryUrl, commitSHA } = this._config
const firstSpan = spans[0]
firstSpan.setTag(SCI_REPOSITORY_URL, repositoryUrl)
firstSpan.setTag(SCI_COMMIT_SHA, commitSHA)
}

killAll () {
this._killAll = true
}
Expand Down