Skip to content

[SqlClient] Fix IndexOutOfRangeException #4139

Merged
martincostello merged 3 commits into
open-telemetry:mainfrom
martincostello:fix-sql-buffer-overflow
Apr 17, 2026
Merged

[SqlClient] Fix IndexOutOfRangeException #4139
martincostello merged 3 commits into
open-telemetry:mainfrom
martincostello:fix-sql-buffer-overflow

Conversation

@martincostello

Copy link
Copy Markdown
Member

Changes

  • Fix IndexOutOfRangeException if trailing whitespace in an SQL statement aligns with an ArrayPool bucket boundary.
  • Remove unused code related to parsing SQL queries from OpenTelemetry.Instrumentation.StackExchangeRedis.

I ran the benchmarks locally, and the differences are basically statistical noise.

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)

Fix `IndexOutOfRangeException` if trailing whitespace in an SQL statement aligns with an `ArrayPool` bucket boundary.
Remove unused code related to parsing SQL queries from OpenTelemetry.Instrumentation.StackExchangeRedis.
@github-actions github-actions Bot added comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient comp:instrumentation.stackexchangeredis Things related to OpenTelemetry.Instrumentation.StackExchangeRedis labels Apr 16, 2026
@codecov

codecov Bot commented Apr 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.00%. Comparing base (a29f306) to head (3893ebc).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/Shared/SqlProcessor.cs 96.55% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4139      +/-   ##
==========================================
+ Coverage   72.89%   73.00%   +0.10%     
==========================================
  Files         460      468       +8     
  Lines       18214    18239      +25     
==========================================
+ Hits        13277    13315      +38     
+ Misses       4937     4924      -13     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 93.86% <96.55%> (+3.90%) ⬆️
unittests-Exporter.Geneva 54.82% <ø> (-0.02%) ⬇️
unittests-Exporter.InfluxDB 95.81% <ø> (ø)
unittests-Exporter.Instana 74.86% <ø> (ø)
unittests-Exporter.OneCollector 94.61% <ø> (ø)
unittests-Extensions 90.78% <ø> (ø)
unittests-Extensions.Enrichment 100.00% <ø> (ø)
unittests-Extensions.Enrichment.AspNetCore 86.27% <ø> (ø)
unittests-Extensions.Enrichment.Http 94.33% <ø> (ø)
unittests-Instrumentation.AWS 83.54% <ø> (ø)
unittests-Instrumentation.AspNet 76.61% <ø> (ø)
unittests-Instrumentation.AspNetCore 70.44% <ø> (ø)
unittests-Instrumentation.Cassandra 23.52% <ø> (?)
unittests-Instrumentation.ConfluentKafka 39.83% <ø> (ø)
unittests-Instrumentation.ElasticsearchClient 80.60% <ø> (ø)
unittests-Instrumentation.EntityFrameworkCore 80.80% <ø> (ø)
unittests-Instrumentation.EventCounters 77.27% <ø> (ø)
unittests-Instrumentation.GrpcCore 91.42% <ø> (ø)
unittests-Instrumentation.GrpcNetClient 73.78% <ø> (ø)
unittests-Instrumentation.Hangfire 86.05% <ø> (ø)
unittests-Instrumentation.Http 74.62% <ø> (ø)
unittests-Instrumentation.Owin 88.62% <ø> (ø)
unittests-Instrumentation.Process 100.00% <ø> (ø)
unittests-Instrumentation.Quartz 78.76% <ø> (ø)
unittests-Instrumentation.Remoting 64.28% <ø> (ø)
unittests-Instrumentation.Runtime 100.00% <ø> (ø)
unittests-Instrumentation.ServiceFabricRemoting 34.68% <ø> (ø)
unittests-Instrumentation.SqlClient 85.21% <ø> (ø)
unittests-Instrumentation.StackExchangeRedis 71.98% <ø> (ø)
unittests-Instrumentation.Wcf 79.68% <ø> (ø)
unittests-OpAmp.Client 80.50% <ø> (+1.46%) ⬆️
unittests-PersistentStorage 68.19% <ø> (-0.66%) ⬇️
unittests-Resources.AWS 73.88% <ø> (ø)
unittests-Resources.Azure 88.31% <ø> (ø)
unittests-Resources.Container 67.34% <ø> (ø)
unittests-Resources.Gcp 71.42% <ø> (ø)
unittests-Resources.Host 72.26% <ø> (ø)
unittests-Resources.OperatingSystem 76.98% <ø> (ø)
unittests-Resources.Process 100.00% <ø> (ø)
unittests-Resources.ProcessRuntime 79.59% <ø> (ø)
unittests-Sampler.AWS 93.72% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/Shared/DatabaseSemanticConventionHelper.cs 83.33% <ø> (+57.47%) ⬆️
src/Shared/SqlProcessor.cs 99.07% <96.55%> (-0.16%) ⬇️

... and 17 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.

Comment thread src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md Outdated
Comment thread src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md Outdated
@martincostello
martincostello marked this pull request as ready for review April 16, 2026 15:01
@martincostello
martincostello requested a review from a team as a code owner April 16, 2026 15:01
Copilot AI review requested due to automatic review settings April 16, 2026 15:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a length-sensitive IndexOutOfRangeException in the shared SQL parsing/sanitization logic used by database instrumentations, and removes now-unneeded SQL parser linkage from the StackExchangeRedis instrumentation.

Changes:

  • Prevent summary-buffer overruns in SqlProcessor by adding capacity checks/growth for the summary buffer and returning any additionally rented buffer to the pool.
  • Add regression coverage via unit + fuzz/property tests for bucket-aligned SQL lengths that previously could trigger the exception.
  • Stop linking SQL query parsing code into StackExchangeRedis; compile SQL-parsing-related helper methods only for instrumentations that opt in via INCLUDE_SQL_QUERY_PARSER.

Reviewed changes

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

Show a summary per file
File Description
test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs Adds a targeted xUnit regression test around GetSanitizedSql behavior.
test/OpenTelemetry.Contrib.Shared.FuzzTests/SqlProcessorTests.cs Adds a property test to exercise ArrayPool bucket-aligned statement lengths without throwing.
src/Shared/SqlProcessor.cs Implements safe summary buffer appends with dynamic growth to avoid IndexOutOfRangeException.
src/Shared/DatabaseSemanticConventionHelper.cs Wraps SQL-parser-dependent methods/usings in #if INCLUDE_SQL_QUERY_PARSER.
src/OpenTelemetry.Instrumentation.StackExchangeRedis/OpenTelemetry.Instrumentation.StackExchangeRedis.csproj Removes compilation of shared SQL parser sources from StackExchangeRedis.
src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj Defines INCLUDE_SQL_QUERY_PARSER to keep query parsing features enabled.
src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md Documents the exception fix in Unreleased.
src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj Defines INCLUDE_SQL_QUERY_PARSER to keep query parsing features enabled.
src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md Documents the exception fix in Unreleased.

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

Comment thread test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorTests.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient comp:instrumentation.stackexchangeredis Things related to OpenTelemetry.Instrumentation.StackExchangeRedis

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants