Skip to content

Conversation

@juan-fernandez
Copy link
Collaborator

@juan-fernandez juan-fernandez commented May 8, 2023

What does this PR do?

  • Add DD_TRACE_GIT_METADATA_ENABLED config env var, with a default of true
  • Attempt to read DD_GIT_REPOSITORY_URL and DD_GIT_COMMIT_SHA env vars.
  • Attempt to read git.repository_url and git.commit.sha from DD_TAGS.
  • If they're present, set these values as tags to the first span in a trace.

Motivation

Allow some products to provide source code related features.

Additional Notes

Unrelated fix: warp esbuild.spec.js test in a describe.

@github-actions
Copy link

github-actions bot commented May 8, 2023

Overall package size

Self size: 4.11 MB
Deduped: 58.24 MB
No deduping: 58.28 MB

Dependency sizes

name version self size total size
@datadog/pprof 2.2.1 14.24 MB 15.12 MB
@datadog/native-iast-taint-tracking 1.4.1 14.85 MB 14.86 MB
@datadog/native-appsec 3.1.0 13.31 MB 13.32 MB
protobufjs 7.1.2 2.76 MB 6.55 MB
@datadog/native-iast-rewriter 2.0.1 2.09 MB 2.1 MB
@datadog/native-metrics 2.0.0 898.77 kB 1.3 MB
opentracing 0.14.7 194.81 kB 194.81 kB
semver 7.3.8 88.2 kB 118.6 kB
@datadog/sketches-js 2.1.0 109.9 kB 109.9 kB
lodash.sortby 4.7.0 75.76 kB 75.76 kB
lru-cache 7.14.0 74.95 kB 74.95 kB
ipaddr.js 2.0.1 59.52 kB 59.52 kB
ignore 5.2.0 48.87 kB 48.87 kB
import-in-the-middle 1.3.5 34.34 kB 38.81 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
retry 0.10.1 27.44 kB 27.44 kB
lodash.uniq 4.5.0 25.01 kB 25.01 kB
limiter 1.1.5 23.17 kB 23.17 kB
lodash.kebabcase 4.1.1 17.75 kB 17.75 kB
lodash.pick 4.4.0 16.33 kB 16.33 kB
node-abort-controller 3.0.1 14.33 kB 14.33 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 kB
diagnostics_channel 1.1.0 7.07 kB 7.07 kB
path-to-regexp 0.1.7 6.78 kB 6.78 kB
koalas 1.0.2 6.47 kB 6.47 kB
methods 1.1.2 5.29 kB 5.29 kB
module-details-from-path 1.0.3 4.47 kB 4.47 kB

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

@codecov
Copy link

codecov bot commented May 8, 2023

Codecov Report

Merging #3118 (4490619) into master (c42c3df) will decrease coverage by 2.53%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3118      +/-   ##
==========================================
- Coverage   87.33%   84.81%   -2.53%     
==========================================
  Files         325      275      -50     
  Lines       11830    10587    -1243     
  Branches       33       33              
==========================================
- Hits        10332     8979    -1353     
- Misses       1498     1608     +110     
Impacted Files Coverage Δ
packages/dd-trace/src/constants.js 100.00% <ø> (ø)
packages/dd-trace/src/config.js 98.89% <100.00%> (+0.03%) ⬆️
packages/dd-trace/src/git_metadata_tagger.js 100.00% <100.00%> (ø)
packages/dd-trace/src/span_processor.js 50.00% <100.00%> (+1.89%) ⬆️

... and 63 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@juan-fernandez juan-fernandez changed the title SCI Embedding of git metadata SCI Embedding of git metadata - part 1 May 8, 2023
@juan-fernandez juan-fernandez changed the title SCI Embedding of git metadata - part 1 SCI Embedding of git metadata May 8, 2023
@juan-fernandez juan-fernandez marked this pull request as ready for review May 8, 2023 10:35
@juan-fernandez juan-fernandez requested a review from a team as a code owner May 8, 2023 10:35
@juan-fernandez juan-fernandez requested a review from sashacmc May 8, 2023 10:36
@pr-commenter
Copy link

pr-commenter bot commented May 8, 2023

Benchmarks

Comparing candidate commit 4490619 in PR branch juan-fernandez/sci-embedding with baseline commit c42c3df in branch master.

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

}
}

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 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

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.

@juan-fernandez juan-fernandez force-pushed the juan-fernandez/sci-embedding branch from 03302f1 to 8201501 Compare May 10, 2023 15:41

tagGitMetadata (spanContext) {
if (this._config.gitMetadataEnabled) {
spanContext._trace.tags[SCI_COMMIT_SHA] = this._config.commitSHA
Copy link
Collaborator Author

@juan-fernandez juan-fernandez May 10, 2023

Choose a reason for hiding this comment

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

these trace tags are only added to the local root in format.js via extractChunkTags

@juan-fernandez juan-fernandez requested a review from rochdev May 10, 2023 15:43
Copy link

@sashacmc sashacmc left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@juan-fernandez juan-fernandez merged commit 4e7da80 into master May 11, 2023
@juan-fernandez juan-fernandez deleted the juan-fernandez/sci-embedding branch May 11, 2023 07:39
rochdev pushed a commit that referenced this pull request May 11, 2023
rochdev pushed a commit that referenced this pull request May 11, 2023
rochdev pushed a commit that referenced this pull request May 12, 2023
rochdev pushed a commit that referenced this pull request May 12, 2023
thedavl pushed a commit that referenced this pull request May 30, 2023
@khanayan123 khanayan123 mentioned this pull request Jan 2, 2024
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.

4 participants