Skip to content

Conversation

olivwalsh
Copy link
Contributor

What does this PR do?

Read from .git folder for Git metadata if git.commit.sha and git.repository_url were not found in the environment variables nor in the git.properties file.

Motivation

Enable users copying .git/ files to their deployment servers to have their traces tagged with Git metadata to leverage source code related features.

Copy link

github-actions bot commented Sep 24, 2025

Overall package size

Self size: 12.49 MB
Deduped: 112.58 MB
No deduping: 112.98 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 10.2.1 | 20.64 MB | 20.65 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.10.0 | 9.91 MB | 10.3 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.4 | 2.95 MB | 5.6 MB | | @datadog/wasm-js-rewriter | 4.0.1 | 2.85 MB | 3.58 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api | 1.9.0 | 1.22 MB | 1.22 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.14.4 | 123.18 kB | 851.76 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 7.0.5 | 63.38 kB | 63.38 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.3 | 23.74 kB | 23.74 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | mutexify | 1.4.0 | 5.71 kB | 8.74 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

Copy link

codecov bot commented Sep 24, 2025

Codecov Report

❌ Patch coverage is 98.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.93%. Comparing base (bae4e64) to head (f6d1a9a).

Files with missing lines Patch % Lines
packages/dd-trace/src/git_properties.js 98.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6517      +/-   ##
==========================================
+ Coverage   83.90%   83.93%   +0.03%     
==========================================
  Files         485      485              
  Lines       20337    20406      +69     
==========================================
+ Hits        17063    17128      +65     
- Misses       3274     3278       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@olivwalsh olivwalsh changed the title SCI embedding = read Git metadata from .git folder SCI embedding - read Git metadata from .git folder Sep 24, 2025
@pr-commenter
Copy link

pr-commenter bot commented Sep 24, 2025

Benchmarks

Benchmark execution time: 2025-10-01 07:38:13

Comparing candidate commit f6d1a9a in PR branch olivia.walsh/handle-reading-git-folder with baseline commit bae4e64 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1604 metrics, 66 unstable metrics.

This comment has been minimized.

Comment on lines 2440 to 2446
it('does not crash if git.properties is not available', () => {
process.env.DD_GIT_PROPERTIES_FILE = '/does/not/exist'
const config = new Config({})
expect(config).to.have.property('commitSHA', undefined)
expect(config).to.have.property('repositoryUrl', undefined)
expect(() => {
const config = new Config({})
expect(config).to.be.an('object')
}).to.not.throw()
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modified this test as we only want to test that applying the config does not crash if the git.properties file was not found.
Also with the changes introduced in this PR, if the git.properties file is not found, Git tagging might still be applied if information is present in the .git folder.

@olivwalsh olivwalsh marked this pull request as ready for review September 26, 2025 13:00
@olivwalsh olivwalsh requested review from a team as code owners September 26, 2025 13:00
@olivwalsh olivwalsh requested review from khanayan123 and removed request for a team September 26, 2025 13:00
Copy link
Collaborator

@watson watson left a comment

Choose a reason for hiding this comment

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

Nice work! I found a few edge-cases when reading .git/config and .git/HEAD. I've added comments on how to potentially deal with them 😃

@olivwalsh olivwalsh requested a review from watson September 30, 2025 16:19
Copy link
Collaborator

@watson watson left a comment

Choose a reason for hiding this comment

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

Sorry for the delayed review, I just have a tiny change request for performance reasons, but other than that everything looks amazing 🤩

const headContent = gitHeadContent.trim()

// Handle detached head case
if (isValidCommitSHA(headContent)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you call test on the regex, it handles everything for you that's done by isValidCommitSHA + it doesn't allocate a match object in memory, so it's more performant as well:

-    if (isValidCommitSHA(headContent)) {
+    if (commitSHARegex.test(headContent)) {

If you do the same below for the other calls to isValidCommitSHA functions, you can just get rid of the function alltogether :)

Comment on lines +80 to +82
if (gitRefMatch?.length > 1) {
return gitRefMatch[1]
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: This isn't important, but just a small optimization that you can use if you want 😊

Suggested change
if (gitRefMatch?.length > 1) {
return gitRefMatch[1]
}
return gitRefMatch?.[1]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants