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

Add DSM support for AWS SNS #6253

Merged
merged 11 commits into from
Nov 14, 2024
Merged

Add DSM support for AWS SNS #6253

merged 11 commits into from
Nov 14, 2024

Conversation

vandonr
Copy link
Contributor

@vandonr vandonr commented Nov 7, 2024

Summary of changes

  • sending an SNS message will set a checkpoint for DSM
  • the pathway hash determined on checkpoint will be injected in the message

Reason for change

Implementation details

Test coverage

Added integration test similar to the one we have for SQS

Other details

JIRA: AIDM-414

Copy link
Contributor

github-actions bot commented Nov 7, 2024

Snapshots difference summary

The following differences have been observed in committed snapshots. It is meant to help the reviewer.
The diff is simplistic, so please check some files anyway while we improve it.

1 occurrences of :

+    Name: sns.request,
+    Resource: SNS.PublishBatch,
+    Service: Samples.AWS.SimpleNotificationService-aws-sns,
+    Type: http,
+    ParentId: Id_2,
+    Tags: {
+      aws.agent: dotnet-aws-sdk,
+      aws.operation: PublishBatch,
+      aws.requestId: Guid_3,
+      aws.service: SNS,
+      aws.topic.arn: arn:aws:sns:us-east-1:000000000000:MyTopic,
+      aws.topic.name: MyTopic,
+      aws_service: SNS,
+      component: aws-sdk,
+      env: integration_tests,
+      http.method: POST,
+      http.status_code: 200,
+      http.url: http://localhost:00000/,
+      language: dotnet,
+      runtime-id: Guid_1,
+      span.kind: client,
+      topicname: MyTopic,
+      _dd.base_service: Samples.AWS.SimpleNotificationService
+    },
+    Metrics: {
+      _dd.top_level: 1.0
+    }
+  },
+  {
+    TraceId: Id_1,
+    SpanId: Id_6,

1 occurrences of :

+    Name: aws.sns.send,
+    Resource: SNS.PublishBatch,
+    Service: Samples.AWS.SimpleNotificationService,
+    Type: http,
+    ParentId: Id_2,
+    Tags: {
+      aws.agent: dotnet-aws-sdk,
+      aws.operation: PublishBatch,
+      aws.requestId: Guid_3,
+      aws.service: SNS,
+      aws.topic.arn: arn:aws:sns:us-east-1:000000000000:MyTopic,
+      aws.topic.name: MyTopic,
+      aws_service: SNS,
+      component: aws-sdk,
+      env: integration_tests,
+      http.method: POST,
+      http.status_code: 200,
+      http.url: http://localhost:00000/,
+      language: dotnet,
+      peer.service: MyTopic,
+      span.kind: producer,
+      topicname: MyTopic,
+      _dd.peer.service.source: topicname
+    }
+  },
+  {
+    TraceId: Id_1,
+    SpanId: Id_6,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noticed that the SNS example was missing from the slnf, so I decided to do a pass and add all missing projects.

@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Nov 7, 2024

Datadog Report

Branch report: vandonr/dsm
Commit report: 3b4c98c
Test service: dd-trace-dotnet

✅ 0 Failed, 447478 Passed, 2726 Skipped, 19h 14m 23.67s Total Time

@andrewlock
Copy link
Member

andrewlock commented Nov 7, 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.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6253) - mean (72ms)  : 62, 82
     .   : milestone, 72,
    master - mean (72ms)  : 63, 82
     .   : milestone, 72,

    section CallTarget+Inlining+NGEN
    This PR (6253) - mean (1,107ms)  : 1087, 1128
     .   : milestone, 1107,
    master - mean (1,110ms)  : 1091, 1129
     .   : milestone, 1110,

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

    section CallTarget+Inlining+NGEN
    This PR (6253) - mean (768ms)  : 752, 784
     .   : milestone, 768,
    master - mean (765ms)  : 747, 783
     .   : milestone, 765,

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

    section CallTarget+Inlining+NGEN
    This PR (6253) - mean (725ms)  : 710, 740
     .   : milestone, 725,
    master - mean (726ms)  : 709, 742
     .   : milestone, 726,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6253) - mean (190ms)  : 186, 195
     .   : milestone, 190,
    master - mean (191ms)  : 184, 199
     .   : milestone, 191,

    section CallTarget+Inlining+NGEN
    This PR (6253) - mean (1,227ms)  : 1208, 1247
     .   : milestone, 1227,
    master - mean (1,228ms)  : 1207, 1249
     .   : milestone, 1228,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6253) - mean (276ms)  : 272, 281
     .   : milestone, 276,
    master - mean (277ms)  : 273, 281
     .   : milestone, 277,

    section CallTarget+Inlining+NGEN
    This PR (6253) - mean (943ms)  : 924, 961
     .   : milestone, 943,
    master - mean (943ms)  : 925, 961
     .   : milestone, 943,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6253) - mean (265ms)  : 262, 268
     .   : milestone, 265,
    master - mean (266ms)  : 262, 270
     .   : milestone, 266,

    section CallTarget+Inlining+NGEN
    This PR (6253) - mean (930ms)  : 912, 947
     .   : milestone, 930,
    master - mean (931ms)  : 914, 947
     .   : milestone, 931,

Loading

@andrewlock
Copy link
Member

andrewlock commented Nov 7, 2024

Benchmarks Report for tracer 🐌

Benchmarks for #6253 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.152
  • 1 benchmarks are slower, with geometric mean 1.153
  • 1 benchmarks have more 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.11μs 45.3ns 300ns 0.0162 0.0081 0 5.61 KB
master StartStopWithChild netcoreapp3.1 10.2μs 54.4ns 272ns 0.0158 0.00527 0 5.8 KB
master StartStopWithChild net472 16.4μs 60.8ns 236ns 1.05 0.32 0.0986 6.21 KB
#6253 StartStopWithChild net6.0 8.15μs 44.8ns 265ns 0.0121 0.00809 0 5.61 KB
#6253 StartStopWithChild netcoreapp3.1 10.3μs 57.2ns 375ns 0.0222 0.0111 0 5.8 KB
#6253 StartStopWithChild net472 16.4μs 68.7ns 266ns 1.03 0.292 0.0892 6.21 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 492μs 292ns 1.09μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 662μs 354ns 1.32μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 854μs 559ns 2.17μs 0.425 0 0 3.3 KB
#6253 WriteAndFlushEnrichedTraces net6.0 541μs 956ns 3.7μs 0 0 0 2.7 KB
#6253 WriteAndFlushEnrichedTraces netcoreapp3.1 663μs 246ns 954ns 0 0 0 2.7 KB
#6253 WriteAndFlushEnrichedTraces net472 863μs 527ns 1.97μs 0.425 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 199μs 1.14μs 8.86μs 0.185 0 0 18.73 KB
master SendRequest netcoreapp3.1 225μs 1.31μs 12.9μs 0.214 0 0 20.89 KB
master SendRequest net472 0.0134ns 0.00221ns 0.00828ns 0 0 0 0 b
#6253 SendRequest net6.0 206μs 1.2μs 11.1μs 0.192 0 0 18.73 KB
#6253 SendRequest netcoreapp3.1 226μs 1.32μs 11.5μs 0.219 0 0 20.89 KB
#6253 SendRequest net472 0.00145ns 0.000432ns 0.00156ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6253

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 41.5 KB 41.78 KB 272 B 0.66%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 540μs 2.49μs 9.33μs 0.306 0 0 41.5 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 663μs 1.83μs 6.84μs 0.34 0 0 41.74 KB
master WriteAndFlushEnrichedTraces net472 864μs 4.17μs 17.2μs 8.42 2.66 0.443 53.3 KB
#6253 WriteAndFlushEnrichedTraces net6.0 587μs 2.68μs 10.4μs 0.579 0 0 41.78 KB
#6253 WriteAndFlushEnrichedTraces netcoreapp3.1 676μs 3.37μs 15.4μs 0.332 0 0 41.75 KB
#6253 WriteAndFlushEnrichedTraces net472 899μs 3.37μs 12.6μs 8.54 2.25 0.45 53.25 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.25μs 0.711ns 2.75ns 0.0143 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.81μs 1.02ns 3.94ns 0.0136 0 0 1.02 KB
master ExecuteNonQuery net472 2.14μs 0.862ns 3.11ns 0.156 0.00107 0 987 B
#6253 ExecuteNonQuery net6.0 1.32μs 1.06ns 4.1ns 0.0145 0 0 1.02 KB
#6253 ExecuteNonQuery netcoreapp3.1 1.75μs 1.17ns 4.36ns 0.0135 0 0 1.02 KB
#6253 ExecuteNonQuery net472 1.96μs 1.24ns 4.81ns 0.156 0.000981 0 987 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.16μs 0.431ns 1.61ns 0.0135 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.56μs 1.06ns 4.09ns 0.0133 0 0 976 B
master CallElasticsearch net472 2.56μs 3.88ns 15ns 0.158 0 0 995 B
master CallElasticsearchAsync net6.0 1.34μs 0.644ns 2.49ns 0.0134 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.69μs 0.961ns 3.6ns 0.0135 0 0 1.02 KB
master CallElasticsearchAsync net472 2.73μs 1.93ns 7.47ns 0.167 0 0 1.05 KB
#6253 CallElasticsearch net6.0 1.23μs 0.956ns 3.58ns 0.0134 0 0 976 B
#6253 CallElasticsearch netcoreapp3.1 1.49μs 1.17ns 4.39ns 0.0134 0 0 976 B
#6253 CallElasticsearch net472 2.57μs 2.68ns 10.4ns 0.157 0 0 995 B
#6253 CallElasticsearchAsync net6.0 1.26μs 0.503ns 1.88ns 0.0132 0 0 952 B
#6253 CallElasticsearchAsync netcoreapp3.1 1.6μs 0.753ns 2.82ns 0.0136 0 0 1.02 KB
#6253 CallElasticsearchAsync net472 2.68μs 2.29ns 8.87ns 0.166 0 0 1.05 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.3μs 1.19ns 4.47ns 0.0131 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.66μs 1.7ns 6.59ns 0.0125 0 0 952 B
master ExecuteAsync net472 1.85μs 0.455ns 1.76ns 0.145 0 0 915 B
#6253 ExecuteAsync net6.0 1.29μs 0.882ns 3.3ns 0.013 0 0 952 B
#6253 ExecuteAsync netcoreapp3.1 1.66μs 0.533ns 1.99ns 0.0124 0 0 952 B
#6253 ExecuteAsync net472 1.81μs 0.39ns 1.51ns 0.145 0 0 915 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.47μs 1.39ns 5.19ns 0.0311 0 0 2.31 KB
master SendAsync netcoreapp3.1 5.23μs 2.08ns 8.05ns 0.0392 0 0 2.85 KB
master SendAsync net472 7.24μs 2.09ns 8.08ns 0.496 0 0 3.12 KB
#6253 SendAsync net6.0 4.28μs 1.71ns 6.38ns 0.0322 0 0 2.31 KB
#6253 SendAsync netcoreapp3.1 5.24μs 2.16ns 8.09ns 0.0391 0 0 2.85 KB
#6253 SendAsync net472 7.24μs 1.32ns 4.94ns 0.493 0 0 3.12 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.5μs 0.543ns 2.03ns 0.0227 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.3μs 0.966ns 3.61ns 0.0218 0 0 1.64 KB
master EnrichedLog net472 2.62μs 1.07ns 4.14ns 0.249 0 0 1.57 KB
#6253 EnrichedLog net6.0 1.53μs 0.569ns 2.13ns 0.023 0 0 1.64 KB
#6253 EnrichedLog netcoreapp3.1 2.22μs 0.808ns 2.91ns 0.0222 0 0 1.64 KB
#6253 EnrichedLog net472 2.63μs 1.1ns 4.12ns 0.249 0 0 1.57 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 120μs 138ns 535ns 0.0601 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 124μs 115ns 446ns 0 0 0 4.28 KB
master EnrichedLog net472 153μs 139ns 500ns 0.683 0.228 0 4.46 KB
#6253 EnrichedLog net6.0 120μs 122ns 455ns 0 0 0 4.28 KB
#6253 EnrichedLog netcoreapp3.1 123μs 135ns 504ns 0 0 0 4.28 KB
#6253 EnrichedLog net472 151μs 171ns 662ns 0.676 0.225 0 4.46 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μs 0.625ns 2.34ns 0.03 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.15μs 1.41ns 5.46ns 0.029 0 0 2.2 KB
master EnrichedLog net472 4.9μs 1.46ns 5.66ns 0.32 0 0 2.02 KB
#6253 EnrichedLog net6.0 2.98μs 0.891ns 3.45ns 0.0313 0 0 2.2 KB
#6253 EnrichedLog netcoreapp3.1 4.21μs 1.34ns 5.01ns 0.0295 0 0 2.2 KB
#6253 EnrichedLog net472 4.91μs 1.46ns 5.66ns 0.32 0 0 2.02 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.41μs 0.673ns 2.6ns 0.0165 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.75μs 3.74ns 14ns 0.0157 0 0 1.14 KB
master SendReceive net472 1.98μs 0.707ns 2.45ns 0.183 0 0 1.16 KB
#6253 SendReceive net6.0 1.35μs 0.549ns 2.13ns 0.016 0 0 1.14 KB
#6253 SendReceive netcoreapp3.1 1.69μs 1.21ns 4.69ns 0.0154 0 0 1.14 KB
#6253 SendReceive net472 2μs 1.19ns 4.59ns 0.183 0 0 1.16 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.75μs 1.03ns 3.85ns 0.0219 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 4.03μs 1.73ns 6.69ns 0.0221 0 0 1.65 KB
master EnrichedLog net472 4.45μs 1.77ns 6.38ns 0.323 0 0 2.04 KB
#6253 EnrichedLog net6.0 2.78μs 0.539ns 2.02ns 0.0223 0 0 1.6 KB
#6253 EnrichedLog netcoreapp3.1 3.94μs 1.16ns 4.35ns 0.0216 0 0 1.65 KB
#6253 EnrichedLog net472 4.44μs 9.82ns 36.7ns 0.323 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6253

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net472 1.153 849.11 978.82

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 403ns 0.171ns 0.662ns 0.00807 0 0 576 B
master StartFinishSpan netcoreapp3.1 619ns 1.56ns 6.02ns 0.00778 0 0 576 B
master StartFinishSpan net472 693ns 1.02ns 3.94ns 0.0916 0 0 578 B
master StartFinishScope net6.0 487ns 0.371ns 1.44ns 0.00969 0 0 696 B
master StartFinishScope netcoreapp3.1 764ns 0.639ns 2.47ns 0.00951 0 0 696 B
master StartFinishScope net472 850ns 2.83ns 10.9ns 0.104 0 0 658 B
#6253 StartFinishSpan net6.0 409ns 0.348ns 1.35ns 0.008 0 0 576 B
#6253 StartFinishSpan netcoreapp3.1 559ns 0.463ns 1.79ns 0.00785 0 0 576 B
#6253 StartFinishSpan net472 641ns 1.02ns 3.96ns 0.0918 0 0 578 B
#6253 StartFinishScope net6.0 485ns 0.351ns 1.36ns 0.00976 0 0 696 B
#6253 StartFinishScope netcoreapp3.1 695ns 0.656ns 2.54ns 0.00946 0 0 696 B
#6253 StartFinishScope net472 979ns 0.757ns 2.93ns 0.104 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6253

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0 1.152 673.74 584.87

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 675ns 0.864ns 3.34ns 0.00967 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 888ns 0.532ns 2.06ns 0.00907 0 0 696 B
master RunOnMethodBegin net472 1.21μs 0.751ns 2.91ns 0.104 0 0 658 B
#6253 RunOnMethodBegin net6.0 585ns 0.301ns 1.17ns 0.00969 0 0 696 B
#6253 RunOnMethodBegin netcoreapp3.1 957ns 0.732ns 2.83ns 0.00944 0 0 696 B
#6253 RunOnMethodBegin net472 1.16μs 0.769ns 2.98ns 0.104 0 0 658 B

@andrewlock
Copy link
Member

andrewlock commented Nov 8, 2024

Throughput/Crank Report ⚡

Throughput results for AspNetCoreSimpleController comparing the following branches/commits:

Cases where throughput results for the PR are worse than latest master (5% drop or greater), results are shown in red.

Note that these results are based on a single point-in-time result for each branch. For full results, see one of the many, many dashboards!

gantt
    title Throughput Linux x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6253) (11.101M)   : 0, 11101001
    master (11.174M)   : 0, 11173945
    benchmarks/2.9.0 (11.033M)   : 0, 11032866

    section Automatic
    This PR (6253) (7.236M)   : 0, 7236470
    master (7.312M)   : 0, 7312280
    benchmarks/2.9.0 (7.786M)   : 0, 7785853

    section Trace stats
    master (7.553M)   : 0, 7553254

    section Manual
    master (11.102M)   : 0, 11101684

    section Manual + Automatic
    This PR (6253) (6.814M)   : 0, 6813696
    master (6.740M)   : 0, 6739647

    section DD_TRACE_ENABLED=0
    master (10.282M)   : 0, 10282028

Loading
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6253) (9.571M)   : 0, 9571042
    master (9.530M)   : 0, 9530431
    benchmarks/2.9.0 (9.495M)   : 0, 9494821

    section Automatic
    This PR (6253) (6.415M)   : 0, 6415352
    master (6.395M)   : 0, 6394628

    section Trace stats
    master (6.680M)   : 0, 6679934

    section Manual
    master (9.631M)   : 0, 9630609

    section Manual + Automatic
    This PR (6253) (6.046M)   : 0, 6045556
    master (5.995M)   : 0, 5995215

    section DD_TRACE_ENABLED=0
    master (8.922M)   : 0, 8921645

Loading
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6253) (9.541M)   : 0, 9540968
    master (9.758M)   : 0, 9758058
    benchmarks/2.9.0 (10.020M)   : 0, 10019592

    section Automatic
    This PR (6253) (6.209M)   : 0, 6209266
    master (6.513M)   : 0, 6512759
    benchmarks/2.9.0 (7.255M)   : 0, 7255257

    section Trace stats
    master (7.074M)   : 0, 7073637

    section Manual
    master (9.827M)   : 0, 9827350

    section Manual + Automatic
    This PR (6253) (5.934M)   : 0, 5934211
    master (5.944M)   : 0, 5943752

    section DD_TRACE_ENABLED=0
    master (9.348M)   : 0, 9348488

Loading

Comment on lines -99 to -100
null or "" => ".pre3.7.101.88",
{ } v when new Version(v) < new Version("3.7.101.88") => ".pre3.7.101.88",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was wrong, the value used in the sample app to determine if we do a batch request or not is 3.7.3, and it's the value used by default if there is no version specified.

@vandonr vandonr marked this pull request as ready for review November 12, 2024 09:17
@vandonr vandonr requested review from a team as code owners November 12, 2024 09:17
Copy link
Contributor

@bouwkast bouwkast left a comment

Choose a reason for hiding this comment

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

I've modified tests to run on .NET Framework could you just check those over to make sure they look good (I think they are fine)

@@ -23,17 +25,24 @@ public static CallTargetState BeforePublish<TPublishRequest>(TPublishRequest req
var requestProxy = request.DuckCast<IAmazonSNSRequestWithTopicArn>();

var scope = AwsSnsCommon.CreateScope(Tracer.Instance, sendType.OperationName, SpanKinds.Producer, out var tags);

var topicName = AwsSnsCommon.GetTopicName(requestProxy.TopicArn);
Copy link
Contributor

Choose a reason for hiding this comment

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

requestProxy.TopicArn appears that it could be null here as there is the null check directly below.

Granted the GetTopicName seems to just return null when given a null string

@@ -26,6 +26,9 @@ public static async Task StartSNSTasks(AmazonSimpleNotificationServiceClient sns
await PublishMessageAsync(snsClient, topicArn);
#if AWS_SNS_3_7_3
await PublishBatchAsync(snsClient, topicArn);
#else
// do a second publish if batch is not available to keep a constant number of spans and make assertions simpler
await PublishMessageAsync(snsClient, topicArn);
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is okay but I'm unsure how I really feel about this as it is a bit misleading. The span count will be consistent, but not the operations between them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, yeah, but the operations are reflected in the snapshots, which are different anyway. Having this operation really makes the code simpler imho 😶

Copy link
Contributor

@bouwkast bouwkast left a comment

Choose a reason for hiding this comment

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

👍

@vandonr vandonr merged commit 5325593 into master Nov 14, 2024
77 of 79 checks passed
@vandonr vandonr deleted the vandonr/dsm branch November 14, 2024 14:52
@github-actions github-actions bot added this to the vNext-v3 milestone Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants