Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI Visibility] - Find .git folder when using GetFrom method as a fallback #5425

Merged

Conversation

tonyredondo
Copy link
Member

Summary of changes

This PR changes the behavior of GitInfo.GetFrom to have a fallback when the current folder provided doesn't contain git information.

Reason for change

There's a case when we can provide only the current folder from a target project and we need to support looking at the .git folder given that target project folder

Implementation details

We only look for the parent .git folder when we cannot parse the current provided folder.

@tonyredondo tonyredondo self-assigned this Apr 10, 2024
@tonyredondo tonyredondo requested review from a team as code owners April 10, 2024 10:58
{
if (gitDirectory == null)
{
return new GitInfo();
gitInfo = new GitInfo();
Copy link
Member

Choose a reason for hiding this comment

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

Should we not create a new one here? Or if we do want to always, just move gitInfo = new (); to the top of the method, before the if

Suggested change
gitInfo = new GitInfo();
gitInfo = default;

Copy link
Member Author

@tonyredondo tonyredondo Apr 10, 2024

Choose a reason for hiding this comment

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

changed that method in ce8fd1a

tracer/src/Datadog.Trace/Ci/GitInfo.cs Outdated Show resolved Hide resolved
Comment on lines 107 to 108
// Return the partial gitInfo instance with the initial source root
return gitInfo;
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm, that's pretty unusual pattern for a TryGet* method 🤔 I would assume that if TryGetFrom returns false, the output variable is not valid to use. Maybe we should rename it to TryPopulateFrom and pass in the gitInfo as a variable instead? 🤔 Doesn't really matter, this just looks "wrong" to me 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

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

fair, let me change that

Copy link
Member Author

Choose a reason for hiding this comment

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

done in ce8fd1a

@DataDog DataDog deleted a comment from datadog-ddstaging bot Apr 10, 2024
@DataDog DataDog deleted a comment from andrewlock Apr 10, 2024
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Apr 10, 2024

Datadog Report

Branch report: tony/civisibility-find-git-folder-when-using-getfrom
Commit report: ce8fd1a
Test service: dd-trace-dotnet

✅ 0 Failed, 334504 Passed, 1513 Skipped, 42m 10.19s Wall Time

@andrewlock
Copy link
Member

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5425) - mean (73ms)  : 64, 82
     .   : milestone, 73,
    master - mean (72ms)  : 63, 81
     .   : milestone, 72,

    section CallTarget+Inlining+NGEN
    This PR (5425) - mean (1,022ms)  : 1000, 1045
     .   : milestone, 1022,
    master - mean (1,022ms)  : 1004, 1040
     .   : milestone, 1022,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5425) - mean (110ms)  : 106, 114
     .   : milestone, 110,
    master - mean (109ms)  : 106, 112
     .   : milestone, 109,

    section CallTarget+Inlining+NGEN
    This PR (5425) - mean (742ms)  : 727, 757
     .   : milestone, 742,
    master - mean (744ms)  : 730, 758
     .   : milestone, 744,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5425) - mean (94ms)  : 89, 98
     .   : milestone, 94,
    master - mean (93ms)  : 89, 96
     .   : milestone, 93,

    section CallTarget+Inlining+NGEN
    This PR (5425) - mean (697ms)  : 676, 718
     .   : milestone, 697,
    master - mean (698ms)  : 683, 713
     .   : milestone, 698,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5425) - mean (188ms)  : 185, 192
     .   : milestone, 188,
    master - mean (187ms)  : 183, 191
     .   : milestone, 187,

    section CallTarget+Inlining+NGEN
    This PR (5425) - mean (1,106ms)  : 1084, 1127
     .   : milestone, 1106,
    master - mean (1,099ms)  : 1075, 1124
     .   : milestone, 1099,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5425) - mean (275ms)  : 268, 282
     .   : milestone, 275,
    master - mean (270ms)  : 267, 274
     .   : milestone, 270,

    section CallTarget+Inlining+NGEN
    This PR (5425) - mean (898ms)  : 879, 917
     .   : milestone, 898,
    master - mean (889ms)  : 869, 909
     .   : milestone, 889,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (5425) - mean (261ms)  : 258, 265
     .   : milestone, 261,
    master - mean (259ms)  : 252, 266
     .   : milestone, 259,

    section CallTarget+Inlining+NGEN
    This PR (5425) - mean (880ms)  : 857, 902
     .   : milestone, 880,
    master - mean (874ms)  : 858, 891
     .   : milestone, 874,

Loading

@andrewlock
Copy link
Member

Benchmarks Report for tracer 🐌

Benchmarks for #5425 compared to master:

  • All benchmarks have the same speed
  • 1 benchmarks have fewer allocations

The following thresholds were used for comparing the benchmark speeds:

  • Mann–Whitney U test with statistical test for significance of 5%
  • Only results indicating a difference greater than 10% and 0.3 ns are considered.

Allocation changes below 0.5% are ignored.

Benchmark details

Benchmarks.Trace.ActivityBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartStopWithChild net6.0 8.61μs 43.6ns 214ns 0.0262 0.0131 0 7.54 KB
master StartStopWithChild netcoreapp3.1 10.5μs 53.9ns 259ns 0.0264 0.0105 0 7.63 KB
master StartStopWithChild net472 17μs 64.6ns 250ns 1.35 0.376 0.12 8.07 KB
#5425 StartStopWithChild net6.0 8.56μs 46ns 244ns 0.0384 0.0171 0 7.54 KB
#5425 StartStopWithChild netcoreapp3.1 10.4μs 57.5ns 359ns 0.0196 0.0098 0 7.63 KB
#5425 StartStopWithChild net472 16.9μs 55.8ns 209ns 1.36 0.372 0.0931 8.07 KB
Benchmarks.Trace.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 450μs 449ns 1.74μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 622μs 308ns 1.19μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 818μs 611ns 2.37μs 0.406 0 0 3.3 KB
#5425 WriteAndFlushEnrichedTraces net6.0 462μs 170ns 636ns 0 0 0 2.7 KB
#5425 WriteAndFlushEnrichedTraces netcoreapp3.1 628μs 299ns 1.16μs 0 0 0 2.7 KB
#5425 WriteAndFlushEnrichedTraces net472 812μs 576ns 2.16μs 0.406 0 0 3.3 KB
Benchmarks.Trace.AspNetCoreBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendRequest net6.0 168μs 290ns 1.12μs 0.17 0 0 18.38 KB
master SendRequest netcoreapp3.1 192μs 301ns 1.17μs 0.191 0 0 20.54 KB
master SendRequest net472 0.000434ns 0.000182ns 0.000657ns 0 0 0 0 b
#5425 SendRequest net6.0 173μs 252ns 941ns 0.17 0 0 18.38 KB
#5425 SendRequest netcoreapp3.1 196μs 277ns 1.07μs 0.196 0 0 20.54 KB
#5425 SendRequest net472 0.00108ns 0.000421ns 0.00163ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #5425

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1 41.96 KB 41.63 KB -336 B -0.80%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 576μs 1.35μs 5.22μs 0.548 0 0 41.84 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 650μs 943ns 3.53μs 0.331 0 0 41.96 KB
master WriteAndFlushEnrichedTraces net472 871μs 4.33μs 19.4μs 8.08 2.55 0.425 53.21 KB
#5425 WriteAndFlushEnrichedTraces net6.0 537μs 789ns 2.95μs 0.536 0 0 41.69 KB
#5425 WriteAndFlushEnrichedTraces netcoreapp3.1 672μs 1.01μs 3.91μs 0.338 0 0 41.63 KB
#5425 WriteAndFlushEnrichedTraces net472 861μs 3.1μs 12μs 8.28 2.48 0.414 53.24 KB
Benchmarks.Trace.DbCommandBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteNonQuery net6.0 1.13μs 0.664ns 2.48ns 0.0113 0 0 800 B
master ExecuteNonQuery netcoreapp3.1 1.59μs 0.427ns 1.54ns 0.0104 0 0 800 B
master ExecuteNonQuery net472 1.81μs 0.399ns 1.54ns 0.121 0 0 762 B
#5425 ExecuteNonQuery net6.0 1.17μs 0.732ns 2.84ns 0.0115 0 0 800 B
#5425 ExecuteNonQuery netcoreapp3.1 1.5μs 0.872ns 3.38ns 0.0105 0 0 800 B
#5425 ExecuteNonQuery net472 1.83μs 0.875ns 3.03ns 0.121 0 0 762 B
Benchmarks.Trace.ElasticsearchBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master CallElasticsearch net6.0 1.3μs 0.503ns 1.88ns 0.0143 0 0 1.02 KB
master CallElasticsearch netcoreapp3.1 1.6μs 0.481ns 1.8ns 0.0138 0 0 1.02 KB
master CallElasticsearch net472 2.6μs 1.47ns 5.48ns 0.164 0 0 1.04 KB
master CallElasticsearchAsync net6.0 1.46μs 0.873ns 3.27ns 0.0138 0 0 1 KB
master CallElasticsearchAsync netcoreapp3.1 1.68μs 0.739ns 2.86ns 0.0142 0 0 1.07 KB
master CallElasticsearchAsync net472 2.66μs 1.52ns 5.7ns 0.172 0 0 1.09 KB
#5425 CallElasticsearch net6.0 1.42μs 1.04ns 4.01ns 0.0142 0 0 1.02 KB
#5425 CallElasticsearch netcoreapp3.1 1.57μs 2.32ns 9ns 0.0141 0 0 1.02 KB
#5425 CallElasticsearch net472 2.49μs 1.16ns 4.51ns 0.164 0 0 1.04 KB
#5425 CallElasticsearchAsync net6.0 1.39μs 1.63ns 6.29ns 0.014 0 0 1 KB
#5425 CallElasticsearchAsync netcoreapp3.1 1.77μs 0.742ns 2.87ns 0.0148 0 0 1.07 KB
#5425 CallElasticsearchAsync net472 2.68μs 1.43ns 5.53ns 0.172 0 0 1.09 KB
Benchmarks.Trace.GraphQLBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master ExecuteAsync net6.0 1.33μs 1.33ns 5.15ns 0.0128 0 0 944 B
master ExecuteAsync netcoreapp3.1 1.82μs 0.765ns 2.86ns 0.0126 0 0 944 B
master ExecuteAsync net472 1.96μs 0.502ns 1.88ns 0.144 0 0 907 B
#5425 ExecuteAsync net6.0 1.34μs 0.388ns 1.5ns 0.0134 0 0 944 B
#5425 ExecuteAsync netcoreapp3.1 1.67μs 1.1ns 4.28ns 0.0127 0 0 944 B
#5425 ExecuteAsync net472 1.95μs 0.99ns 3.83ns 0.144 0 0 907 B
Benchmarks.Trace.HttpClientBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendAsync net6.0 4.09μs 1.77ns 6.62ns 0.0295 0 0 2.18 KB
master SendAsync netcoreapp3.1 5.05μs 1.98ns 7.4ns 0.0354 0 0 2.71 KB
master SendAsync net472 7.69μs 1.95ns 7.3ns 0.485 0 0 3.07 KB
#5425 SendAsync net6.0 4.14μs 2.17ns 8.4ns 0.0292 0 0 2.18 KB
#5425 SendAsync netcoreapp3.1 5.14μs 0.963ns 3.6ns 0.0359 0 0 2.71 KB
#5425 SendAsync net472 7.63μs 3.03ns 10.9ns 0.486 0 0 3.07 KB
Benchmarks.Trace.ILoggerBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 1.6μs 0.901ns 3.49ns 0.0235 0 0 1.68 KB
master EnrichedLog netcoreapp3.1 2.25μs 1.05ns 3.94ns 0.023 0 0 1.68 KB
master EnrichedLog net472 2.7μs 0.736ns 2.55ns 0.255 0 0 1.61 KB
#5425 EnrichedLog net6.0 1.49μs 0.87ns 3.26ns 0.0238 0 0 1.68 KB
#5425 EnrichedLog netcoreapp3.1 2.17μs 1.1ns 4.11ns 0.0222 0 0 1.68 KB
#5425 EnrichedLog net472 2.58μs 2.11ns 7.91ns 0.255 0 0 1.61 KB
Benchmarks.Trace.Log4netBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 112μs 86.6ns 336ns 0.0565 0 0 4.26 KB
master EnrichedLog netcoreapp3.1 117μs 103ns 398ns 0.0589 0 0 4.26 KB
master EnrichedLog net472 146μs 83ns 311ns 0.66 0.22 0 4.45 KB
#5425 EnrichedLog net6.0 112μs 79ns 296ns 0.0558 0 0 4.26 KB
#5425 EnrichedLog netcoreapp3.1 119μs 279ns 1.08μs 0.0588 0 0 4.26 KB
#5425 EnrichedLog net472 147μs 123ns 475ns 0.662 0.221 0 4.45 KB
Benchmarks.Trace.NLogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 3.02μs 1.18ns 4.56ns 0.0319 0 0 2.24 KB
master EnrichedLog netcoreapp3.1 4.15μs 1.86ns 6.97ns 0.0293 0 0 2.24 KB
master EnrichedLog net472 5.03μs 1.84ns 7.11ns 0.324 0 0 2.05 KB
#5425 EnrichedLog net6.0 3.09μs 0.708ns 2.65ns 0.0309 0 0 2.24 KB
#5425 EnrichedLog netcoreapp3.1 4.23μs 1.69ns 6.1ns 0.0301 0 0 2.24 KB
#5425 EnrichedLog net472 4.86μs 2.62ns 10.1ns 0.325 0 0 2.05 KB
Benchmarks.Trace.RedisBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master SendReceive net6.0 1.51μs 0.833ns 3.12ns 0.0165 0 0 1.19 KB
master SendReceive netcoreapp3.1 1.81μs 0.705ns 2.64ns 0.0163 0 0 1.19 KB
master SendReceive net472 2.29μs 1.22ns 4.56ns 0.19 0 0 1.2 KB
#5425 SendReceive net6.0 1.44μs 0.554ns 2.07ns 0.0166 0 0 1.19 KB
#5425 SendReceive netcoreapp3.1 1.84μs 1.82ns 6.79ns 0.0164 0 0 1.19 KB
#5425 SendReceive net472 2.23μs 1.37ns 5.14ns 0.19 0 0 1.2 KB
Benchmarks.Trace.SerilogBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master EnrichedLog net6.0 2.63μs 0.715ns 2.58ns 0.0224 0 0 1.58 KB
master EnrichedLog netcoreapp3.1 3.97μs 1.31ns 4.92ns 0.022 0 0 1.63 KB
master EnrichedLog net472 4.56μs 2.37ns 9.16ns 0.319 0 0 2.02 KB
#5425 EnrichedLog net6.0 2.81μs 1.24ns 4.82ns 0.0226 0 0 1.58 KB
#5425 EnrichedLog netcoreapp3.1 4.18μs 1.6ns 6ns 0.0207 0 0 1.63 KB
#5425 EnrichedLog net472 4.44μs 2.31ns 8.95ns 0.321 0 0 2.02 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 543ns 0.926ns 3.47ns 0.00789 0 0 568 B
master StartFinishSpan netcoreapp3.1 744ns 1.51ns 5.85ns 0.00758 0 0 568 B
master StartFinishSpan net472 742ns 1.34ns 5.19ns 0.0902 0 0 570 B
master StartFinishScope net6.0 589ns 0.77ns 2.88ns 0.00948 0 0 688 B
master StartFinishScope netcoreapp3.1 832ns 4.19ns 18.7ns 0.00914 0 0 688 B
master StartFinishScope net472 976ns 2.66ns 10.3ns 0.103 0 0 650 B
#5425 StartFinishSpan net6.0 544ns 0.649ns 2.43ns 0.00803 0 0 568 B
#5425 StartFinishSpan netcoreapp3.1 693ns 3.02ns 11.7ns 0.00741 0 0 568 B
#5425 StartFinishSpan net472 797ns 1.46ns 5.66ns 0.0905 0.000393 0 570 B
#5425 StartFinishScope net6.0 592ns 1.13ns 4.37ns 0.00975 0 0 688 B
#5425 StartFinishScope netcoreapp3.1 796ns 1.65ns 6.4ns 0.00913 0 0 688 B
#5425 StartFinishScope net472 961ns 1.64ns 6.34ns 0.103 0 0 650 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 674ns 0.877ns 3.16ns 0.00948 0 0 688 B
master RunOnMethodBegin netcoreapp3.1 957ns 3.5ns 13.6ns 0.00927 0 0 688 B
master RunOnMethodBegin net472 1.05μs 2.65ns 10.3ns 0.103 0 0 650 B
#5425 RunOnMethodBegin net6.0 750ns 1.17ns 4.54ns 0.00962 0 0 688 B
#5425 RunOnMethodBegin netcoreapp3.1 954ns 2.02ns 7.82ns 0.00921 0 0 688 B
#5425 RunOnMethodBegin net472 1.04μs 1.79ns 6.94ns 0.103 0 0 650 B

@tonyredondo tonyredondo merged commit 7b32c30 into master Apr 10, 2024
54 of 57 checks passed
@tonyredondo tonyredondo deleted the tony/civisibility-find-git-folder-when-using-getfrom branch April 10, 2024 13:31
@github-actions github-actions bot added this to the vNext-v2 milestone Apr 10, 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.

3 participants