Skip to content

[SqlClient] Sanitize login/user names#3663

Merged
alanwest merged 2 commits intoopen-telemetry:mainfrom
martincostello:gh-3661
Jan 7, 2026
Merged

[SqlClient] Sanitize login/user names#3663
alanwest merged 2 commits intoopen-telemetry:mainfrom
martincostello:gh-3661

Conversation

@martincostello
Copy link
Copy Markdown
Member

@martincostello martincostello commented Jan 7, 2026

Fixes #3661

Changes

Sanitize LOGIN and USER values in SQL queries.

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)

Sanitize `LOGIN` and `USER` values in SQL queries.

Resolves open-telemetry#3661.
@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 (7aac7d8) to head (905d92e).
⚠️ 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    #3663      +/-   ##
==========================================
- Coverage   71.54%   71.52%   -0.03%     
==========================================
  Files         455      455              
  Lines       17617    17631      +14     
==========================================
+ Hits        12604    12610       +6     
- Misses       5013     5021       +8     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 89.01% <100.00%> (+0.31%) ⬆️
unittests-Exporter.Geneva 53.64% <ø> (-0.12%) ⬇️
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.11% <ø> (-0.23%) ⬇️
unittests-Instrumentation.AWS 83.42% <ø> (ø)
unittests-Instrumentation.AspNet 78.14% <ø> (+0.22%) ⬆️
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.19% <ø> (ø)
unittests-Instrumentation.StackExchangeRedis 71.80% <ø> (ø)
unittests-Instrumentation.Wcf 79.57% <ø> (ø)
unittests-OpAmp.Client 80.16% <ø> (-0.57%) ⬇️
unittests-PersistentStorage 76.92% <ø> (+0.33%) ⬆️
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.

Files with missing lines Coverage Δ
src/Shared/SqlProcessor.cs 99.14% <100.00%> (+0.04%) ⬆️

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

Add changes to CHANGELOGs.
@github-actions github-actions Bot added comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient labels Jan 7, 2026
@martincostello martincostello marked this pull request as ready for review January 7, 2026 17:34
@martincostello martincostello requested a review from a team as a code owner January 7, 2026 17:34
Copilot AI review requested due to automatic review settings January 7, 2026 17:34
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 implements sanitization of LOGIN and USER names in SQL queries to prevent sensitive information from appearing in telemetry. The implementation adds a new Login keyword, introduces a sanitization mechanism, and updates both the query text (replaces names with ?) and query summaries (excludes names entirely) for CREATE, ALTER, and DROP operations involving LOGIN or USER keywords.

Key Changes

  • Added Login keyword support with associated initialization and configuration in the SQL keyword system
  • Implemented SanitizeNextToken method and SanitizeNextNonKeywordToken state flag to replace LOGIN/USER names with placeholders
  • Updated CaptureNextTokenInSummary to exclude LOGIN/USER names from query summaries

Reviewed changes

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

File Description
src/Shared/SqlProcessor.cs Core implementation: adds Login keyword, sanitization logic, and summary exclusion for LOGIN/USER names
test/OpenTelemetry.Contrib.Shared.Tests/SqlProcessorAdditionalTestCases.json Test cases covering CREATE/ALTER/DROP operations for both USER and LOGIN keywords
src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md Documents the sanitization feature for SQL Client instrumentation
src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md Documents the sanitization feature for EF Core instrumentation

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

Comment thread src/Shared/SqlProcessor.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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] SQL sanitization does redact login/user values for queries that alter/create/drop them

5 participants