Skip to content

[Infra] Disallow instance string.Equals methods#4332

Merged
martincostello merged 2 commits into
open-telemetry:mainfrom
martincostello:disallow-instance-string-equals
Apr 29, 2026
Merged

[Infra] Disallow instance string.Equals methods#4332
martincostello merged 2 commits into
open-telemetry:mainfrom
martincostello:disallow-instance-string-equals

Conversation

@martincostello
Copy link
Copy Markdown
Member

Relates to open-telemetry/opentelemetry-dotnet#7210.

Changes

  • Disallow use of string instance Equals() methods and require the static versions be used instead.
  • Apply minor refactoring suggested by Visual Studio in files touched.

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)

- Disallow use of string instance `Equals()` methods and require the static versions be used instead.
- Apply minor refactoring suggested by Visual Studio in files touched.
@github-actions github-actions Bot added infra Infra work - CI/CD, code coverage, linters comp:exporter.geneva Things related to OpenTelemetry.Exporter.Geneva comp:instrumentation.aws Things related to OpenTelemetry.Instrumentation.AWS comp:sampler.aws Things related to OpenTelemetry.Samplers.AWS labels Apr 29, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 86.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.98%. Comparing base (bf729bf) to head (f584b53).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...strumentation.AWS/Implementation/AWSServiceType.cs 71.42% 2 Missing ⚠️
...emetry.Instrumentation.AWS/Implementation/Utils.cs 50.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4332      +/-   ##
==========================================
- Coverage   76.02%   75.98%   -0.04%     
==========================================
  Files         466      466              
  Lines       18567    18569       +2     
==========================================
- Hits        14115    14110       -5     
- Misses       4452     4459       +7     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 89.38% <ø> (-0.02%) ⬇️
unittests-Exporter.Geneva 55.14% <100.00%> (-0.23%) ⬇️
unittests-Exporter.InfluxDB 95.81% <ø> (ø)
unittests-Exporter.Instana 92.00% <ø> (ø)
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 85.22% <63.63%> (ø)
unittests-Instrumentation.AspNet 76.99% <ø> (ø)
unittests-Instrumentation.AspNetCore 70.44% <ø> (ø)
unittests-Instrumentation.Cassandra 92.85% <ø> (ø)
unittests-Instrumentation.ConfluentKafka 78.52% <ø> (ø)
unittests-Instrumentation.ElasticsearchClient 80.60% <ø> (ø)
unittests-Instrumentation.EntityFrameworkCore 81.39% <ø> (ø)
unittests-Instrumentation.EventCounters 77.27% <ø> (ø)
unittests-Instrumentation.GrpcCore 91.27% <ø> (ø)
unittests-Instrumentation.GrpcNetClient 73.78% <ø> (ø)
unittests-Instrumentation.Hangfire 88.91% <ø> (ø)
unittests-Instrumentation.Http 74.62% <ø> (ø)
unittests-Instrumentation.Owin 88.62% <ø> (ø)
unittests-Instrumentation.Process 100.00% <ø> (ø)
unittests-Instrumentation.Quartz 78.76% <ø> (ø)
unittests-Instrumentation.Remoting 66.06% <ø> (ø)
unittests-Instrumentation.Runtime 100.00% <ø> (ø)
unittests-Instrumentation.ServiceFabricRemoting 40.83% <ø> (ø)
unittests-Instrumentation.SqlClient 84.36% <ø> (-0.21%) ⬇️
unittests-Instrumentation.StackExchangeRedis 93.63% <ø> (ø)
unittests-Instrumentation.Wcf 81.57% <ø> (ø)
unittests-OpAmp.Client 84.36% <ø> (+0.61%) ⬆️
unittests-PersistentStorage 70.41% <ø> (ø)
unittests-Resources.AWS 74.49% <ø> (ø)
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 97.52% <100.00%> (ø)

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

Files with missing lines Coverage Δ
...xporter.Geneva/Internal/ConnectionStringBuilder.cs 95.91% <100.00%> (ø)
...rter.Geneva/Metrics/GenevaMetricExporterOptions.cs 100.00% <100.00%> (ø)
...metry.Exporter.Geneva/Metrics/TlvMetricExporter.cs 87.87% <100.00%> (-3.64%) ⬇️
...c/OpenTelemetry.Sampler.AWS/SamplingRuleApplier.cs 99.26% <100.00%> (ø)
...strumentation.AWS/Implementation/AWSServiceType.cs 71.42% <71.42%> (ø)
...emetry.Instrumentation.AWS/Implementation/Utils.cs 66.66% <50.00%> (ø)

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

@martincostello martincostello marked this pull request as ready for review April 29, 2026 13:22
@martincostello martincostello requested a review from a team as a code owner April 29, 2026 13:22
Copilot AI review requested due to automatic review settings April 29, 2026 13:22
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 strengthens repository-wide string comparison consistency by banning instance string.Equals(...) overloads and updating touched code to use the static string.Equals(...) alternatives.

Changes:

  • Added banned-symbol entries to disallow System.String.Equals(string) and System.String.Equals(string, StringComparison).
  • Replaced instance Equals calls with string.Equals(...) across several AWS, Geneva exporter, and sampler components.
  • Applied minor IDE refactorings (expression-bodied members) in AWS instrumentation utilities.

Reviewed changes

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

Show a summary per file
File Description
src/OpenTelemetry.Sampler.AWS/SamplingRuleApplier.cs Switched tag/resource key comparisons to string.Equals(...).
src/OpenTelemetry.Instrumentation.AWS/Implementation/Utils.cs Updated tag key comparison and refactored small helpers to expression-bodied members.
src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSServiceType.cs Replaced instance comparisons with string.Equals(..., OrdinalIgnoreCase).
src/OpenTelemetry.Exporter.Geneva/Metrics/TlvMetricExporter.cs Updated reserved-dimension key checks to use static string.Equals(...).
src/OpenTelemetry.Exporter.Geneva/Metrics/GenevaMetricExporterOptions.cs Updated reserved-dimension validation comparisons to use static string.Equals(...).
src/OpenTelemetry.Exporter.Geneva/Internal/ConnectionStringBuilder.cs Updated private-preview flag parsing comparisons to use static string.Equals(...).
build/BannedSymbols.txt Added new banned-symbol rules for instance string.Equals overloads.

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

Comment thread build/BannedSymbols.txt
Comment thread build/BannedSymbols.txt Outdated
Add note about why.
@martincostello martincostello added this pull request to the merge queue Apr 29, 2026
Merged via the queue into open-telemetry:main with commit e0e10cc Apr 29, 2026
316 of 317 checks passed
@martincostello martincostello deleted the disallow-instance-string-equals branch April 29, 2026 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:exporter.geneva Things related to OpenTelemetry.Exporter.Geneva comp:instrumentation.aws Things related to OpenTelemetry.Instrumentation.AWS comp:sampler.aws Things related to OpenTelemetry.Samplers.AWS infra Infra work - CI/CD, code coverage, linters

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants