Skip to content

[SqlClient] Add skippable SQL sanitizer tests#3660

Merged
martincostello merged 3 commits intoopen-telemetry:mainfrom
martincostello:extend-sql-tests
Jan 8, 2026
Merged

[SqlClient] Add skippable SQL sanitizer tests#3660
martincostello merged 3 commits intoopen-telemetry:mainfrom
martincostello:extend-sql-tests

Conversation

@martincostello
Copy link
Copy Markdown
Member

@martincostello martincostello commented Jan 7, 2026

Relates to #3657, #3659 and #3666.

Changes

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

Copilot AI review requested due to automatic review settings January 7, 2026 12:05
@martincostello martincostello requested a review from a team as a code owner January 7, 2026 12:05
@github-actions github-actions Bot added infra Infra work - CI/CD, code coverage, linters dependencies Pull requests that update a dependency file labels Jan 7, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.52%. Comparing base (a292e97) to head (1f2d76c).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3660      +/-   ##
==========================================
- Coverage   71.70%   71.52%   -0.18%     
==========================================
  Files         445      455      +10     
  Lines       17589    17640      +51     
==========================================
+ Hits        12612    12617       +5     
- Misses       4977     5023      +46     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 89.05% <ø> (ø)
unittests-Exporter.Geneva 53.76% <ø> (-0.02%) ⬇️
unittests-Exporter.InfluxDB 95.14% <ø> (ø)
unittests-Exporter.Instana 74.86% <ø> (ø)
unittests-Exporter.OneCollector 94.62% <ø> (ø)
unittests-Extensions 90.65% <ø> (ø)
unittests-Extensions.Enrichment 100.00% <ø> (ø)
unittests-Extensions.Enrichment.AspNetCore 86.27% <ø> (ø)
unittests-Extensions.Enrichment.Http 94.33% <ø> (ø)
unittests-Instrumentation.AWS 83.42% <ø> (ø)
unittests-Instrumentation.AspNet 77.92% <ø> (-0.23%) ⬇️
unittests-Instrumentation.AspNetCore 71.69% <ø> (ø)
unittests-Instrumentation.Cassandra 23.52% <ø> (?)
unittests-Instrumentation.ConfluentKafka 14.10% <ø> (ø)
unittests-Instrumentation.ElasticsearchClient 80.12% <ø> (ø)
unittests-Instrumentation.EntityFrameworkCore 80.80% <ø> (ø)
unittests-Instrumentation.EventCounters 77.27% <ø> (ø)
unittests-Instrumentation.GrpcCore 91.42% <ø> (ø)
unittests-Instrumentation.GrpcNetClient 79.61% <ø> (ø)
unittests-Instrumentation.Hangfire 86.05% <ø> (ø)
unittests-Instrumentation.Http 74.22% <ø> (ø)
unittests-Instrumentation.Owin 88.62% <ø> (ø)
unittests-Instrumentation.Process 100.00% <ø> (ø)
unittests-Instrumentation.Quartz 78.76% <ø> (ø)
unittests-Instrumentation.Runtime 100.00% <ø> (ø)
unittests-Instrumentation.ServiceFabricRemoting 34.54% <ø> (ø)
unittests-Instrumentation.SqlClient 87.12% <ø> (ø)
unittests-Instrumentation.StackExchangeRedis 71.80% <ø> (ø)
unittests-Instrumentation.Wcf 79.44% <ø> (-0.14%) ⬇️
unittests-OpAmp.Client 80.16% <ø> (-0.57%) ⬇️
unittests-PersistentStorage 74.91% <ø> (ø)
unittests-Resources.AWS 74.42% <ø> (ø)
unittests-Resources.Azure 85.31% <ø> (ø)
unittests-Resources.Container 67.34% <ø> (ø)
unittests-Resources.Gcp 71.42% <ø> (ø)
unittests-Resources.Host 71.85% <ø> (ø)
unittests-Resources.OperatingSystem 76.98% <ø> (ø)
unittests-Resources.Process 100.00% <ø> (ø)
unittests-Resources.ProcessRuntime 79.59% <ø> (ø)
unittests-Sampler.AWS 94.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 14 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the SQL sanitizer test infrastructure by adding support for skippable test cases. It introduces two new test cases that document known limitations related to stored procedure handling and sensitive data sanitization.

Key Changes:

  • Added Xunit.SkippableFact package dependency to enable conditional test skipping
  • Extended the test case model with an optional Skip property to specify skip reasons
  • Added two skipped test cases for stored procedure scenarios that require future fixes

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs Updated test method to use SkippableTheory and added skip logic based on test case configuration
test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTestCases.cs Extended TestCase class with nullable Skip property and updated deserialization to handle it
test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorAdditionalTestCases.json Added two skipped test cases: one for stored procedures without parameters and one for stored procedures with sensitive data
test/OpenTelemetry.Contrib.Shared.Tests/OpenTelemetry.Contrib.Shared.Tests.csproj Added Xunit.SkippableFact package reference
Directory.Packages.props Specified version 1.5.61 for Xunit.SkippableFact package

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorAdditionalTestCases.json Outdated
Add support for skipping test cases for `SqlProcessor`.
Add more test cases for query sanitization and summary.
Copy link
Copy Markdown
Member

@alanwest alanwest left a comment

Choose a reason for hiding this comment

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

💯

@martincostello martincostello added this pull request to the merge queue Jan 8, 2026
Merged via the queue into open-telemetry:main with commit 5771ebc Jan 8, 2026
320 checks passed
@martincostello martincostello deleted the extend-sql-tests branch January 8, 2026 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file infra Infra work - CI/CD, code coverage, linters

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants