Skip to content

Make telemetry headers mandatory in tests#6835

Merged
kevingosse merged 1 commit intomasterfrom
kevin/optional_headers
Apr 11, 2025
Merged

Make telemetry headers mandatory in tests#6835
kevingosse merged 1 commit intomasterfrom
kevin/optional_headers

Conversation

@kevingosse
Copy link
Contributor

Summary of changes

Remove the OptionalTelemetryHeaders property from the mock agent.

Reason for change

It was originally added because libdatadog didn't send the proper headers. The latest versions fixed this.

Implementation details

Deleting some code.

Test coverage

This was needed for CreatedumpTests.SendReportThroughTelemetry.

@kevingosse kevingosse added the area:tests unit tests, integration tests label Apr 10, 2025
@kevingosse kevingosse requested a review from a team as a code owner April 10, 2025 09:14
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.

🎉

@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Apr 10, 2025

Datadog Report

Branch report: kevin/optional_headers
Commit report: 4de6a17
Test service: dd-trace-dotnet

✅ 0 Failed, 255678 Passed, 2436 Skipped, 20h 57m 36.63s Total Time

@andrewlock
Copy link
Member

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 (6835) - mean (69ms)  : 66, 72
     .   : milestone, 69,
    master - mean (69ms)  : 67, 72
     .   : milestone, 69,

    section CallTarget+Inlining+NGEN
    This PR (6835) - mean (1,011ms)  : 978, 1043
     .   : milestone, 1011,
    master - mean (1,010ms)  : 981, 1039
     .   : milestone, 1010,

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

    section CallTarget+Inlining+NGEN
    This PR (6835) - mean (694ms)  : 674, 715
     .   : milestone, 694,
    master - mean (698ms)  : 679, 717
     .   : milestone, 698,

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

    section CallTarget+Inlining+NGEN
    This PR (6835) - mean (651ms)  : 631, 672
     .   : milestone, 651,
    master - mean (655ms)  : 636, 673
     .   : milestone, 655,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Framework 4.6.2) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6835) - mean (189ms)  : 185, 194
     .   : milestone, 189,
    master - mean (189ms)  : 185, 193
     .   : milestone, 189,

    section CallTarget+Inlining+NGEN
    This PR (6835) - mean (1,106ms)  : 1077, 1134
     .   : milestone, 1106,
    master - mean (1,107ms)  : 1082, 1133
     .   : milestone, 1107,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET Core 3.1) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6835) - mean (267ms)  : 263, 272
     .   : milestone, 267,
    master - mean (268ms)  : 263, 272
     .   : milestone, 268,

    section CallTarget+Inlining+NGEN
    This PR (6835) - mean (876ms)  : 851, 902
     .   : milestone, 876,
    master - mean (879ms)  : 843, 916
     .   : milestone, 879,

Loading
gantt
    title Execution time (ms) HttpMessageHandler (.NET 6) 
    dateFormat  X
    axisFormat %s
    todayMarker off
    section Baseline
    This PR (6835) - mean (262ms)  : 257, 266
     .   : milestone, 262,
    master - mean (260ms)  : 257, 263
     .   : milestone, 260,

    section CallTarget+Inlining+NGEN
    This PR (6835) - mean (866ms)  : 831, 901
     .   : milestone, 866,
    master - mean (869ms)  : 836, 902
     .   : milestone, 869,

Loading

@datadog-ddstaging
Copy link

Datadog Report

Branch report: kevin/optional_headers
Commit report: 4de6a17
Test service: dd-trace-dotnet

✅ 0 Failed, 2 Passed, 0 Skipped, 0s Total Time

@kevingosse kevingosse merged commit e0108b3 into master Apr 11, 2025
121 of 126 checks passed
@kevingosse kevingosse deleted the kevin/optional_headers branch April 11, 2025 08:29
@github-actions github-actions bot added this to the vNext-v3 milestone Apr 11, 2025
chojomok pushed a commit that referenced this pull request Jul 15, 2025
## Summary of changes

Remove the `OptionalTelemetryHeaders` property from the mock agent.

## Reason for change

It was originally added because libdatadog didn't send the proper
headers. The latest versions fixed this.

## Implementation details

Deleting some code.

## Test coverage

This was needed for `CreatedumpTests.SendReportThroughTelemetry`.
andrewlock added a commit that referenced this pull request Jan 12, 2026
## Summary of changes

Removes unused properties and functionality from  `MockTelemetryAgent`

## Reason for change

Noticed the obsolete behaviour while working on something else. The
`OptionalHeaders` option was added as part of the original crashtracking
work, and was partially removed as no longer required in #6835, and the
parts here were just missed.

## Implementation details

Delete unused code

## Test coverage

As long as the tests still pass, we're good

## Other details

Noticed while working on #8017
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