[Tracer] Add peer.service to Service Remoting integration #4435

zacharycmontoya · 2023-07-23T17:23:58Z

Summary of changes

Initial refactorings:
- Splits up the v0 tags objects for Service Remoting client spans and server spans, so we can make stronger assertions on the different spans
- Moves tag name constants to tracer/src/Datadog.Trace/Tags.cs
Add service-fabric.service-remoting.service tag for v0 and v1
For v1 only, change the operation names of the Service Remoting spans to match the convention of other v1 operation names:
- Client: service_remoting.client => service_remoting.client.request
- Server: service_remoting.server => service_remoting.server.request
For v1 only, add the following peer.service tags to the Service Remoting client spans where peer.service has the following precedence: custom > service-fabric.service-remoting.service > service-fabric.service-remoting.uri
- peer.service:
- peer.service.remapped_from
- _dd.peer.service.source

Reason for change

We must add the peer.service tag for client spans. As for the peer.service precursors, the RPC service name is the most specific identifier and if for some reason we do not have that, then the URI will suffice.

A Service Fabric application contains one or more services that run your code, so if you have a Service Fabric application named HelloWorld and a stateless service called HelloWorldStateless, clients would create a connection to a specific service using the URI fabric:/HelloWorld/HelloWorldStateless. The resulting peer.service-related tags are:

service-fabric.service-remoting.service: HelloWorld/HelloWorldStateless
service-fabric.service-remoting.uri: fabric:/HelloWorld/HelloWorldStateless

Implementation details

The implementation follows from other peer.service PR's and auto-populates peer.service and _dd.peer.service.source based on the above precursors using a new tags class, ServiceRemotingClientV1Tags. This PR adds a new tag to client spans called service-fabric.service-remoting.service which is a newly allocated substring of the existing service-fabric.service-remoting.uri value that already exists on the in-memory Service Remoting request object. Since the URI is likely long-lived (one URI per client connection), we can likely optimize this by using a ConditionalWeakTable here.

Test coverage

Adds unit tests for the new tags class ServiceRemotingClientV1Tags and for the updated ClientSchema. However there are no integration tests at this moment. I've authored a setup locally and have seen the changes reflected in the spans.

Note: I plan to add the integration tests and test applications in a follow-up PR, so we can merge the implementation more quickly.

Other details

Also adds call to NamingSchema.RemapPeerService for the following integrations:

Aerospike
GrpcDotnet (client spans only)
RabbitMQ (publisher spans only)

Refactoring: - Moves the ServiceFabric and ServiceRemoting tag constants to the Tags.cs file - Makes the ServiceRemotingTags class abstract and splits the implementation types into ServiceRemotingServerTags and ServiceRemotingClientTags New changes: - Adds a ServiceRemotingClientV1Tags type that derives from ServiceRemotingClientTags - Adds a new method ClientSchema.CreateServiceRemotingClientTags and updates the client tags creation with this call - Adds tests to ClientSchemaTests for the new tags creation method

… remoting URI and extracts everything after the starting "fabric:/" substring. This will not be populated if the URI does not begin with "fabric:/". Implement peer.service tags for ServiceRemotingClientV1Tags with the following precursors: - Custom (_dd.peer.service.source="peer.service") - Service (_dd.peer.service.source="service-fabric.service-remoting.service") - URI (_dd.peer.service.source="service-fabric.service-remoting.uri") For example, when calling the RPC method "HelloWorldAsync" on a ServiceFabric application located at URI "fabric:/HelloWorld/HelloWorldStateless", a span with the following information would be generated: - OperationName: "service_remoting.client" - ResourceName: "fabric:/HelloWorld/HelloWorldStateless/HelloWorldAsync" - Tags (only a subset): - service-fabric.service-remoting.uri="fabric:/HelloWorld/HelloWorldStateless" - service-fabric.service-remoting.method-name="HelloWorldAsync" - peer.service="fabric:/HelloWorld/HelloWorldStateless" - _dd.peer.service.source="service-fabric.service-remoting.uri" Testing: - Add unit tests in InstrumentationTagsTests.cs - Update the SpanMetadata assertions (though they're not actually tested in CI) - Manual end-to-end testing that peer.service is set to the ServiceName - Manaul end-to-end testing that remapping sets peer.service.remapped_from for the original peer.service value

…) in missing places

datadog-ddstaging · 2023-07-23T17:45:04Z

Datadog Report

Branch report: zach/v1-schema/service-fabric-remoting
Commit report: cff8c37

✅ dd-trace-dotnet: 0 Failed, 0 New Flaky, 296671 Passed, 1091 Skipped, 26m 17.49s Wall Time

andrewlock · 2023-07-23T17:54:04Z

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 (4435) - mean (3,000ms)  : 2911, 3090
     .   : milestone, 3000,
    master - mean (2,987ms)  : 2907, 3067
     .   : milestone, 2987,

    section CallTarget+Inlining+NGEN
    This PR (4435) - mean (3,819ms)  : 3724, 3915
     .   : milestone, 3819,
    master - mean (3,824ms)  : 3733, 3915
     .   : milestone, 3824,

gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4435) - mean (3,122ms)  : 3024, 3219
     .   : milestone, 3122,
    master - mean (3,124ms)  : 3032, 3216
     .   : milestone, 3124,

    section CallTarget+Inlining+NGEN
    This PR (4435) - mean (3,621ms)  : 3554, 3688
     .   : milestone, 3621,
    master - mean (3,609ms)  : 3543, 3675
     .   : milestone, 3609,

gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4435) - mean (3,075ms)  : 2982, 3167
     .   : milestone, 3075,
    master - mean (3,082ms)  : 2998, 3166
     .   : milestone, 3082,

    section CallTarget+Inlining+NGEN
    This PR (4435) - mean (3,592ms)  : 3527, 3657
     .   : milestone, 3592,
    master - mean (3,581ms)  : 3510, 3651
     .   : milestone, 3581,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4435) - mean (193ms)  : 186, 199
     .   : milestone, 193,
    master - mean (192ms)  : 187, 196
     .   : milestone, 192,

    section CallTarget+Inlining+NGEN
    This PR (4435) - mean (1,121ms)  : 1091, 1151
     .   : milestone, 1121,
    master - mean (1,125ms)  : 1092, 1158
     .   : milestone, 1125,

gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4435) - mean (374ms)  : 367, 380
     .   : milestone, 374,
    master - mean (372ms)  : 368, 377
     .   : milestone, 372,

    section CallTarget+Inlining+NGEN
    This PR (4435) - mean (1,174ms)  : 1146, 1202
     .   : milestone, 1174,
    master - mean (1,176ms)  : 1146, 1205
     .   : milestone, 1176,

gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (4435) - mean (363ms)  : 356, 370
     .   : milestone, 363,
    master - mean (360ms)  : 354, 366
     .   : milestone, 360,

    section CallTarget+Inlining+NGEN
    This PR (4435) - mean (1,142ms)  : 1106, 1178
     .   : milestone, 1142,
    master - mean (1,133ms)  : 1108, 1157
     .   : milestone, 1133,

andrewlock · 2023-07-23T18:00:24Z

Benchmarks Report 🐌

Benchmarks for #4435 compared to master:

1 benchmarks are faster, with geometric mean 1.295
4 benchmarks are slower, with geometric mean 1.175
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.AgentWriterBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	465μs	131ns	507ns	0	2.62 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	647μs	441ns	1.71μs	0	2.63 KB
master	`WriteAndFlushEnrichedTraces`	net472	792μs	343ns	1.28μs	0.396	3.22 KB
#4435	`WriteAndFlushEnrichedTraces`	net6.0	488μs	183ns	685ns	0	2.62 KB
#4435	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	627μs	148ns	555ns	0	2.62 KB
#4435	`WriteAndFlushEnrichedTraces`	net472	798μs	334ns	1.25μs	0.398	3.22 KB

Benchmarks.Trace.AppSecBodyBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #4435

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.AppSecBodyBenchmark.ObjectExtractorMoreComplexBody‑net472	1.155	4,143.06	4,783.89

Faster 🎉 in #4435

Benchmark	base/diff	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.AppSecBodyBenchmark.ObjectExtractorSimpleBody‑net472	1.295	191.15	147.59

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`AllCycleSimpleBody`	net6.0	38.6μs	39.9ns	155ns	0.0193	0	1.65 KB
master	`AllCycleSimpleBody`	netcoreapp3.1	41.4μs	99.2ns	371ns	0	0	1.63 KB
master	`AllCycleSimpleBody`	net472	41.7μs	11.9ns	44.4ns	0.251	0	1.69 KB
master	`AllCycleMoreComplexBody`	net6.0	219μs	79.8ns	309ns	0.11	0	9.22 KB
master	`AllCycleMoreComplexBody`	netcoreapp3.1	230μs	146ns	507ns	0.115	0	9.12 KB
master	`AllCycleMoreComplexBody`	net472	240μs	63ns	227ns	1.44	0	9.28 KB
master	`ObjectExtractorSimpleBody`	net6.0	117ns	0.0438ns	0.164ns	0.00391	0	280 B
master	`ObjectExtractorSimpleBody`	netcoreapp3.1	175ns	0.0936ns	0.338ns	0.0037	0	272 B
master	`ObjectExtractorSimpleBody`	net472	191ns	0.648ns	2.51ns	0.0446	0	281 B
master	`ObjectExtractorMoreComplexBody`	net6.0	3.06μs	0.701ns	2.62ns	0.0549	0	3.88 KB
master	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	3.92μs	2.05ns	7.93ns	0.051	0	3.78 KB
master	`ObjectExtractorMoreComplexBody`	net472	4.15μs	3.13ns	12.1ns	0.617	0.00619	3.89 KB
#4435	`AllCycleSimpleBody`	net6.0	39.1μs	88.4ns	342ns	0.0193	0	1.65 KB
#4435	`AllCycleSimpleBody`	netcoreapp3.1	41.1μs	23.5ns	87.9ns	0.0206	0	1.63 KB
#4435	`AllCycleSimpleBody`	net472	42.5μs	90.9ns	340ns	0.249	0	1.69 KB
#4435	`AllCycleMoreComplexBody`	net6.0	222μs	58ns	209ns	0.111	0	9.22 KB
#4435	`AllCycleMoreComplexBody`	netcoreapp3.1	232μs	78.9ns	295ns	0.114	0	9.12 KB
#4435	`AllCycleMoreComplexBody`	net472	239μs	43ns	155ns	1.43	0	9.28 KB
#4435	`ObjectExtractorSimpleBody`	net6.0	117ns	0.0371ns	0.139ns	0.00397	0	280 B
#4435	`ObjectExtractorSimpleBody`	netcoreapp3.1	174ns	0.126ns	0.488ns	0.00377	0	272 B
#4435	`ObjectExtractorSimpleBody`	net472	147ns	0.15ns	0.582ns	0.0446	0	281 B
#4435	`ObjectExtractorMoreComplexBody`	net6.0	2.99μs	0.872ns	3.14ns	0.0538	0	3.88 KB
#4435	`ObjectExtractorMoreComplexBody`	netcoreapp3.1	4.07μs	1.39ns	5.4ns	0.0504	0	3.78 KB
#4435	`ObjectExtractorMoreComplexBody`	net472	4.78μs	8.11ns	31.4ns	0.617	0.00718	3.89 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendRequest`	net6.0	169μs	93.5ns	362ns	0.167	18.08 KB
master	`SendRequest`	netcoreapp3.1	188μs	232ns	900ns	0.187	20.14 KB
master	`SendRequest`	net472	0.000576ns	0.000224ns	0.000837ns	0	0 b
#4435	`SendRequest`	net6.0	168μs	120ns	465ns	0.251	18.08 KB
#4435	`SendRequest`	netcoreapp3.1	189μs	269ns	1.04μs	0.19	20.15 KB
#4435	`SendRequest`	net472	0.000837ns	0.00041ns	0.00148ns	0	0 b

Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #4435

Benchmark	Base Allocated	Diff Allocated	Change	Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑netcoreapp3.1	41.66 KB	41.97 KB	307 B	0.74%

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Gen 2	Allocated
master	`WriteAndFlushEnrichedTraces`	net6.0	520μs	347ns	1.3μs	0.514	0	0	41.81 KB
master	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	618μs	865ns	3.35μs	0.309	0	0	41.66 KB
master	`WriteAndFlushEnrichedTraces`	net472	764μs	2.9μs	11.2μs	8.1	2.31	0.386	53.24 KB
#4435	`WriteAndFlushEnrichedTraces`	net6.0	512μs	267ns	1.04μs	0.768	0	0	41.67 KB
#4435	`WriteAndFlushEnrichedTraces`	netcoreapp3.1	636μs	1.85μs	7.16μs	0.312	0	0	41.97 KB
#4435	`WriteAndFlushEnrichedTraces`	net472	796μs	4.04μs	19μs	8.2	2.34	0.391	53.23 KB

Benchmarks.Trace.DbCommandBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #4435

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.DbCommandBenchmark.ExecuteNonQuery‑net6.0	1.219	880.03	1,072.87

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteNonQuery`	net6.0	880ns	0.181ns	0.702ns	0.0104	0	768 B
master	`ExecuteNonQuery`	netcoreapp3.1	1.23μs	0.457ns	1.77ns	0.0105	0	768 B
master	`ExecuteNonQuery`	net472	1.39μs	0.665ns	2.3ns	0.116	0.000694	730 B
#4435	`ExecuteNonQuery`	net6.0	1.07μs	0.413ns	1.6ns	0.0107	0	768 B
#4435	`ExecuteNonQuery`	netcoreapp3.1	1.19μs	0.384ns	1.49ns	0.0101	0	768 B
#4435	`ExecuteNonQuery`	net472	1.38μs	0.699ns	2.61ns	0.116	0	730 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`CallElasticsearch`	net6.0	1.1μs	0.408ns	1.58ns	0.0139	0	992 B
master	`CallElasticsearch`	netcoreapp3.1	1.21μs	0.327ns	1.18ns	0.0135	0	992 B
master	`CallElasticsearch`	net472	2.13μs	0.6ns	2.32ns	0.159	0.00107	1 KB
master	`CallElasticsearchAsync`	net6.0	1.07μs	0.57ns	2.21ns	0.0136	0	968 B
master	`CallElasticsearchAsync`	netcoreapp3.1	1.39μs	0.475ns	1.78ns	0.0139	0	1.04 KB
master	`CallElasticsearchAsync`	net472	2.24μs	0.337ns	1.31ns	0.167	0.00112	1.06 KB
#4435	`CallElasticsearch`	net6.0	1.05μs	0.366ns	1.37ns	0.0138	0	992 B
#4435	`CallElasticsearch`	netcoreapp3.1	1.27μs	0.329ns	1.19ns	0.0135	0	992 B
#4435	`CallElasticsearch`	net472	2.21μs	0.65ns	2.52ns	0.159	0	1 KB
#4435	`CallElasticsearchAsync`	net6.0	1.09μs	0.416ns	1.56ns	0.0136	0	968 B
#4435	`CallElasticsearchAsync`	netcoreapp3.1	1.39μs	0.358ns	1.29ns	0.0139	0	1.04 KB
#4435	`CallElasticsearchAsync`	net472	2.08μs	0.453ns	1.69ns	0.168	0.00105	1.06 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`ExecuteAsync`	net6.0	1.09μs	0.514ns	1.92ns	0.0127	0	912 B
master	`ExecuteAsync`	netcoreapp3.1	1.31μs	1.14ns	4.1ns	0.0122	0	912 B
master	`ExecuteAsync`	net472	1.47μs	0.833ns	3.22ns	0.139	0.000738	875 B
#4435	`ExecuteAsync`	net6.0	1.16μs	0.325ns	1.22ns	0.0127	0	912 B
#4435	`ExecuteAsync`	netcoreapp3.1	1.45μs	0.414ns	1.49ns	0.0121	0	912 B
#4435	`ExecuteAsync`	net472	1.53μs	0.686ns	2.47ns	0.138	0.000765	875 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`SendAsync`	net6.0	3.66μs	1.14ns	4.41ns	0.0273	1.94 KB
master	`SendAsync`	netcoreapp3.1	4.32μs	2.18ns	8.43ns	0.0323	2.48 KB
master	`SendAsync`	net472	6.97μs	1.67ns	6.45ns	0.483	3.05 KB
#4435	`SendAsync`	net6.0	3.55μs	2.77ns	10.7ns	0.0266	1.94 KB
#4435	`SendAsync`	netcoreapp3.1	4.36μs	1.47ns	5.52ns	0.0327	2.48 KB
#4435	`SendAsync`	net472	6.9μs	3.7ns	13.3ns	0.481	3.05 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	1.28μs	0.537ns	2.08ns	0.023	1.62 KB
master	`EnrichedLog`	netcoreapp3.1	1.85μs	0.59ns	2.21ns	0.0221	1.62 KB
master	`EnrichedLog`	net472	2.35μs	2.68ns	10.4ns	0.244	1.54 KB
#4435	`EnrichedLog`	net6.0	1.27μs	0.646ns	2.42ns	0.0228	1.62 KB
#4435	`EnrichedLog`	netcoreapp3.1	1.83μs	1.05ns	3.91ns	0.022	1.62 KB
#4435	`EnrichedLog`	net472	2.25μs	2.58ns	10ns	0.244	1.54 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`EnrichedLog`	net6.0	112μs	143ns	514ns	0.0556	0	4.21 KB
master	`EnrichedLog`	netcoreapp3.1	116μs	150ns	562ns	0.0585	0	4.21 KB
master	`EnrichedLog`	net472	147μs	135ns	523ns	0.658	0.219	4.38 KB
#4435	`EnrichedLog`	net6.0	112μs	159ns	617ns	0.0558	0	4.21 KB
#4435	`EnrichedLog`	netcoreapp3.1	117μs	232ns	900ns	0	0	4.21 KB
#4435	`EnrichedLog`	net472	146μs	152ns	587ns	0.653	0.218	4.38 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.73μs	1.1ns	3.98ns	0.03	2.18 KB
master	`EnrichedLog`	netcoreapp3.1	3.73μs	1.23ns	4.42ns	0.028	2.18 KB
master	`EnrichedLog`	net472	4.46μs	1.11ns	4.29ns	0.314	1.99 KB
#4435	`EnrichedLog`	net6.0	2.98μs	1.26ns	4.87ns	0.0297	2.18 KB
#4435	`EnrichedLog`	netcoreapp3.1	3.83μs	1.99ns	7.46ns	0.0287	2.18 KB
#4435	`EnrichedLog`	net472	4.47μs	0.804ns	3.01ns	0.314	1.99 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Gen 1	Allocated
master	`SendReceive`	net6.0	1.16μs	0.631ns	2.36ns	0.0163	0	1.16 KB
master	`SendReceive`	netcoreapp3.1	1.49μs	0.574ns	1.99ns	0.0157	0	1.16 KB
master	`SendReceive`	net472	1.75μs	0.778ns	2.91ns	0.185	0.00088	1.16 KB
#4435	`SendReceive`	net6.0	1.15μs	0.681ns	2.55ns	0.016	0	1.16 KB
#4435	`SendReceive`	netcoreapp3.1	1.49μs	0.429ns	1.49ns	0.0157	0	1.16 KB
#4435	`SendReceive`	net472	1.8μs	0.454ns	1.7ns	0.185	0	1.16 KB

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`EnrichedLog`	net6.0	2.39μs	1.03ns	3.98ns	0.0214	1.53 KB
master	`EnrichedLog`	netcoreapp3.1	3.65μs	1.09ns	4.09ns	0.0199	1.58 KB
master	`EnrichedLog`	net472	4.02μs	7.77ns	30.1ns	0.31	1.96 KB
#4435	`EnrichedLog`	net6.0	2.48μs	0.535ns	2ns	0.0212	1.53 KB
#4435	`EnrichedLog`	netcoreapp3.1	3.59μs	14.6ns	56.6ns	0.0212	1.58 KB
#4435	`EnrichedLog`	net472	3.95μs	1.13ns	4.36ns	0.31	1.96 KB

Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #4435

Benchmark	diff/base	Base Median (ns)	Diff Median (ns)	Modality
Benchmarks.Trace.SpanBenchmark.StartFinishSpan‑netcoreapp3.1	1.174	513.18	602.40
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0	1.155	448.46	517.82

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`StartFinishSpan`	net6.0	381ns	0.158ns	0.591ns	0.00756	536 B
master	`StartFinishSpan`	netcoreapp3.1	513ns	0.211ns	0.818ns	0.00732	536 B
master	`StartFinishSpan`	net472	595ns	0.183ns	0.707ns	0.0854	538 B
master	`StartFinishScope`	net6.0	449ns	0.17ns	0.637ns	0.00917	656 B
master	`StartFinishScope`	netcoreapp3.1	706ns	0.289ns	1.04ns	0.00885	656 B
master	`StartFinishScope`	net472	795ns	0.425ns	1.59ns	0.098	618 B
#4435	`StartFinishSpan`	net6.0	389ns	0.347ns	1.34ns	0.0076	536 B
#4435	`StartFinishSpan`	netcoreapp3.1	603ns	0.27ns	0.973ns	0.00725	536 B
#4435	`StartFinishSpan`	net472	577ns	0.14ns	0.525ns	0.0852	538 B
#4435	`StartFinishScope`	net6.0	518ns	0.22ns	0.794ns	0.00918	656 B
#4435	`StartFinishScope`	netcoreapp3.1	678ns	0.586ns	2.27ns	0.00876	656 B
#4435	`StartFinishScope`	net472	818ns	0.273ns	1.02ns	0.0978	618 B

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

Raw results

Branch	Method	Toolchain	Mean	StdError	StdDev	Gen 0	Allocated
master	`RunOnMethodBegin`	net6.0	566ns	0.169ns	0.633ns	0.0092	656 B
master	`RunOnMethodBegin`	netcoreapp3.1	806ns	0.208ns	0.752ns	0.00874	656 B
master	`RunOnMethodBegin`	net472	969ns	0.634ns	2.46ns	0.0981	618 B
#4435	`RunOnMethodBegin`	net6.0	596ns	0.156ns	0.606ns	0.00934	656 B
#4435	`RunOnMethodBegin`	netcoreapp3.1	792ns	0.802ns	3ns	0.00861	656 B
#4435	`RunOnMethodBegin`	net472	969ns	0.328ns	1.27ns	0.098	618 B

andrewlock · 2023-07-23T18:19:43Z

Throughput/Crank Report:zap:

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 (4435) (11.058M)   : 0, 11058185
    master (11.035M)   : 0, 11035132
    benchmarks/2.33.0 (11.157M)   : 0, 11156685
    benchmarks/2.9.0 (11.316M)   : 0, 11316157

    section Automatic
    This PR (4435) (7.788M)   : 0, 7787881
    master (7.818M)   : 0, 7818455
    benchmarks/2.33.0 (7.759M)   : 0, 7758975
    benchmarks/2.9.0 (8.200M)   : 0, 8199799

    section Trace stats
    master (7.834M)   : 0, 7833902
    benchmarks/2.33.0 (7.761M)   : 0, 7760502

    section Manual
    This PR (4435) (9.971M)   : 0, 9971459
    master (9.927M)   : 0, 9926837
    benchmarks/2.33.0 (9.991M)   : 0, 9991023

    section Manual + Automatic
    This PR (4435) (7.478M)   : 0, 7478418
    master (7.513M)   : 0, 7513466
    benchmarks/2.33.0 (7.517M)   : 0, 7517119

    section Version Conflict
    master (6.718M)   : 0, 6717538
    benchmarks/2.33.0 (6.674M)   : 0, 6674489

gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4435) (9.735M)   : 0, 9735007
    master (9.539M)   : 0, 9539380
    benchmarks/2.33.0 (9.547M)   : 0, 9547169
    benchmarks/2.9.0 (9.576M)   : 0, 9576191

    section Automatic
    This PR (4435) (6.931M)   : 0, 6930871
    master (6.778M)   : 0, 6778360
    benchmarks/2.33.0 (6.748M)   : 0, 6748132

    section Trace stats
    master (6.775M)   : 0, 6774718
    benchmarks/2.33.0 (6.679M)   : 0, 6678668

    section Manual
    This PR (4435) (8.494M)   : 0, 8494491
    master (8.384M)   : 0, 8384345
    benchmarks/2.33.0 (8.513M)   : 0, 8513138

    section Manual + Automatic
    This PR (4435) (6.467M)   : 0, 6466858
    master (6.590M)   : 0, 6589697
    benchmarks/2.33.0 (6.511M)   : 0, 6510702

    section Version Conflict
    master (5.774M)   : 0, 5774243
    benchmarks/2.33.0 (5.790M)   : 0, 5789875

gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4435) (10.354M)   : 0, 10353912
    master (10.359M)   : 0, 10358796
    benchmarks/2.33.0 (10.242M)   : 0, 10241972
    benchmarks/2.9.0 (10.251M)   : 0, 10250862

    section Automatic
    This PR (4435) (7.392M)   : 0, 7392137
    master (7.368M)   : 0, 7367777
    benchmarks/2.33.0 (7.374M)   : 0, 7373893
    benchmarks/2.9.0 (7.536M)   : 0, 7535754

    section Trace stats
    master (7.353M)   : 0, 7353241
    benchmarks/2.33.0 (7.277M)   : 0, 7277270

    section Manual
    This PR (4435) (9.543M)   : 0, 9543187
    master (9.127M)   : 0, 9127035
    benchmarks/2.33.0 (9.163M)   : 0, 9163360

    section Manual + Automatic
    This PR (4435) (7.288M)   : 0, 7287882
    master (7.155M)   : 0, 7155148
    benchmarks/2.33.0 (7.173M)   : 0, 7172915

    section Version Conflict
    master (6.471M)   : 0, 6471439
    benchmarks/2.33.0 (6.599M)   : 0, 6598779

gantt
    title Throughput Linux x64 (ASM) (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (4435) (7.028M)   : 0, 7027604
    master (7.346M)   : 0, 7345576
    benchmarks/2.33.0 (7.689M)   : 0, 7688992
    benchmarks/2.9.0 (7.836M)   : 0, 7835585

    section No attack
    This PR (4435) (2.062M)   : 0, 2061668
    master (2.118M)   : 0, 2117965
    benchmarks/2.33.0 (2.211M)   : 0, 2210506
    benchmarks/2.9.0 (3.245M)   : 0, 3244991

    section Attack
    This PR (4435) (1.792M)   : 0, 1792317
    master (1.824M)   : 0, 1823617
    benchmarks/2.33.0 (1.908M)   : 0, 1908362
    benchmarks/2.9.0 (2.520M)   : 0, 2519584

    section Blocking
    This PR (4435) (3.532M)   : crit ,0, 3531948
    master (3.742M)   : 0, 3741678
    benchmarks/2.33.0 (3.840M)   : 0, 3839512

pablomartinezbernardo

Let's determine the proper v1 operation name for these RPC-style spans. Maybe chane from service_remoting.client => service_remoting.client.request and service_remoting.server => service_remoting.server.request?

As discussed internally, completely agree. That might end up changing after UNC, but at this point it's the safest bet, and we can afford to introduce "breaking" changes with this v1 schema without consequence.

Should I add my client test application, even though we're not set up to run it in CI right now?

I would! Specially because this is not as straight forward to test as other integrations. Out of curiosity, what does your test app do different compared to the one we already have?

- Client spans: service_remoting.client => service_remoting.client.request - Server spans: service_remoting.server => service_remoting.server.request

zacharycmontoya · 2023-07-25T17:07:44Z

Let's determine the proper v1 operation name for these RPC-style spans. Maybe chane from service_remoting.client => service_remoting.client.request and service_remoting.server => service_remoting.server.request?

As discussed internally, completely agree. That might end up changing after UNC, but at this point it's the safest bet, and we can afford to introduce "breaking" changes with this v1 schema without consequence.

This has been updated in cff8c37

zacharycmontoya · 2023-07-25T17:08:53Z

Should I add my client test application, even though we're not set up to run it in CI right now?

I would! Specially because this is not as straight forward to test as other integrations. Out of curiosity, what does your test app do different compared to the one we already have?

I just realized that I should be able to re-use the application we already have 🤦🏼 I would like to do this as a follow-up PR so we can get the product changes in as soon as possible. I have unit tests to demonstrate the changes and have tested the Service Remoting client end-to-end so I'm confident in the changes as-is

bouwkast

Looks good to me!

zacharycmontoya added 4 commits July 20, 2023 19:58

Add Tracer.Instance.CurrentTraceSettings.Schema.RemapPeerService(tags…

2477f8e

…) in missing places

Update span metadata markdown files

c3de6af

zacharycmontoya added the status:do-not-merge Work is done. Can review, but do not merge yet. label Jul 23, 2023

zacharycmontoya requested review from a team as code owners July 23, 2023 17:23

pablomartinezbernardo reviewed Jul 24, 2023

View reviewed changes

For v1 spans, perform the following operation name changes:

cff8c37

- Client spans: service_remoting.client => service_remoting.client.request - Server spans: service_remoting.server => service_remoting.server.request

zacharycmontoya removed the status:do-not-merge Work is done. Can review, but do not merge yet. label Jul 25, 2023

bouwkast approved these changes Jul 25, 2023

View reviewed changes

pablomartinezbernardo approved these changes Jul 26, 2023

View reviewed changes

zacharycmontoya merged commit 7bb57cc into master Jul 26, 2023

zacharycmontoya deleted the zach/v1-schema/service-fabric-remoting branch July 26, 2023 22:23

github-actions bot added this to the vNext milestone Jul 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Tracer] Add peer.service to Service Remoting integration #4435

[Tracer] Add peer.service to Service Remoting integration #4435

zacharycmontoya commented Jul 23, 2023 •

edited

Loading

datadog-ddstaging bot commented Jul 23, 2023 •

edited

Loading

andrewlock commented Jul 23, 2023 •

edited

Loading

andrewlock commented Jul 23, 2023 •

edited

Loading

Raw results

Slower ⚠️ in #4435

Faster 🎉 in #4435

Raw results

Raw results

More allocations ⚠️ in #4435

Raw results

Slower ⚠️ in #4435

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #4435

Raw results

Raw results

andrewlock commented Jul 23, 2023 •

edited

Loading

pablomartinezbernardo left a comment

zacharycmontoya commented Jul 25, 2023

zacharycmontoya commented Jul 25, 2023

bouwkast left a comment

[Tracer] Add peer.service to Service Remoting integration #4435

[Tracer] Add peer.service to Service Remoting integration #4435

Conversation

zacharycmontoya commented Jul 23, 2023 • edited Loading

Summary of changes

Reason for change

Implementation details

Test coverage

Other details

datadog-ddstaging bot commented Jul 23, 2023 • edited Loading

Datadog Report

andrewlock commented Jul 23, 2023 • edited Loading

Execution-Time Benchmarks Report ⏱️

andrewlock commented Jul 23, 2023 • edited Loading

Benchmarks Report 🐌

Benchmark details

Raw results

Slower ⚠️ in #4435

Faster 🎉 in #4435

Raw results

Raw results

More allocations ⚠️ in #4435

Raw results

Slower ⚠️ in #4435

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Raw results

Slower ⚠️ in #4435

Raw results

Raw results

andrewlock commented Jul 23, 2023 • edited Loading

Throughput/Crank Report:zap:

pablomartinezbernardo left a comment

Choose a reason for hiding this comment

zacharycmontoya commented Jul 25, 2023

zacharycmontoya commented Jul 25, 2023

bouwkast left a comment

Choose a reason for hiding this comment

zacharycmontoya commented Jul 23, 2023 •

edited

Loading

datadog-ddstaging bot commented Jul 23, 2023 •

edited

Loading

andrewlock commented Jul 23, 2023 •

edited

Loading

andrewlock commented Jul 23, 2023 •

edited

Loading

andrewlock commented Jul 23, 2023 •

edited

Loading