Skip to content

Remove instrumentation for Protobuf Schema tagging #6792

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 21, 2025
Merged

Remove instrumentation for Protobuf Schema tagging #6792

merged 1 commit into from
Mar 21, 2025

Conversation

@bouwkast
Copy link
Collaborator

@bouwkast bouwkast commented Mar 20, 2025

Summary of changes

Attempts to remove the Protobuf Schema tagging instrumentation that was recently added in #6647

Recommended to revert this PR when we do the fix.

Reason for change

Appears to be throwing errors due to MissingMethodExceptions on the DuckTypes.
We need time to investigate it and fix.
This seems to be the simplest approach as there were multiple PRs.

I don't see any code that hits this now so should be good.

Additionally, the Sample project is very flaky I attempted to remove that code generating the proto file.

Implementation details

  • Comment out instrumented method attributes to disable instrumentations and leave them there for less confusion (maybe?)
  • Remove Proto file generation in sample which I think is the cause of the CI build error
  • Skip tests in CI.

Test coverage

  • Skipped now.

Other details

@bouwkast
Remove instrumentation for Protobuf Schema tagging
20435b9 
Currently throwing MissingMethodExceptions
@bouwkast bouwkast requested review from a team as code owners March 20, 2025 20:02
@andrewlock
Copy link
Member

andrewlock commented Mar 20, 2025

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 (6792) - mean (69ms)  : 65, 73
     .   : milestone, 69,
    master - mean (69ms)  : 66, 73
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6792) - mean (1,005ms)  : 982, 1028
     .   : milestone, 1005,
    master - mean (1,004ms)  : 985, 1023
     .   : milestone, 1004,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6792) - mean (103ms)  : 100, 105
     .   : milestone, 103,
    master - mean (102ms)  : 100, 104
     .   : milestone, 102,

    section CallTarget+Inlining+NGEN
    This PR (6792) - mean (688ms)  : 668, 708
     .   : milestone, 688,
    master - mean (687ms)  : 665, 709
     .   : milestone, 687,
Loading 
gantt
    title Execution time (ms) FakeDbCommand (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6792) - mean (89ms)  : 88, 91
     .   : milestone, 89,
    master - mean (89ms)  : 87, 91
     .   : milestone, 89,

    section CallTarget+Inlining+NGEN
    This PR (6792) - mean (645ms)  : 629, 661
     .   : milestone, 645,
    master - mean (643ms)  : 623, 662
     .   : milestone, 643,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6792) - mean (190ms)  : 186, 194
     .   : milestone, 190,
    master - mean (190ms)  : 186, 194
     .   : milestone, 190,

    section CallTarget+Inlining+NGEN
    This PR (6792) - mean (1,112ms)  : 1085, 1138
     .   : milestone, 1112,
    master - mean (1,111ms)  : 1084, 1138
     .   : milestone, 1111,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6792) - mean (270ms)  : 263, 277
     .   : milestone, 270,
    master - mean (269ms)  : 265, 274
     .   : milestone, 269,

    section CallTarget+Inlining+NGEN
    This PR (6792) - mean (875ms)  : 847, 902
     .   : milestone, 875,
    master - mean (876ms)  : 850, 901
     .   : milestone, 876,
Loading 
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6792) - mean (262ms)  : 258, 265
     .   : milestone, 262,
    master - mean (262ms)  : 259, 265
     .   : milestone, 262,

    section CallTarget+Inlining+NGEN
    This PR (6792) - mean (858ms)  : 829, 888
     .   : milestone, 858,
    master - mean (863ms)  : 823, 903
     .   : milestone, 863,
Loading

@andrewlock
Copy link
Member

andrewlock commented Mar 20, 2025

Benchmarks Report for tracer 🐌

Benchmarks for #6792 compared to master:

  • 1 benchmarks are faster, with geometric mean 1.131
  • 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.2μs 43.3ns 225ns 0.0159 0.00797 0 5.61 KB
master StartStopWithChild netcoreapp3.1 10.4μs 57.2ns 319ns 0.00997 0 0 5.81 KB
master StartStopWithChild net472 16.1μs 57.3ns 222ns 1.03 0.299 0.097 6.22 KB
#6792 StartStopWithChild net6.0 7.86μs 44.6ns 322ns 0.0163 0.00815 0 5.61 KB
#6792 StartStopWithChild netcoreapp3.1 10.4μs 59ns 429ns 0.0243 0.00974 0 5.8 KB
#6792 StartStopWithChild net472 16.2μs 40.9ns 153ns 1.05 0.321 0.0962 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 500μs 714ns 2.77μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 640μs 669ns 2.5μs 0 0 0 2.7 KB
master WriteAndFlushEnrichedTraces net472 853μs 1.03μs 4μs 0.425 0 0 3.3 KB
#6792 WriteAndFlushEnrichedTraces net6.0 497μs 723ns 2.7μs 0 0 0 2.7 KB
#6792 WriteAndFlushEnrichedTraces netcoreapp3.1 646μs 693ns 2.59μs 0 0 0 2.7 KB
#6792 WriteAndFlushEnrichedTraces net472 869μs 906ns 3.51μs 0.431 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 128μs 449ns 1.74μs 0.189 0 0 14.47 KB
master SendRequest netcoreapp3.1 144μs 243ns 942ns 0.217 0 0 17.27 KB
master SendRequest net472 0.000649ns 0.000312ns 0.00121ns 0 0 0 0 b
#6792 SendRequest net6.0 128μs 412ns 1.6μs 0.19 0 0 14.47 KB
#6792 SendRequest netcoreapp3.1 145μs 373ns 1.45μs 0.21 0 0 17.27 KB
#6792 SendRequest net472 0.000434ns 0.000205ns 0.000766ns 0 0 0 0 b
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark - Same speed ✔️ More allocations ⚠️

More allocations ⚠️ in #6792

Benchmark Base Allocated Diff Allocated Change Change %
Benchmarks.Trace.CIVisibilityProtocolWriterBenchmark.WriteAndFlushEnrichedTraces‑net6.0 41.55 KB 41.79 KB 242 B 0.58%

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master WriteAndFlushEnrichedTraces net6.0 575μs 3.04μs 16.3μs 0.563 0 0 41.55 KB
master WriteAndFlushEnrichedTraces netcoreapp3.1 695μs 3.98μs 32.8μs 0.319 0 0 41.88 KB
master WriteAndFlushEnrichedTraces net472 849μs 3.93μs 15.7μs 8.69 2.48 0.414 53.29 KB
#6792 WriteAndFlushEnrichedTraces net6.0 588μs 3.2μs 17.8μs 0.576 0 0 41.79 KB
#6792 WriteAndFlushEnrichedTraces netcoreapp3.1 708μs 4.06μs 32μs 0.365 0 0 41.73 KB
#6792 WriteAndFlushEnrichedTraces net472 859μs 3.39μs 13.1μs 8.13 2.57 0.428 53.32 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.33μs 1.53ns 5.74ns 0.0146 0 0 1.02 KB
master ExecuteNonQuery netcoreapp3.1 1.76μs 1.21ns 4.54ns 0.0131 0 0 1.02 KB
master ExecuteNonQuery net472 2μs 3.77ns 14.6ns 0.157 0.000998 0 987 B
#6792 ExecuteNonQuery net6.0 1.28μs 0.895ns 3.47ns 0.014 0 0 1.02 KB
#6792 ExecuteNonQuery netcoreapp3.1 1.82μs 3.4ns 13.2ns 0.0136 0 0 1.02 KB
#6792 ExecuteNonQuery net472 2.06μs 2.41ns 9.32ns 0.157 0.00104 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.25μs 0.825ns 3.19ns 0.0133 0 0 976 B
master CallElasticsearch netcoreapp3.1 1.55μs 0.772ns 2.89ns 0.0132 0 0 976 B
master CallElasticsearch net472 2.56μs 1.4ns 5.42ns 0.157 0 0 995 B
master CallElasticsearchAsync net6.0 1.36μs 0.503ns 1.74ns 0.0128 0 0 952 B
master CallElasticsearchAsync netcoreapp3.1 1.64μs 2.5ns 9.7ns 0.0139 0 0 1.02 KB
master CallElasticsearchAsync net472 2.76μs 1.28ns 4.95ns 0.166 0 0 1.05 KB
#6792 CallElasticsearch net6.0 1.18μs 3.9ns 14.6ns 0.0132 0 0 976 B
#6792 CallElasticsearch netcoreapp3.1 1.52μs 0.692ns 2.59ns 0.0129 0 0 976 B
#6792 CallElasticsearch net472 2.69μs 1.31ns 5.07ns 0.157 0 0 995 B
#6792 CallElasticsearchAsync net6.0 1.29μs 0.998ns 3.73ns 0.0134 0 0 952 B
#6792 CallElasticsearchAsync netcoreapp3.1 1.69μs 0.693ns 2.59ns 0.0136 0 0 1.02 KB
#6792 CallElasticsearchAsync net472 2.61μs 0.732ns 2.74ns 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.24μs 0.398ns 1.43ns 0.0131 0 0 952 B
master ExecuteAsync netcoreapp3.1 1.72μs 0.545ns 2.04ns 0.0129 0 0 952 B
master ExecuteAsync net472 1.76μs 0.474ns 1.77ns 0.145 0 0 915 B
#6792 ExecuteAsync net6.0 1.33μs 1.01ns 3.93ns 0.0133 0 0 952 B
#6792 ExecuteAsync netcoreapp3.1 1.68μs 0.603ns 2.09ns 0.0126 0 0 952 B
#6792 ExecuteAsync net472 1.82μs 0.258ns 0.93ns 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.33μs 2.46ns 9.19ns 0.0326 0 0 2.31 KB
master SendAsync netcoreapp3.1 5.53μs 2.97ns 11.5ns 0.0386 0 0 2.85 KB
master SendAsync net472 7.53μs 2.65ns 10.2ns 0.493 0 0 3.12 KB
#6792 SendAsync net6.0 4.46μs 2.27ns 8.8ns 0.0314 0 0 2.31 KB
#6792 SendAsync netcoreapp3.1 5.41μs 1.95ns 7.56ns 0.0373 0 0 2.85 KB
#6792 SendAsync net472 7.45μs 1.06ns 3.95ns 0.494 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.54μs 1.34ns 5.03ns 0.024 0 0 1.69 KB
master EnrichedLog netcoreapp3.1 2.16μs 1.12ns 4.19ns 0.0221 0 0 1.69 KB
master EnrichedLog net472 2.6μs 1.48ns 5.75ns 0.258 0 0 1.62 KB
#6792 EnrichedLog net6.0 1.51μs 0.536ns 2ns 0.0234 0 0 1.69 KB
#6792 EnrichedLog netcoreapp3.1 2.17μs 1.38ns 5.15ns 0.0228 0 0 1.69 KB
#6792 EnrichedLog net472 2.67μs 1.15ns 4.46ns 0.258 0 0 1.62 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 112μs 211ns 816ns 0.0558 0 0 4.3 KB
master EnrichedLog netcoreapp3.1 116μs 134ns 501ns 0 0 0 4.3 KB
master EnrichedLog net472 148μs 165ns 640ns 0.669 0.223 0 4.49 KB
#6792 EnrichedLog net6.0 112μs 150ns 581ns 0.0563 0 0 4.3 KB
#6792 EnrichedLog netcoreapp3.1 116μs 239ns 925ns 0.0575 0 0 4.3 KB
#6792 EnrichedLog net472 150μs 146ns 565ns 0.673 0.224 0 4.49 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.16μs 1.7ns 6.13ns 0.0316 0 0 2.25 KB
master EnrichedLog netcoreapp3.1 4.09μs 0.812ns 2.81ns 0.0308 0 0 2.25 KB
master EnrichedLog net472 4.87μs 2.45ns 9.15ns 0.328 0 0 2.07 KB
#6792 EnrichedLog net6.0 3.04μs 1.24ns 4.82ns 0.0319 0 0 2.25 KB
#6792 EnrichedLog netcoreapp3.1 4.27μs 1.87ns 6.98ns 0.0298 0 0 2.25 KB
#6792 EnrichedLog net472 4.94μs 1.72ns 6.45ns 0.328 0 0 2.07 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.31μs 0.571ns 2.14ns 0.0158 0 0 1.14 KB
master SendReceive netcoreapp3.1 1.79μs 1.27ns 4.92ns 0.0153 0 0 1.14 KB
master SendReceive net472 2.09μs 0.933ns 3.61ns 0.183 0 0 1.16 KB
#6792 SendReceive net6.0 1.42μs 0.777ns 3.01ns 0.0156 0 0 1.14 KB
#6792 SendReceive netcoreapp3.1 1.81μs 0.797ns 3.09ns 0.0153 0 0 1.14 KB
#6792 SendReceive net472 2.11μs 1.66ns 6.42ns 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.86μs 0.947ns 3.67ns 0.0228 0 0 1.62 KB
master EnrichedLog netcoreapp3.1 4.07μs 1.32ns 4.95ns 0.0223 0 0 1.67 KB
master EnrichedLog net472 4.34μs 2.43ns 9.4ns 0.328 0 0 2.07 KB
#6792 EnrichedLog net6.0 2.69μs 0.831ns 3.11ns 0.0228 0 0 1.62 KB
#6792 EnrichedLog netcoreapp3.1 3.92μs 1.6ns 5.97ns 0.0215 0 0 1.67 KB
#6792 EnrichedLog net472 4.39μs 1.88ns 7.02ns 0.328 0 0 2.07 KB
Benchmarks.Trace.SpanBenchmark - Same speed ✔️ Same allocations ✔️

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master StartFinishSpan net6.0 420ns 0.129ns 0.481ns 0.00798 0 0 576 B
master StartFinishSpan netcoreapp3.1 589ns 0.345ns 1.29ns 0.00748 0 0 576 B
master StartFinishSpan net472 674ns 0.719ns 2.79ns 0.0918 0 0 578 B
master StartFinishScope net6.0 473ns 0.0729ns 0.273ns 0.0098 0 0 696 B
master StartFinishScope netcoreapp3.1 760ns 0.543ns 2.1ns 0.00933 0 0 696 B
master StartFinishScope net472 855ns 0.228ns 0.883ns 0.104 0 0 658 B
#6792 StartFinishSpan net6.0 405ns 0.0768ns 0.297ns 0.00814 0 0 576 B
#6792 StartFinishSpan netcoreapp3.1 622ns 0.182ns 0.705ns 0.00769 0 0 576 B
#6792 StartFinishSpan net472 638ns 0.116ns 0.418ns 0.0918 0 0 578 B
#6792 StartFinishScope net6.0 493ns 0.0516ns 0.186ns 0.00972 0 0 696 B
#6792 StartFinishScope netcoreapp3.1 694ns 0.552ns 2.14ns 0.0093 0 0 696 B
#6792 StartFinishScope net472 808ns 0.464ns 1.8ns 0.104 0 0 658 B
Benchmarks.Trace.TraceAnnotationsBenchmark - Faster 🎉 Same allocations ✔️

Faster 🎉 in #6792

Benchmark base/diff Base Median (ns) Diff Median (ns) Modality
Benchmarks.Trace.TraceAnnotationsBenchmark.RunOnMethodBegin‑net6.0 1.131 669.70 592.18

Raw results

Branch Method Toolchain Mean StdError StdDev Gen 0 Gen 1 Gen 2 Allocated
master RunOnMethodBegin net6.0 670ns 0.208ns 0.804ns 0.00975 0 0 696 B
master RunOnMethodBegin netcoreapp3.1 958ns 0.361ns 1.35ns 0.00909 0 0 696 B
master RunOnMethodBegin net472 1.05μs 0.445ns 1.67ns 0.104 0 0 658 B
#6792 RunOnMethodBegin net6.0 592ns 0.139ns 0.501ns 0.00985 0 0 696 B
#6792 RunOnMethodBegin netcoreapp3.1 948ns 0.654ns 2.53ns 0.00942 0 0 696 B
#6792 RunOnMethodBegin net472 1.11μs 0.183ns 0.635ns 0.104 0 0 658 B

@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Mar 20, 2025 •
edited
Loading

Datadog Report

All test runs 20435b9 🔗

✅ 2 Total Test Services: 0 Failed, 2 Passed
❄️ 1 with New Flaky

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Test Service View
dd-trace-dotnet 0 0 1 573400 5586 50h 4m 57.97s Link
exploration_tests 0 0 0 22085 3 1h 28m 59.15s Link

@lucaspimentel lucaspimentel requested a review from a team March 20, 2025 22:00
andrewlock
andrewlock approved these changes Mar 21, 2025
View reviewed changes
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.

Thanks!

tracer/test/Datadog.Trace.ClrProfiler.IntegrationTests/GoogleProtobufTests.cs
[Fact]
[SkippableFact]
[Trait("Category", "EndToEnd")]
[Trait("SkipInCI", "True")] // broken
Copy link
Member

@andrewlock andrewlock Mar 21, 2025

Choose a reason for hiding this comment

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

I totally forgot we have this trait, I would have skipped them directly 😄

@bouwkast bouwkast merged commit 34a9b7f into master Mar 21, 2025
69 of 74 checks passed
@bouwkast bouwkast deleted the steven/delete-protobuf-instrumentation-tagging branch March 21, 2025 17:34
@github-actions github-actions bot added this to the vNext-v3 milestone Mar 21, 2025
@bouwkast bouwkast added area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) type:bug labels Mar 24, 2025
vandonr added a commit that referenced this pull request Apr 7, 2025
@vandonr
Revert "Remove instrumentation for Protobuf Schema tagging (#6792)"
e4affc7 
This reverts commit 34a9b7f.
@vandonr vandonr mentioned this pull request Apr 7, 2025
Merged
vandonr added a commit that referenced this pull request Apr 8, 2025
@vandonr
restore Protobuf instrumentation for schema tracking (#6826)
1c0629b 
## Summary of changes

revert the revert of #6792
The missingMethodExceptions were most likely caused by a trimmed app
([see
discussion](https://dd.slack.com/archives/C07TBLYDQHX/p1741907211362359)).
We had some defense against this for _instrumented_ types, but not for
duck-typed types, and this is where the issue laid. This was taken care
of in #6807 for the generic case, and also after [talking with
DSM](https://dd.slack.com/archives/C069TJPTFPG/p1743500681812779), they
agree that we should try to instrument only user-defined protobufs, so I
excluded the microsoft protobufs (that were causing the issue). Thus,
this PR should be fine to merge as-is, even though it's best to merge it
after #6807 for maximum safety.

## Reason for change

## Implementation details

## Test coverage

## Other details
<!-- Fixes #{issue} -->

<!-- ⚠️ Note: where possible, please obtain 2 approvals prior to
merging. Unless CODEOWNERS specifies otherwise, for external teams it is
typically best to have one review from a team member, and one review
from apm-dotnet. Trivial changes do not require 2 reviews. -->
chojomok pushed a commit that referenced this pull request Jul 15, 2025
@bouwkast @chojomok
Remove instrumentation for Protobuf Schema tagging (#6792)
a029d43 
## Summary of changes

Attempts to remove the Protobuf Schema tagging instrumentation that was
recently added in #6647

Recommended to revert this PR when we do the fix.

## Reason for change

Appears to be throwing errors due to MissingMethodExceptions on the
DuckTypes.
We need time to investigate it and fix.
This seems to be the simplest approach as there were multiple PRs.

I don't see any code that hits this now so should be good.

Additionally, the Sample project is very flaky I attempted to remove
that code generating the proto file.

## Implementation details

- Comment out instrumented method attributes to disable instrumentations
and leave them there for less confusion (maybe?)
- Remove Proto file generation in sample which I think is the cause of
the CI build error
- Skip tests in CI.

## Test coverage

- Skipped now.

## Other details
<!-- Fixes #{issue} -->

<!-- ⚠️ Note: where possible, please obtain 2 approvals prior to
merging. Unless CODEOWNERS specifies otherwise, for external teams it is
typically best to have one review from a team member, and one review
from apm-dotnet. Trivial changes do not require 2 reviews. -->
chojomok pushed a commit that referenced this pull request Jul 15, 2025
@vandonr @chojomok
restore Protobuf instrumentation for schema tracking (#6826)
3f3aa83 
## Summary of changes

revert the revert of #6792
The missingMethodExceptions were most likely caused by a trimmed app
([see
discussion](https://dd.slack.com/archives/C07TBLYDQHX/p1741907211362359)).
We had some defense against this for _instrumented_ types, but not for
duck-typed types, and this is where the issue laid. This was taken care
of in #6807 for the generic case, and also after [talking with
DSM](https://dd.slack.com/archives/C069TJPTFPG/p1743500681812779), they
agree that we should try to instrument only user-defined protobufs, so I
excluded the microsoft protobufs (that were causing the issue). Thus,
this PR should be fine to merge as-is, even though it's best to merge it
after #6807 for maximum safety.

## Reason for change

## Implementation details

## Test coverage

## Other details
<!-- Fixes #{issue} -->

<!-- ⚠️ Note: where possible, please obtain 2 approvals prior to
merging. Unless CODEOWNERS specifies otherwise, for external teams it is
typically best to have one review from a team member, and one review
from apm-dotnet. Trivial changes do not require 2 reviews. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucaspimentel lucaspimentel lucaspimentel approved these changes

@andrewlock andrewlock andrewlock approved these changes

Assignees

No one assigned

Labels

area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) type:bug

Projects

None yet

Milestone

3.13.0

Development

Successfully merging this pull request may close these issues.

4 participants

@bouwkast @andrewlock @lucaspimentel