Skip to content

Conversation

@ganeshnj
Copy link

@ganeshnj ganeshnj commented Nov 19, 2024

Summary of changes

Use native TraceExporter to send traces.

Reason for change

As a strategic goal to move common stuff across the trace SDKs to native components, this PR targets the Trace exporter component.

Goal for the PR is to get into merging state so when the blocking changes are fixed. We can speed the merging.

Implementation details

TraceExporter implements the IApi interface which is responsible to send the traces to the agent. It has no-op for stats since the stats computation happens inside the native side when enabled.

Remaining work (will be different PRs)

  • Conditionally enable data pipeline on Azure App Services only
  • Improve logging

⚠️ This change targets a feature branch since we can't merge this in current form because some ad-hoc hacking done to get the CI working.

⚠️ reviewers must focus on changes in tracer dir only, rest of changes are just boilerplate for making CI work and it will be removed after vcpkg integration.

Test coverage

  • More tests are added to make sure TraceExporter works with TCP, UDP and Windows named pipes
  • Tests are updated to exercise both when data pipeline is enabled/disabled.

Other details

@ganeshnj ganeshnj changed the base branch from master to feature/data-pipeline November 19, 2024 14:01
@ganeshnj ganeshnj force-pushed the ganeshnj/feature/datapipeline-bindings branch from 9fdb202 to b4f83a1 Compare November 19, 2024 14:03
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Nov 19, 2024

Datadog Report

Branch report: ganeshnj/feature/datapipeline-bindings
Commit report: d982e5f
Test service: dd-trace-dotnet

❌ 926 Failed (0 Known Flaky), 56736 Passed, 208 Skipped, 5h 47m 35.51s Total Time

❌ Failed Tests (926)

This report shows up to 5 failed tests.

  • StartStopWithChild - Benchmarks.Trace.ActivityBenchmark - Details

  • StartStopWithChild - Benchmarks.Trace.ActivityBenchmark - Details

  • StartStopWithChild - Benchmarks.Trace.ActivityBenchmark - Details

  • AzureFunctionsTests+InProcessRuntimeV4.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error
     Expected collection to contain at least 21 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.
    
  • AzureFunctionsTests+IsolatedRuntimeV4SdkV1.SubmitsTraces - Datadog.Trace.ClrProfiler.IntegrationTests - Details

    Expand for error
     Expected collection to contain at least 21 item(s) because we want to ensure that we don't timeout while waiting for spans from the mock tracer agent, but found 0: {empty}.
    

@ganeshnj ganeshnj changed the title feat: setup boilerplate for data-pipeline integration APMSP-1543 feat: setup boilerplate for data-pipeline ®integration Nov 25, 2024
@ganeshnj ganeshnj changed the title APMSP-1543 feat: setup boilerplate for data-pipeline ®integration APMSP-1543 feat: setup boilerplate for data-pipeline integration Nov 25, 2024
@ganeshnj ganeshnj force-pushed the ganeshnj/feature/datapipeline-bindings branch from 968ca73 to 31c6659 Compare November 25, 2024 16:33
@github-actions

This comment was marked as outdated.

@ganeshnj ganeshnj changed the base branch from feature/data-pipeline to master November 25, 2024 16:39
@ganeshnj ganeshnj changed the base branch from master to feature/data-pipeline November 25, 2024 16:39
@ganeshnj ganeshnj force-pushed the ganeshnj/feature/datapipeline-bindings branch 3 times, most recently from a5b2c7c to f3dcd3b Compare December 3, 2024 09:29
@andrewlock

This comment was marked as outdated.

@ganeshnj ganeshnj force-pushed the ganeshnj/feature/datapipeline-bindings branch 2 times, most recently from b47644d to 356bf2b Compare December 3, 2024 13:58
@ganeshnj ganeshnj changed the base branch from feature/data-pipeline to master December 3, 2024 13:58
@ganeshnj ganeshnj changed the base branch from master to feature/data-pipeline December 3, 2024 13:58
@andrewlock

This comment was marked as outdated.

@ganeshnj ganeshnj force-pushed the ganeshnj/feature/datapipeline-bindings branch from 555cc8c to 23abd8b Compare December 6, 2024 14:27
@ganeshnj ganeshnj changed the base branch from feature/data-pipeline to master December 9, 2024 16:19
@ganeshnj ganeshnj changed the base branch from master to feature/data-pipeline December 9, 2024 16:19
@ganeshnj ganeshnj force-pushed the ganeshnj/feature/datapipeline-bindings branch from 23abd8b to cf6363a Compare December 16, 2024 16:20
@ganeshnj ganeshnj changed the base branch from feature/data-pipeline to master December 16, 2024 16:26
@ganeshnj ganeshnj force-pushed the ganeshnj/feature/datapipeline-bindings branch from cf6363a to eafc440 Compare December 16, 2024 18:55
@andrewlock
Copy link
Member

andrewlock commented Dec 16, 2024

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.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6314) - mean (68ms)  : 65, 71
     .   : milestone, 68,
    master - mean (68ms)  : 66, 69
     .   : milestone, 68,

    section CallTarget+Inlining+NGEN
    This PR (6314) - mean (1,018ms)  : 1002, 1034
     .   : milestone, 1018,
    master - mean (1,007ms)  : 985, 1028
     .   : milestone, 1007,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6314) - mean (102ms)  : 100, 105
     .   : milestone, 102,
    master - mean (102ms)  : 100, 104
     .   : milestone, 102,

    section CallTarget+Inlining+NGEN
    This PR (6314) - mean (695ms)  : 682, 707
     .   : milestone, 695,
    master - mean (714ms)  : 644, 784
     .   : milestone, 714,

Loading
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6314) - mean (89ms)  : 87, 91
     .   : milestone, 89,
    master - mean (90ms)  : 87, 92
     .   : milestone, 90,

    section CallTarget+Inlining+NGEN
    This PR (6314) - mean (664ms)  : 648, 681
     .   : milestone, 664,
    master - mean (659ms)  : 636, 682
     .   : milestone, 659,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.8) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6314) - mean (190ms)  : 186, 194
     .   : milestone, 190,
    master - mean (190ms)  : 186, 193
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6314) - mean (1,164ms)  : 1143, 1184
     .   : milestone, 1164,
    master - mean (1,114ms)  : 1084, 1143
     .   : milestone, 1114,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6314) - mean (269ms)  : 265, 272
     .   : milestone, 269,
    master - mean (269ms)  : 265, 274
     .   : milestone, 269,

    section CallTarget+Inlining+NGEN
    This PR (6314) - mean (927ms)  : 916, 938
     .   : milestone, 927,
    master - mean (882ms)  : 843, 922
     .   : milestone, 882,

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

    section CallTarget+Inlining+NGEN
    This PR (6314) - mean (928ms)  : crit, 905, 951
     .   : crit, milestone, 928,
    master - mean (870ms)  : 844, 896
     .   : milestone, 870,

Loading

@ganeshnj ganeshnj force-pushed the ganeshnj/feature/datapipeline-bindings branch 4 times, most recently from 22b9f30 to 067753f Compare December 19, 2024 17:10
@andrewlock andrewlock force-pushed the ganeshnj/feature/datapipeline-bindings branch from 4c68524 to c18b698 Compare June 3, 2025 16:43
Copy link
Member

@andrewlock andrewlock 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 👍

Copy link
Contributor

@GreenMatan GreenMatan left a comment

Choose a reason for hiding this comment

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

Reviewed Debugger's related changes and LGTM

@ganeshnj ganeshnj merged commit ffce5ef into master Jun 4, 2025
133 checks passed
@ganeshnj ganeshnj deleted the ganeshnj/feature/datapipeline-bindings branch June 4, 2025 08:54
@github-actions github-actions bot added this to the vNext-v3 milestone Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:enhancement Improvement to an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants