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

[Tracing] Update telemetry metrics for OpenTelemetry integration #6454

Merged
merged 3 commits into from
Jan 10, 2025

Conversation

zacharycmontoya
Copy link
Collaborator

@zacharycmontoya zacharycmontoya commented Dec 17, 2024

Summary of changes

For the OpenTelemetry integration, update the tag generated for the span_created telemetry metric from integration_name:opentelemetry to integration_name:otel to align with other tracing libraries

Reason for change

Other tracing libraries generate span metrics using integration_name:otel.library or integration_name:otel to distinguish between OpenTelemetry Instrumentation Library usage or OpenTelemetry Custom Instrumentation usage. We cannot distinguish between the two, so we can simply use integration_name:otel

Implementation details

Update the static tag from integration_name:opentelemetry to integration_name:otel

Test coverage

This PR adds a test to assert the value of the integration_name tag attached to the span_creatted telemetry metric on all of our tracing integration tests. This was then used to test the updated tag value.

Other details

N/A

@github-actions github-actions bot added the area:tests unit tests, integration tests label Dec 17, 2024
@datadog-ddstaging
Copy link

datadog-ddstaging bot commented Dec 17, 2024

Datadog Report

Branch report: zach.montoya/otel-custom-instr-telemetry
Commit report: 716ae47
Test service: dd-trace-dotnet

✅ 0 Failed, 231476 Passed, 2070 Skipped, 19h 25m 32.75s Total Time
❄️ 1 New Flaky

New Flaky Tests (1)

  • EarlyFlakeDetection - Datadog.Trace.ClrProfiler.IntegrationTests.CI.NUnitEvpTests - Last Failure

    Expand for error
     The sample did not exit in 600000ms. Memory dump taken: True. Killing process.
    

@andrewlock
Copy link
Member

andrewlock commented Dec 17, 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 (6454) - mean (69ms)  : 66, 72
     .   : milestone, 69,
    master - mean (68ms)  : 66, 71
     .   : milestone, 68,

    section CallTarget+Inlining+NGEN
    This PR (6454) - mean (983ms)  : 959, 1007
     .   : milestone, 983,
    master - mean (979ms)  : 957, 1000
     .   : milestone, 979,

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

    section CallTarget+Inlining+NGEN
    This PR (6454) - mean (680ms)  : 666, 694
     .   : milestone, 680,
    master - mean (682ms)  : 668, 696
     .   : milestone, 682,

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

    section CallTarget+Inlining+NGEN
    This PR (6454) - mean (634ms)  : 618, 649
     .   : milestone, 634,
    master - mean (639ms)  : 625, 654
     .   : milestone, 639,

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

    section CallTarget+Inlining+NGEN
    This PR (6454) - mean (1,107ms)  : 1081, 1134
     .   : milestone, 1107,
    master - mean (1,099ms)  : 1062, 1136
     .   : milestone, 1099,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6454) - mean (280ms)  : 274, 286
     .   : milestone, 280,
    master - mean (278ms)  : 274, 283
     .   : milestone, 278,

    section CallTarget+Inlining+NGEN
    This PR (6454) - mean (872ms)  : 845, 899
     .   : milestone, 872,
    master - mean (869ms)  : 838, 899
     .   : milestone, 869,

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

    section CallTarget+Inlining+NGEN
    This PR (6454) - mean (853ms)  : 824, 883
     .   : milestone, 853,
    master - mean (845ms)  : 814, 877
     .   : milestone, 845,

Loading

@andrewlock
Copy link
Member

andrewlock commented Dec 18, 2024

Benchmarks Report for tracer 🐌

Benchmarks for #6454 compared to master:

  • 2 benchmarks are slower, with geometric mean 1.116
  • 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.03μs 45ns 302ns 0.0117 0.00389 0 5.62 KB
master StartStopWithChild netcoreapp3.1 10μs 53.6ns 289ns 0.0214 0.0107 0 5.8 KB
master StartStopWithChild net472 16.4μs 51.5ns 199ns 1.06 0.336 0.112 6.2 KB
#6454 StartStopWithChild net6.0 7.79μs 44ns 317ns 0.0159 0.00795 0 5.61 KB
#6454 StartStopWithChild netcoreapp3.1 10.6μs 57.2ns 308ns 0.0209 0.00522 0 5.8 KB
#6454 StartStopWithChild net472 16.6μs 51.4ns 199ns 1.02 0.286 0.0758 6.19 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 467μs 304ns 1.14μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 662μs 400ns 1.5μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 858μs 461ns 1.6μs 0.428 0 0 3.3 KB
#6454 WriteAndFlushEnrichedTraces net6.0 479μs 245ns 948ns 0 0 0 2.7 KB
#6454 WriteAndFlushEnrichedTraces netcoreapp3.1 640μs 467ns 1.81μs 0 0 0 2.7 KB
#6454 WriteAndFlushEnrichedTraces net472 862μs 871ns 3.37μs 0.428 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 151μs 998ns 9.88μs 0.147 0 0 14.47 KB
master SendRequest netcoreapp3.1 172μs 934ns 5.45μs 0.168 0 0 17.27 KB
master SendRequest net472 0.00361ns 0.00116ns 0.0045ns 0 0 0 0 b
#6454 SendRequest net6.0 151μs 883ns 8.04μs 0.144 0 0 14.47 KB
#6454 SendRequest netcoreapp3.1 169μs 968ns 8.71μs 0.164 0 0 17.27 KB
#6454 SendRequest net472 0.00381ns 0.0014ns 0.00542ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ Fewer allocations 🎉

Fewer allocations 🎉 in #6454

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 41.7 KB 41.41 KB -286 B -0.69%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 567μs 3.15μs 19.9μs 0.568 0 0 41.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 674μs 3.81μs 25.9μs 0.329 0 0 41.9 KB
master WriteAndFlushEnrichedTraces net472 843μs 3.93μs 16.2μs 8.5 2.55 0.425 53.27 KB
#6454 WriteAndFlushEnrichedTraces net6.0 546μs 2.92μs 15.7μs 0.563 0 0 41.41 KB
#6454 WriteAndFlushEnrichedTraces netcoreapp3.1 693μs 3.86μs 23.5μs 0.322 0 0 41.81 KB
#6454 WriteAndFlushEnrichedTraces net472 851μs 3.7μs 14.3μs 8.25 2.6 0.434 53.29 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.41μs 1.21ns 4.69ns 0.0141 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.78μs 3.06ns 11.8ns 0.0133 0 0 1.02 KB
master ExecuteNonQuery net472 2.1μs 3.9ns 15.1ns 0.156 0.00104 0 987 B
#6454 ExecuteNonQuery net6.0 1.38μs 1.02ns 3.94ns 0.0145 0 0 1.02 KB
#6454 ExecuteNonQuery netcoreapp3.1 1.92μs 1.79ns 6.94ns 0.0132 0 0 1.02 KB
#6454 ExecuteNonQuery net472 1.95μs 3.35ns 13ns 0.156 0.000976 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.32μs 1.1ns 3.98ns 0.0138 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.65μs 1.37ns 5.31ns 0.0133 0 0 976 B
master CallElasticsearch net472 2.45μs 1.05ns 3.79ns 0.158 0 0 995 B
master CallElasticsearchAsync net6.0 1.36μs 0.553ns 2.14ns 0.013 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.65μs 1.68ns 6.52ns 0.0133 0 0 1.02 KB
master CallElasticsearchAsync net472 2.53μs 1.59ns 5.74ns 0.167 0 0 1.05 KB
#6454 CallElasticsearch net6.0 1.21μs 0.623ns 2.33ns 0.0133 0 0 976 B
#6454 CallElasticsearch netcoreapp3.1 1.52μs 1.03ns 3.56ns 0.0132 0 0 976 B
#6454 CallElasticsearch net472 2.52μs 1.99ns 7.46ns 0.158 0 0 995 B
#6454 CallElasticsearchAsync net6.0 1.36μs 0.454ns 1.7ns 0.013 0 0 952 B
#6454 CallElasticsearchAsync netcoreapp3.1 1.73μs 0.636ns 2.38ns 0.0136 0 0 1.02 KB
#6454 CallElasticsearchAsync net472 2.65μs 2.11ns 8.18ns 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.33μs 0.938ns 3.51ns 0.0133 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.63μs 0.764ns 2.86ns 0.0128 0 0 952 B
master ExecuteAsync net472 1.81μs 0.551ns 2.13ns 0.145 0 0 915 B
#6454 ExecuteAsync net6.0 1.29μs 0.408ns 1.53ns 0.0135 0 0 952 B
#6454 ExecuteAsync netcoreapp3.1 1.63μs 0.92ns 3.44ns 0.0123 0 0 952 B
#6454 ExecuteAsync net472 1.82μs 5.39ns 20.9ns 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.6μs 1.17ns 4.37ns 0.0324 0 0 2.31 KB
master SendAsync netcoreapp3.1 5.19μs 1.14ns 3.96ns 0.0364 0 0 2.85 KB
master SendAsync net472 7.41μs 1.18ns 4.42ns 0.495 0 0 3.12 KB
#6454 SendAsync net6.0 4.43μs 1.59ns 5.96ns 0.0308 0 0 2.31 KB
#6454 SendAsync netcoreapp3.1 5.22μs 1.85ns 7.17ns 0.0392 0 0 2.85 KB
#6454 SendAsync net472 7.39μs 1.72ns 6.42ns 0.495 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.48μs 0.71ns 2.75ns 0.023 0 0 1.64 KB
master EnrichedLog netcoreapp3.1 2.17μs 1.07ns 3.88ns 0.0223 0 0 1.64 KB
master EnrichedLog net472 2.71μs 1.17ns 4.39ns 0.249 0 0 1.57 KB
#6454 EnrichedLog net6.0 1.46μs 0.497ns 1.92ns 0.0228 0 0 1.64 KB
#6454 EnrichedLog netcoreapp3.1 2.32μs 5.2ns 20.1ns 0.0218 0 0 1.64 KB
#6454 EnrichedLog net472 2.76μs 2.88ns 11.1ns 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 117μs 133ns 514ns 0.0583 0 0 4.28 KB
master EnrichedLog netcoreapp3.1 122μs 164ns 637ns 0 0 0 4.28 KB
master EnrichedLog net472 153μs 208ns 806ns 0.687 0.229 0 4.46 KB
#6454 EnrichedLog net6.0 117μs 216ns 837ns 0 0 0 4.28 KB
#6454 EnrichedLog netcoreapp3.1 123μs 121ns 453ns 0 0 0 4.28 KB
#6454 EnrichedLog net472 150μs 70.8ns 274ns 0.673 0.224 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.03μs 0.901ns 3.49ns 0.0304 0 0 2.2 KB
master EnrichedLog netcoreapp3.1 4.2μs 2.1ns 8.13ns 0.0293 0 0 2.2 KB
master EnrichedLog net472 4.96μs 1.2ns 4.64ns 0.32 0 0 2.02 KB
#6454 EnrichedLog net6.0 2.99μs 0.881ns 3.41ns 0.0301 0 0 2.2 KB
#6454 EnrichedLog netcoreapp3.1 4.18μs 2.25ns 8.71ns 0.0291 0 0 2.2 KB
#6454 EnrichedLog net472 4.82μs 1.54ns 5.97ns 0.319 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.32μs 0.64ns 2.39ns 0.0164 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.79μs 0.897ns 3.47ns 0.0152 0 0 1.14 KB
master SendReceive net472 2.27μs 1.28ns 4.94ns 0.183 0 0 1.16 KB
#6454 SendReceive net6.0 1.34μs 0.639ns 2.47ns 0.0161 0 0 1.14 KB
#6454 SendReceive netcoreapp3.1 1.76μs 0.469ns 1.76ns 0.0149 0 0 1.14 KB
#6454 SendReceive net472 2.12μs 1.88ns 7.27ns 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.69μs 0.851ns 3.18ns 0.0215 0 0 1.6 KB
master EnrichedLog netcoreapp3.1 3.86μs 2.07ns 8ns 0.0211 0 0 1.65 KB
master EnrichedLog net472 4.4μs 2.8ns 10.9ns 0.322 0 0 2.04 KB
#6454 EnrichedLog net6.0 2.75μs 1.11ns 4.29ns 0.0221 0 0 1.6 KB
#6454 EnrichedLog netcoreapp3.1 3.97μs 14.1ns 54.8ns 0.0218 0 0 1.65 KB
#6454 EnrichedLog net472 4.44μs 3.97ns 15.4ns 0.322 0 0 2.04 KB
Benchmarks.Trace.SpanBenchmark - Slower ⚠️ Same allocations ✔️

Slower ⚠️ in #6454

Benchmark diff/base Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑netcoreapp3.1 1.116 664.64 741.98
Benchmarks.Trace.SpanBenchmark.StartFinishScope‑net6.0 1.116 479.27 534.96

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 396ns 0.625ns 2.42ns 0.00798 0 0 576 B
master StartFinishSpan netcoreapp3.1 603ns 0.964ns 3.61ns 0.00763 0 0 576 B
master StartFinishSpan net472 625ns 0.828ns 3.21ns 0.0917 0 0 578 B
master StartFinishScope net6.0 479ns 0.53ns 1.98ns 0.00975 0 0 696 B
master StartFinishScope netcoreapp3.1 663ns 1.2ns 4.63ns 0.00964 0 0 696 B
master StartFinishScope net472 863ns 1.6ns 6.2ns 0.104 0 0 658 B
#6454 StartFinishSpan net6.0 406ns 0.325ns 1.26ns 0.00811 0 0 576 B
#6454 StartFinishSpan netcoreapp3.1 572ns 0.388ns 1.5ns 0.00772 0 0 576 B
#6454 StartFinishSpan net472 631ns 0.416ns 1.61ns 0.0917 0 0 578 B
#6454 StartFinishScope net6.0 536ns 0.544ns 2.11ns 0.00967 0 0 696 B
#6454 StartFinishScope netcoreapp3.1 741ns 0.765ns 2.96ns 0.00935 0 0 696 B
#6454 StartFinishScope net472 801ns 0.571ns 2.21ns 0.105 0 0 658 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 595ns 0.276ns 1.03ns 0.00982 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 952ns 0.563ns 2.18ns 0.00909 0 0 696 B
master RunOnMethodBegin net472 1.1μs 0.24ns 0.928ns 0.104 0 0 658 B
#6454 RunOnMethodBegin net6.0 646ns 0.449ns 1.74ns 0.00978 0 0 696 B
#6454 RunOnMethodBegin netcoreapp3.1 911ns 0.592ns 2.22ns 0.0092 0 0 696 B
#6454 RunOnMethodBegin net472 1.11μs 0.822ns 3.18ns 0.104 0 0 658 B

@zacharycmontoya zacharycmontoya force-pushed the zach.montoya/otel-custom-instr-telemetry branch from 58f7387 to 7a16426 Compare December 18, 2024 22:30
@zacharycmontoya zacharycmontoya changed the title Update tags for spans_created metrics [Tracing] Update telemetry metrics for OpenTelemtetry integration Dec 18, 2024
@andrewlock
Copy link
Member

andrewlock commented Dec 19, 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 (6454) (11.172M)   : 0, 11171561
    master (11.166M)   : 0, 11165619
    benchmarks/2.9.0 (11.033M)   : 0, 11032866

    section Automatic
    This PR (6454) (7.226M)   : 0, 7226315
    master (7.232M)   : 0, 7232435
    benchmarks/2.9.0 (7.786M)   : 0, 7785853

    section Trace stats
    master (7.555M)   : 0, 7555239

    section Manual
    master (11.218M)   : 0, 11218357

    section Manual + Automatic
    This PR (6454) (6.688M)   : 0, 6687605
    master (6.695M)   : 0, 6694720

    section DD_TRACE_ENABLED=0
    master (10.195M)   : 0, 10194994

Loading
gantt
    title Throughput Linux arm64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6454) (9.495M)   : 0, 9495307
    master (9.350M)   : 0, 9350086
    benchmarks/2.9.0 (9.495M)   : 0, 9494821

    section Automatic
    This PR (6454) (6.375M)   : 0, 6375065
    master (6.223M)   : 0, 6222880

    section Trace stats
    master (6.644M)   : 0, 6644302

    section Manual
    master (9.490M)   : 0, 9490052

    section Manual + Automatic
    This PR (6454) (5.910M)   : 0, 5909686
    master (5.826M)   : 0, 5826117

    section DD_TRACE_ENABLED=0
    master (8.806M)   : 0, 8806093

Loading
gantt
    title Throughput Windows x64 (Total requests) 
    dateFormat  X
    axisFormat %s
    section Baseline
    This PR (6454) (9.903M)   : 0, 9902503
    master (10.038M)   : 0, 10038108
    benchmarks/2.9.0 (10.020M)   : 0, 10019592

    section Automatic
    This PR (6454) (6.509M)   : 0, 6509367
    master (6.759M)   : 0, 6759031
    benchmarks/2.9.0 (7.255M)   : 0, 7255257

    section Trace stats
    master (7.327M)   : 0, 7327150

    section Manual
    master (10.148M)   : 0, 10147695

    section Manual + Automatic
    This PR (6454) (5.872M)   : crit ,0, 5872352
    master (6.270M)   : 0, 6270298

    section DD_TRACE_ENABLED=0
    master (9.572M)   : 0, 9571995

Loading

{
spansCreatedByIntegration.Should().NotBeEmpty();

var spansCreated = spansCreatedByIntegration.Should().ContainKey($"integration_name:{integrationNameTagValue}").WhoseValue;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This adds a new check (for all our integration tests!) that when the integration is enabled we also receive a spans_created metric that corresponds to the enabled integration.

The "otel" check above shows how this is the only integration whose tag value differs from its corresponding IntegrationId.ToString() 😅

Copy link
Member

Choose a reason for hiding this comment

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

I have a mild concern that this will overall increase flakiness in the tests. Telemetry is one of the areas most likely to flake, because it is typically sent at shutdown in our small samples, and that's where there are known bugs in the runtime etc.

If we do see flake around this, I'd suggest that most of this check is (or could be) checked with unit tests already 🙂

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Acknowledged, we can keep a close eye on it post-merge 👍🏼

@zacharycmontoya zacharycmontoya force-pushed the zach.montoya/otel-custom-instr-telemetry branch 2 times, most recently from b2025e7 to cb0a799 Compare December 20, 2024 19:20
@zacharycmontoya zacharycmontoya changed the title [Tracing] Update telemetry metrics for OpenTelemtetry integration [Tracing] Update telemetry metrics for OpenTelemetry integration Jan 2, 2025
…sserting they have the tag "integration_name:IntegrationId.ToString()"
…pan_created telemetry metric from `integration_name:opentelemetry` to `integration_name:otel` to align with other tracing libraries
…erTests so that we can accurately test the metrics that are generated when TelemetryController.IntegrationGeneratedSpan(IntegrationId) is invoked in product code, which generates both a spans_created telemetry metric and an updated integration telemetry
@zacharycmontoya zacharycmontoya force-pushed the zach.montoya/otel-custom-instr-telemetry branch from 6232fd9 to 716ae47 Compare January 9, 2025 16:45
@zacharycmontoya zacharycmontoya marked this pull request as ready for review January 9, 2025 21:06
@zacharycmontoya zacharycmontoya requested review from a team as code owners January 9, 2025 21:06
Comment on lines +189 to +193
var integrationNameTagValue = integrationName switch
{
"OpenTelemetry" => "otel",
_ => integrationName.ToLowerInvariant(),
};
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if you want to do this or not, because this grabs directly from the metric tag:

Suggested change
var integrationNameTagValue = integrationName switch
{
"OpenTelemetry" => "otel",
_ => integrationName.ToLowerInvariant(),
};
var integrationNameTagValue = integrationId.GetMetricTag()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking of not doing this, so that we could write the assertion without invoking the exact product code. So this "otel" or convert from integration name seemed like a better test case

{
spansCreatedByIntegration.Should().NotBeEmpty();

var spansCreated = spansCreatedByIntegration.Should().ContainKey($"integration_name:{integrationNameTagValue}").WhoseValue;
Copy link
Member

Choose a reason for hiding this comment

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

I have a mild concern that this will overall increase flakiness in the tests. Telemetry is one of the areas most likely to flake, because it is typically sent at shutdown in our small samples, and that's where there are known bugs in the runtime etc.

If we do see flake around this, I'd suggest that most of this check is (or could be) checked with unit tests already 🙂

@zacharycmontoya zacharycmontoya merged commit 40b9770 into master Jan 10, 2025
102 of 104 checks passed
@zacharycmontoya zacharycmontoya deleted the zach.montoya/otel-custom-instr-telemetry branch January 10, 2025 22:15
@github-actions github-actions bot added this to the vNext-v3 milestone Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:tests unit tests, integration tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants