Skip to content

Refactor log directory resolution into smaller methods, add tests#7255

Merged
lucaspimentel merged 10 commits into
masterfrom
lpimentel/refactor-log-directory
Mar 17, 2026
Merged

Refactor log directory resolution into smaller methods, add tests#7255
lucaspimentel merged 10 commits into
masterfrom
lpimentel/refactor-log-directory

Conversation

@lucaspimentel

@lucaspimentel lucaspimentel commented Jul 21, 2025

Copy link
Copy Markdown
Member

Summary of changes

Refactor DatadogLoggingFactory log directory resolution logic into smaller, testable methods and add unit tests.

Reason for change

The log directory resolution logic was a single large method mixing concerns:

  • config reading
  • default directory lookup
  • directory creation 😦
  • fallback logic

Breaking it apart makes each piece independently testable and easier to understand.

Implementation details

  • Extract GetDefaultLogDirectory() — determines the default log directory based on platform and Azure environment
  • Extract GetProgramDataDirectory() — resolves the ProgramData path on Windows (with Nano Server fallbacks)
  • Extract TryCreateLogDirectory() — attempts to create a log directory, returning success/failure
  • Make extracted methods internal with [TestingAndPrivateOnly] for testability

Simplify GetLogDirectory() to orchestrate the above methods with clear fallback chain:

  1. DD_TRACE_LOG_DIRECTORY env var
  2. DD_TRACE_LOG_PATH (deprecated, directory extracted from file path)
  3. GetDefaultLogDirectory() (platform/Azure-aware default)
  4. Path.GetTempPath() (last resort)

Test coverage

Flattened nested test classes (FileLoggingConfiguration, RedactedLogConfiguration, SinkConfiguration) into top-level methods with consistent MethodUnderTest_Scenario_ExpectedBehavior naming:

Old (nested class.method) New (flat method)
FileLoggingConfiguration.UsesLogDirectoryWhenItExists GetConfiguration_WithLogDirectory_UsesLogDirectory
FileLoggingConfiguration.UsesObsoleteLogDirectoryWhenAvailable GetConfiguration_WithObsoleteLogPath_UsesObsoleteLogDirectory
FileLoggingConfiguration.UsesEnvironmentFallBackWhenBothNull GetConfiguration_WithNoLogDirectorySettings_UsesEnvironmentFallback
FileLoggingConfiguration.CreatesLogDirectoryWhenItDoesntExist GetConfiguration_WithNonExistentLogDirectory_CreatesDirectory
RedactedLogConfiguration.WhenNoOrInvalidConfiguration_TelemetryLogsEnabled GetConfiguration_WithNoOrInvalidTelemetryLogsSetting_EnablesErrorLogging
RedactedLogConfiguration.WhenDisabled_TelemetryLogsDisabled GetConfiguration_WithTelemetryLogsDisabled_DisablesErrorLogging
SinkConfiguration.WhenNoSinksProvided_UsesFileSink GetConfiguration_WithNoSinksSetting_UsesFileSink
SinkConfiguration.WhenFileSinkIsIncluded_UsesFileSink GetConfiguration_WithFileSinkIncluded_UsesFileSink
SinkConfiguration.WhenFileSinkIsNotIncluded_DoesNotUseFileSink GetConfiguration_WithFileSinkNotIncluded_DoesNotUseFileSink
SinkConfiguration.WhenConsoleSinkIsIncluded_UsesConsoleSink GetConfiguration_WithConsoleSinkIncluded_UsesConsoleSink
SinkConfiguration.WhenConsoleSinkIsNotIncluded_DoesNotUseConsoleSink GetConfiguration_WithConsoleSinkNotIncluded_DoesNotUseConsoleSink

Additional tests for the newly extracted methods:

  • GetDefaultLogDirectory_* — verifies default paths on Windows/Linux, and Azure App Services detection
  • GetProgramDataDirectory_* — verifies ProgramData resolution on Windows
  • TryCreateLogDirectory_* — verifies directory creation success, existing directory, invalid path, and nested path scenarios

Other details

"I refactored a method into smaller methods. Now I have more methods to refactor into even smaller methods." — Claude 🤖

@lucaspimentel lucaspimentel added the type:cleanup Minor code clean up label Jul 21, 2025
@github-actions github-actions Bot added the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label Jul 21, 2025
@lucaspimentel
lucaspimentel force-pushed the lpimentel/refactor-log-directory branch from 8d547a5 to dccf49d Compare August 19, 2025 20:30
@dd-trace-dotnet-ci-bot

dd-trace-dotnet-ci-bot Bot commented Aug 19, 2025

Copy link
Copy Markdown

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing This PR (7255) and master.

✅ No regressions detected - check the details below

Full Metrics Comparison

FakeDbCommand

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration75.20 ± (75.23 - 75.58) ms73.60 ± (73.53 - 73.86) ms-2.1%
.NET Framework 4.8 - Bailout
duration79.80 ± (79.73 - 80.11) ms77.90 ± (77.78 - 78.14) ms-2.4%
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1102.56 ± (1102.61 - 1108.70) ms1081.79 ± (1082.58 - 1088.56) ms-1.9%
.NET Core 3.1 - Baseline
process.internal_duration_ms23.41 ± (23.36 - 23.47) ms22.77 ± (22.73 - 22.81) ms-2.7%
process.time_to_main_ms88.61 ± (88.42 - 88.81) ms86.30 ± (86.10 - 86.49) ms-2.6%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.91 ± (10.90 - 10.91) MB10.91 ± (10.91 - 10.92) MB+0.1%✅⬆️
runtime.dotnet.threads.count12 ± (12 - 12)12 ± (12 - 12)+0.0%
.NET Core 3.1 - Bailout
process.internal_duration_ms23.18 ± (23.13 - 23.23) ms22.70 ± (22.66 - 22.74) ms-2.1%
process.time_to_main_ms89.71 ± (89.49 - 89.94) ms87.40 ± (87.20 - 87.60) ms-2.6%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.94 ± (10.93 - 10.94) MB10.94 ± (10.94 - 10.95) MB+0.0%✅⬆️
runtime.dotnet.threads.count13 ± (13 - 13)13 ± (13 - 13)+0.0%
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms227.75 ± (226.58 - 228.92) ms225.65 ± (224.12 - 227.19) ms-0.9%
process.time_to_main_ms504.40 ± (503.40 - 505.40) ms494.98 ± (494.03 - 495.93) ms-1.9%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed47.18 ± (47.16 - 47.20) MB47.23 ± (47.21 - 47.26) MB+0.1%✅⬆️
runtime.dotnet.threads.count28 ± (28 - 28)28 ± (28 - 28)+0.1%✅⬆️
.NET 6 - Baseline
process.internal_duration_ms21.92 ± (21.87 - 21.97) ms21.49 ± (21.45 - 21.53) ms-2.0%
process.time_to_main_ms76.31 ± (76.14 - 76.48) ms74.49 ± (74.31 - 74.67) ms-2.4%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.63 ± (10.62 - 10.63) MB10.64 ± (10.64 - 10.64) MB+0.1%✅⬆️
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%
.NET 6 - Bailout
process.internal_duration_ms21.85 ± (21.80 - 21.90) ms21.50 ± (21.46 - 21.54) ms-1.6%
process.time_to_main_ms77.49 ± (77.27 - 77.71) ms75.50 ± (75.33 - 75.67) ms-2.6%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed10.72 ± (10.71 - 10.72) MB10.75 ± (10.74 - 10.75) MB+0.3%✅⬆️
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms211.24 ± (209.73 - 212.74) ms206.81 ± (205.27 - 208.35) ms-2.1%
process.time_to_main_ms501.60 ± (500.84 - 502.36) ms492.80 ± (491.99 - 493.61) ms-1.8%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed49.21 ± (49.18 - 49.24) MB49.22 ± (49.18 - 49.27) MB+0.0%✅⬆️
runtime.dotnet.threads.count29 ± (29 - 29)29 ± (29 - 29)-0.1%
.NET 8 - Baseline
process.internal_duration_ms20.07 ± (20.02 - 20.11) ms19.77 ± (19.72 - 19.81) ms-1.5%
process.time_to_main_ms76.05 ± (75.86 - 76.25) ms73.89 ± (73.72 - 74.06) ms-2.8%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed7.64 ± (7.64 - 7.65) MB7.69 ± (7.68 - 7.70) MB+0.6%✅⬆️
runtime.dotnet.threads.count10 ± (10 - 10)10 ± (10 - 10)+0.0%
.NET 8 - Bailout
process.internal_duration_ms20.22 ± (20.18 - 20.27) ms19.65 ± (19.61 - 19.69) ms-2.8%
process.time_to_main_ms77.46 ± (77.25 - 77.66) ms75.05 ± (74.85 - 75.24) ms-3.1%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed7.72 ± (7.71 - 7.73) MB7.78 ± (7.76 - 7.79) MB+0.8%✅⬆️
runtime.dotnet.threads.count11 ± (11 - 11)11 ± (11 - 11)+0.0%
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms169.29 ± (168.26 - 170.31) ms164.91 ± (164.12 - 165.70) ms-2.6%
process.time_to_main_ms485.18 ± (484.38 - 485.98) ms476.45 ± (475.71 - 477.19) ms-1.8%
runtime.dotnet.exceptions.count0 ± (0 - 0)0 ± (0 - 0)+0.0%
runtime.dotnet.mem.committed36.51 ± (36.49 - 36.52) MB36.56 ± (36.55 - 36.58) MB+0.2%✅⬆️
runtime.dotnet.threads.count28 ± (28 - 28)28 ± (28 - 28)+0.0%

HttpMessageHandler

Metric Master (Mean ± 95% CI) Current (Mean ± 95% CI) Change Status
.NET Framework 4.8 - Baseline
duration194.01 ± (193.97 - 194.65) ms193.26 ± (193.12 - 193.85) ms-0.4%
.NET Framework 4.8 - Bailout
duration197.80 ± (197.68 - 198.42) ms196.23 ± (196.04 - 196.66) ms-0.8%
.NET Framework 4.8 - CallTarget+Inlining+NGEN
duration1148.41 ± (1149.16 - 1155.66) ms1147.81 ± (1147.18 - 1153.94) ms-0.1%
.NET Core 3.1 - Baseline
process.internal_duration_ms189.20 ± (188.87 - 189.52) ms188.05 ± (187.69 - 188.40) ms-0.6%
process.time_to_main_ms81.15 ± (80.89 - 81.41) ms81.05 ± (80.79 - 81.30) ms-0.1%
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed16.12 ± (16.10 - 16.14) MB16.07 ± (16.05 - 16.10) MB-0.3%
runtime.dotnet.threads.count20 ± (19 - 20)20 ± (20 - 20)+0.9%✅⬆️
.NET Core 3.1 - Bailout
process.internal_duration_ms187.80 ± (187.46 - 188.15) ms187.40 ± (187.06 - 187.75) ms-0.2%
process.time_to_main_ms82.21 ± (82.08 - 82.34) ms82.05 ± (81.89 - 82.20) ms-0.2%
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed16.14 ± (16.11 - 16.18) MB16.23 ± (16.20 - 16.27) MB+0.6%✅⬆️
runtime.dotnet.threads.count21 ± (21 - 21)21 ± (21 - 21)-0.6%
.NET Core 3.1 - CallTarget+Inlining+NGEN
process.internal_duration_ms396.67 ± (394.88 - 398.46) ms395.43 ± (393.49 - 397.38) ms-0.3%
process.time_to_main_ms476.27 ± (475.67 - 476.88) ms473.24 ± (472.70 - 473.78) ms-0.6%
runtime.dotnet.exceptions.count3 ± (3 - 3)3 ± (3 - 3)+0.0%
runtime.dotnet.mem.committed57.91 ± (57.77 - 58.05) MB57.81 ± (57.65 - 57.97) MB-0.2%
runtime.dotnet.threads.count30 ± (30 - 30)30 ± (29 - 30)-0.5%
.NET 6 - Baseline
process.internal_duration_ms192.79 ± (192.40 - 193.18) ms192.72 ± (192.34 - 193.10) ms-0.0%
process.time_to_main_ms70.51 ± (70.34 - 70.68) ms70.20 ± (70.00 - 70.40) ms-0.4%
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed16.22 ± (16.10 - 16.34) MB16.42 ± (16.39 - 16.45) MB+1.2%✅⬆️
runtime.dotnet.threads.count19 ± (19 - 19)19 ± (19 - 19)+1.3%✅⬆️
.NET 6 - Bailout
process.internal_duration_ms191.54 ± (191.21 - 191.87) ms190.87 ± (190.57 - 191.17) ms-0.4%
process.time_to_main_ms71.34 ± (71.22 - 71.45) ms70.83 ± (70.74 - 70.93) ms-0.7%
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed16.13 ± (15.98 - 16.27) MB16.15 ± (16.01 - 16.30) MB+0.2%✅⬆️
runtime.dotnet.threads.count20 ± (19 - 20)20 ± (19 - 20)+0.8%✅⬆️
.NET 6 - CallTarget+Inlining+NGEN
process.internal_duration_ms431.80 ± (430.17 - 433.42) ms427.45 ± (425.67 - 429.22) ms-1.0%
process.time_to_main_ms479.94 ± (479.08 - 480.81) ms476.70 ± (475.85 - 477.54) ms-0.7%
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed60.32 ± (60.25 - 60.39) MB60.41 ± (60.36 - 60.46) MB+0.1%✅⬆️
runtime.dotnet.threads.count30 ± (30 - 30)30 ± (30 - 31)+0.2%✅⬆️
.NET 8 - Baseline
process.internal_duration_ms189.85 ± (189.51 - 190.19) ms190.42 ± (190.00 - 190.84) ms+0.3%✅⬆️
process.time_to_main_ms69.77 ± (69.59 - 69.95) ms69.90 ± (69.67 - 70.12) ms+0.2%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed11.82 ± (11.79 - 11.85) MB11.75 ± (11.72 - 11.77) MB-0.6%
runtime.dotnet.threads.count18 ± (18 - 18)18 ± (18 - 18)-0.3%
.NET 8 - Bailout
process.internal_duration_ms189.32 ± (188.95 - 189.69) ms189.15 ± (188.90 - 189.39) ms-0.1%
process.time_to_main_ms70.61 ± (70.50 - 70.73) ms70.55 ± (70.45 - 70.65) ms-0.1%
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed11.82 ± (11.78 - 11.85) MB11.74 ± (11.67 - 11.82) MB-0.6%
runtime.dotnet.threads.count19 ± (19 - 19)19 ± (18 - 19)-3.1%
.NET 8 - CallTarget+Inlining+NGEN
process.internal_duration_ms354.37 ± (352.79 - 355.94) ms353.32 ± (352.15 - 354.48) ms-0.3%
process.time_to_main_ms451.88 ± (451.24 - 452.52) ms452.80 ± (452.22 - 453.38) ms+0.2%✅⬆️
runtime.dotnet.exceptions.count4 ± (4 - 4)4 ± (4 - 4)+0.0%
runtime.dotnet.mem.committed48.40 ± (48.35 - 48.44) MB48.45 ± (48.41 - 48.48) MB+0.1%✅⬆️
runtime.dotnet.threads.count30 ± (30 - 30)30 ± (30 - 30)+0.1%✅⬆️
Comparison explanation

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 highlighted 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).

Duration charts
FakeDbCommand (.NET Framework 4.8)
gantt
    title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7255) - mean (74ms)  : 71, 76
    master - mean (75ms)  : 73, 78

    section Bailout
    This PR (7255) - mean (78ms)  : 76, 80
    master - mean (80ms)  : 77, 82

    section CallTarget+Inlining+NGEN
    This PR (7255) - mean (1,086ms)  : 1042, 1129
    master - mean (1,106ms)  : 1061, 1150

Loading
FakeDbCommand (.NET Core 3.1)
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7255) - mean (116ms)  : 112, 120
    master - mean (119ms)  : 115, 123

    section Bailout
    This PR (7255) - mean (117ms)  : 115, 120
    master - mean (120ms)  : 117, 123

    section CallTarget+Inlining+NGEN
    This PR (7255) - mean (756ms)  : 736, 777
    master - mean (769ms)  : 738, 800

Loading
FakeDbCommand (.NET 6)
gantt
    title Execution time (ms) FakeDbCommand (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7255) - mean (102ms)  : 99, 106
    master - mean (105ms)  : 102, 108

    section Bailout
    This PR (7255) - mean (104ms)  : 102, 106
    master - mean (106ms)  : 103, 108

    section CallTarget+Inlining+NGEN
    This PR (7255) - mean (738ms)  : 702, 773
    master - mean (749ms)  : 716, 782

Loading
FakeDbCommand (.NET 8)
gantt
    title Execution time (ms) FakeDbCommand (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7255) - mean (102ms)  : 99, 104
    master - mean (104ms)  : 101, 108

    section Bailout
    This PR (7255) - mean (103ms)  : 100, 105
    master - mean (106ms)  : 102, 109

    section CallTarget+Inlining+NGEN
    This PR (7255) - mean (684ms)  : 658, 709
    master - mean (695ms)  : 670, 719

Loading
HttpMessageHandler (.NET Framework 4.8)
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7255) - mean (193ms)  : 190, 197
    master - mean (194ms)  : 191, 198

    section Bailout
    This PR (7255) - mean (196ms)  : 193, 199
    master - mean (198ms)  : 194, 202

    section CallTarget+Inlining+NGEN
    This PR (7255) - mean (1,151ms)  : 1103, 1199
    master - mean (1,152ms)  : 1106, 1199

Loading
HttpMessageHandler (.NET Core 3.1)
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7255) - mean (277ms)  : 273, 282
    master - mean (279ms)  : 275, 283

    section Bailout
    This PR (7255) - mean (278ms)  : 272, 283
    master - mean (278ms)  : 274, 282

    section CallTarget+Inlining+NGEN
    This PR (7255) - mean (902ms)  : 868, 936
    master - mean (907ms)  : 877, 937

Loading
HttpMessageHandler (.NET 6)
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7255) - mean (271ms)  : 264, 278
    master - mean (272ms)  : 265, 278

    section Bailout
    This PR (7255) - mean (270ms)  : 265, 274
    master - mean (271ms)  : 267, 275

    section CallTarget+Inlining+NGEN
    This PR (7255) - mean (935ms)  : 906, 965
    master - mean (946ms)  : 911, 980

Loading
HttpMessageHandler (.NET 8)
gantt
    title Execution time (ms) HttpMessageHandler (.NET 8)
    dateFormat  x
    axisFormat %Q
    todayMarker off
    section Baseline
    This PR (7255) - mean (270ms)  : 265, 275
    master - mean (269ms)  : 264, 274

    section Bailout
    This PR (7255) - mean (269ms)  : 266, 273
    master - mean (269ms)  : 264, 274

    section CallTarget+Inlining+NGEN
    This PR (7255) - mean (838ms)  : 808, 868
    master - mean (835ms)  : 814, 857

Loading

@pr-commenter

pr-commenter Bot commented Aug 19, 2025

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-03-16 23:06:31

Comparing candidate commit 3d255d6 in PR branch lpimentel/refactor-log-directory with baseline commit 48a1fe5 in branch master.

Found 10 performance improvements and 2 performance regressions! Performance is the same for 169 metrics, 11 unstable metrics.

scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.AllCycleSimpleBody netcoreapp3.1

  • 🟩 execution_time [-20.750ms; -14.300ms] or [-9.646%; -6.648%]

scenario:Benchmarks.Trace.Asm.AppSecBodyBenchmark.ObjectExtractorSimpleBody net6.0

  • 🟩 execution_time [-15.676ms; -11.745ms] or [-7.217%; -5.407%]

scenario:Benchmarks.Trace.CharSliceBenchmark.OptimizedCharSliceWithPool net6.0

  • 🟩 execution_time [-82.226µs; -78.814µs] or [-7.350%; -7.045%]
  • 🟩 throughput [+67.851op/s; +70.792op/s] or [+7.591%; +7.920%]

scenario:Benchmarks.Trace.GraphQLBenchmark.ExecuteAsync netcoreapp3.1

  • 🟩 throughput [+28107.457op/s; +36029.258op/s] or [+7.097%; +9.097%]

scenario:Benchmarks.Trace.Iast.StringAspectsBenchmark.StringConcatBenchmark net6.0

  • 🟩 execution_time [-9.685µs; -5.569µs] or [-19.378%; -11.144%]
  • 🟩 throughput [+2496.771op/s; +4073.110op/s] or [+12.219%; +19.934%]

scenario:Benchmarks.Trace.Log4netBenchmark.EnrichedLog netcoreapp3.1

  • 🟩 execution_time [-33.117ms; -31.236ms] or [-16.407%; -15.475%]

scenario:Benchmarks.Trace.NLogBenchmark.EnrichedLog net472

  • 🟩 throughput [+6415.429op/s; +7113.483op/s] or [+5.002%; +5.547%]

scenario:Benchmarks.Trace.SerilogBenchmark.EnrichedLog net472

  • 🟩 throughput [+7698.037op/s; +8835.768op/s] or [+5.099%; +5.852%]

scenario:Benchmarks.Trace.SingleSpanAspNetCoreBenchmark.SingleSpanAspNetCore netcoreapp3.1

  • 🟥 throughput [-16299034.104op/s; -15187158.922op/s] or [-6.757%; -6.296%]

scenario:Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin net6.0

  • 🟥 execution_time [+12.310ms; +17.551ms] or [+6.192%; +8.828%]

@lucaspimentel
lucaspimentel force-pushed the lpimentel/refactor-log-directory branch from dccf49d to 3d77f7a Compare December 5, 2025 14:21
@lucaspimentel
lucaspimentel force-pushed the lpimentel/refactor-log-directory branch from a69e50b to f0bf2b3 Compare December 17, 2025 23:27
@datadog-datadog-prod-us1

This comment has been minimized.

@lucaspimentel
lucaspimentel force-pushed the lpimentel/refactor-log-directory branch 2 times, most recently from 65a9c16 to 956d0c9 Compare December 18, 2025 22:29
@lucaspimentel
lucaspimentel force-pushed the lpimentel/refactor-log-directory branch from 956d0c9 to a5f7c9b Compare February 27, 2026 00:09
@lucaspimentel lucaspimentel changed the title small refactoring around GetDefaultLogDirectory() [Tracer] Refactor log directory resolution into smaller methods Feb 27, 2026
@lucaspimentel
lucaspimentel marked this pull request as ready for review February 27, 2026 00:15
@lucaspimentel
lucaspimentel requested a review from a team as a code owner February 27, 2026 00:15
@lucaspimentel lucaspimentel changed the title [Tracer] Refactor log directory resolution into smaller methods Refactor log directory resolution into smaller methods, add tests Feb 27, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5f7c9bc77

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tracer/test/Datadog.Trace.Tests/Logging/DatadogLoggingFactoryTests.cs Outdated
@lucaspimentel
lucaspimentel force-pushed the lpimentel/refactor-log-directory branch from a5f7c9b to c2121d7 Compare February 27, 2026 00:25
Comment thread tracer/src/Datadog.Trace/Logging/Internal/DatadogLoggingFactory.cs Outdated
Comment thread tracer/src/Datadog.Trace/Logging/Internal/DatadogLoggingFactory.cs Outdated
Comment thread tracer/test/Datadog.Trace.Tests/Logging/DatadogLoggingFactoryTests.cs Outdated
Comment thread tracer/test/Datadog.Trace.Tests/Logging/DatadogLoggingFactoryTests.cs Outdated
Comment thread tracer/test/Datadog.Trace.Tests/Logging/DatadogLoggingFactoryTests.cs Outdated
Comment thread tracer/test/Datadog.Trace.Tests/Logging/DatadogLoggingFactoryTests.cs Outdated
Comment thread tracer/test/Datadog.Trace.Tests/Logging/DatadogLoggingFactoryTests.cs Outdated
@lucaspimentel
lucaspimentel force-pushed the lpimentel/refactor-log-directory branch from 1af6a0f to 1a2cbf0 Compare February 28, 2026 02:55
@lucaspimentel lucaspimentel added the status:do-not-merge Work is done. Can review, but do not merge yet. label Mar 6, 2026
@lucaspimentel
lucaspimentel force-pushed the lpimentel/refactor-log-directory branch from f3bcc1c to e85a39f Compare March 16, 2026 20:33
@lucaspimentel lucaspimentel removed the status:do-not-merge Work is done. Can review, but do not merge yet. label Mar 16, 2026
@lucaspimentel
lucaspimentel enabled auto-merge (squash) March 16, 2026 22:38
@lucaspimentel
lucaspimentel merged commit 7a84734 into master Mar 17, 2026
71 of 75 checks passed
@lucaspimentel
lucaspimentel deleted the lpimentel/refactor-log-directory branch March 17, 2026 00:26
@github-actions github-actions Bot added this to the vNext-v3 milestone Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) type:cleanup Minor code clean up

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants