Skip to content

Commit 1407421

Browse files
authored
chore: key the cache on DD_TRACE_GIT_METADATA_ENABLED (#8395)
A single \`let cached\` slot pinned the first call's result for the lifetime of the process. Once a fresh \`Config\` (different \`DD_GIT_*\` env, different \`DD_GIT_PROPERTIES_FILE\`, different \`DD_TRACE_GIT_METADATA_ENABLED\` flip at runtime) reached \`getGitMetadata\`, it received the previous instance's metadata and the new options went unread. Two slots — one for the enabled branch's resolved metadata, one for the disabled branch's empty result — match the only flag whose runtime value picks a different resolution path and stay O(1).
1 parent b92bdc1 commit 1407421

2 files changed

Lines changed: 39 additions & 8 deletions

File tree

packages/dd-trace/src/git_metadata.js

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,21 @@ const {
1212
resolveGitHeadSHA,
1313
} = require('./config/git_properties')
1414

15-
/** @type {{ commitSHA: string | undefined, repositoryUrl: string | undefined } | undefined} */
16-
let cached
15+
/**
16+
* @typedef {{ commitSHA: string | undefined, repositoryUrl: string | undefined }} GitMetadata
17+
* @type {{ enabled?: GitMetadata, disabled?: GitMetadata }}
18+
*/
19+
const cache = {}
1720

1821
/**
1922
* @param {import('./config/config-types').ConfigProperties} config
2023
*/
2124
function getGitMetadata (config) {
22-
if (cached) return cached
23-
2425
if (!config.DD_TRACE_GIT_METADATA_ENABLED) {
25-
cached = { commitSHA: undefined, repositoryUrl: undefined }
26-
return cached
26+
cache.disabled ??= { commitSHA: undefined, repositoryUrl: undefined }
27+
return cache.disabled
2728
}
29+
if (cache.enabled) return cache.enabled
2830

2931
let repositoryUrl = removeUserSensitiveInfo(config.DD_GIT_REPOSITORY_URL ?? config.tags[GIT_REPOSITORY_URL])
3032
let commitSHA = config.DD_GIT_COMMIT_SHA ?? config.tags[GIT_COMMIT_SHA]
@@ -59,8 +61,8 @@ function getGitMetadata (config) {
5961

6062
commitSHA ??= resolveGitHeadSHA(gitFolderPath)
6163

62-
cached = { commitSHA, repositoryUrl }
63-
return cached
64+
cache.enabled = { commitSHA, repositoryUrl }
65+
return cache.enabled
6466
}
6567

6668
module.exports = getGitMetadata

packages/dd-trace/test/git_metadata.spec.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,35 @@ describe('git metadata', () => {
183183
assert.strictEqual(getGitMetadata(config), getGitMetadata(config))
184184
})
185185

186+
// Regression: a single shared cache slot returned the disabled-branch
187+
// empty result for any subsequent call once the first call had run with
188+
// `DD_TRACE_GIT_METADATA_ENABLED=false`, even when the next caller passed
189+
// a fresh `Config` with the flag flipped on and the env populated.
190+
it('caches the disabled-branch result independently from the enabled-branch result', () => {
191+
const getGitMetadata = proxyquire.noPreserveCache()('../src/git_metadata', {})
192+
193+
const disabledConfig = { DD_TRACE_GIT_METADATA_ENABLED: false, tags: {} }
194+
const enabledConfig = {
195+
DD_TRACE_GIT_METADATA_ENABLED: true,
196+
DD_GIT_COMMIT_SHA: DUMMY_COMMIT_SHA,
197+
DD_GIT_REPOSITORY_URL: DUMMY_REPOSITORY_URL,
198+
tags: {},
199+
}
200+
201+
assert.deepStrictEqual(getGitMetadata(disabledConfig), {
202+
commitSHA: undefined,
203+
repositoryUrl: undefined,
204+
})
205+
assert.deepStrictEqual(getGitMetadata(enabledConfig), {
206+
commitSHA: DUMMY_COMMIT_SHA,
207+
repositoryUrl: DUMMY_REPOSITORY_URL,
208+
})
209+
assert.deepStrictEqual(getGitMetadata({ DD_TRACE_GIT_METADATA_ENABLED: false, tags: {} }), {
210+
commitSHA: undefined,
211+
repositoryUrl: undefined,
212+
})
213+
})
214+
186215
it('returns undefined when no git source is configured and cwd has no git', () => {
187216
const originalCwd = process.cwd()
188217
const tempDir = fs.mkdtempSync(path.join(TMP_DIR, 'dd-trace-no-git-'))

0 commit comments

Comments
 (0)