Skip to content

[Infra] Disallow instance string.Equals methods#7210

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

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

Conversation

@martincostello
Copy link
Copy Markdown
Member

#7196 (comment)

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.
  • Make some comments clearer.

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.
- Make some comments clearer.
@github-actions github-actions Bot added infra Infra work - CI/CD, code coverage, linters pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package pkg:OpenTelemetry.Extensions.Propagators Issues related to OpenTelemetry.Extensions.Propagators NuGet package pkg:OpenTelemetry.Shims.OpenTracing Issues related to OpenTelemetry.Shims.OpenTracing NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package labels Apr 29, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 91.42857% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.50%. Comparing base (44075cd) to head (0a9d8c7).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...enTelemetry/Metrics/StringArrayEqualityComparer.cs 0.00% 2 Missing ⚠️
src/OpenTelemetry/Metrics/Tags.cs 80.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7210      +/-   ##
==========================================
- Coverage   89.53%   89.50%   -0.03%     
==========================================
  Files         272      272              
  Lines       13345    13343       -2     
==========================================
- Hits        11948    11943       -5     
- Misses       1397     1400       +3     
Flag Coverage Δ
unittests-Project-Experimental 89.25% <91.42%> (-0.28%) ⬇️
unittests-Project-Stable 89.49% <91.42%> (-0.03%) ⬇️
unittests-Solution 89.27% <91.42%> (-0.11%) ⬇️
unittests-UnstableCoreLibraries-Experimental 40.84% <60.00%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
...nTelemetry.Api/Context/Propagation/B3Propagator.cs 89.83% <100.00%> (ø)
...etryProtocol/Implementation/ExperimentalOptions.cs 95.00% <100.00%> (ø)
...mplementation/ExportClient/OtlpGrpcExportClient.cs 79.31% <100.00%> (ø)
...ementation/Serializer/ProtobufOtlpLogSerializer.cs 99.23% <100.00%> (ø)
...entation/Serializer/ProtobufOtlpTraceSerializer.cs 95.05% <100.00%> (ø)
...enTelemetry.Extensions.Propagators/B3Propagator.cs 88.98% <100.00%> (ø)
...lemetry.Extensions.Propagators/JaegerPropagator.cs 96.03% <100.00%> (ø)
...OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs 91.46% <100.00%> (ø)
src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs 85.43% <100.00%> (ø)
...lemetry/Internal/SelfDiagnosticsConfigRefresher.cs 87.73% <100.00%> (+0.69%) ⬆️
... and 3 more

... and 3 files with indirect coverage changes

@martincostello martincostello marked this pull request as ready for review April 29, 2026 13:21
Copilot AI review requested due to automatic review settings April 29, 2026 13:21
@martincostello martincostello requested a review from a team as a code owner April 29, 2026 13:21
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 updates repository-wide conventions to disallow instance string.Equals(...) calls (via banned API enforcement) and refactors touched call sites to use the corresponding static string.Equals(...) overloads, along with a few small readability refactors.

Changes:

  • Add banned-symbol rules for System.String.Equals(string) and System.String.Equals(string, StringComparison) to enforce static string.Equals(...).
  • Replace instance Equals usages across metrics, propagators, OTLP exporter, shims, and shared helpers with static string.Equals(...).
  • Apply minor refactors (expression-bodied members, null-conditional dispose) and clarify performance-related comments.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Shared/StatusHelper.cs Refactors to expression-bodied switch and replaces instance Equals with static string.Equals.
src/OpenTelemetry/Metrics/Tags.cs Uses static string.Equals for tag-key comparisons; minor formatting refactor.
src/OpenTelemetry/Metrics/StringArrayEqualityComparer.cs Uses pattern-matching null checks and static string.Equals for element comparisons.
src/OpenTelemetry/Metrics/Builder/MeterProviderBuilderExtensions.cs Replaces instance Equals in view matching; refactors Build to expression-bodied form.
src/OpenTelemetry/Internal/SelfDiagnosticsConfigRefresher.cs Replaces instance Equals for directory compare; simplifies disposal with null-conditional operators.
src/OpenTelemetry.Shims.OpenTracing/SpanShim.cs Uses static string.Equals for special OpenTracing tag key handling.
src/OpenTelemetry.Shims.OpenTracing/SpanBuilderShim.cs Uses static string.Equals for OpenTracing tag key comparisons.
src/OpenTelemetry.Extensions.Propagators/JaegerPropagator.cs Uses static string.Equals for sampled-flag parsing.
src/OpenTelemetry.Extensions.Propagators/B3Propagator.cs Uses static string.Equals for flags parsing (Recorded decision).
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/Serializer/ProtobufOtlpTraceSerializer.cs Uses static string.Equals in status tag-value mapping and clarifies perf comment wording.
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/Serializer/ProtobufOtlpLogSerializer.cs Uses static string.Equals for {OriginalFormat} attribute detection.
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExportClient/OtlpGrpcExportClient.cs Uses static string.Equals for gRPC “no reply detail” detection.
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExperimentalOptions.cs Uses static string.Equals for env-var parsing and refactors disk-path requirement logic.
src/OpenTelemetry.Api/Context/Propagation/B3Propagator.cs Uses static string.Equals for flags parsing (Recorded decision).
build/BannedSymbols.txt Adds banned API entries preventing instance string.Equals(...) overload usage.

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

Comment thread build/BannedSymbols.txt Outdated
Add note about why.

Co-authored-by: Piotr Kiełkowicz <pkielkow@cisco.com>
@martincostello martincostello added this pull request to the merge queue Apr 29, 2026
Merged via the queue into open-telemetry:main with commit 95cfbee Apr 29, 2026
184 of 186 checks passed
@martincostello martincostello deleted the disallow-instance-string-equals branch April 29, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infra Infra work - CI/CD, code coverage, linters pkg:OpenTelemetry.Api Issues related to OpenTelemetry.Api NuGet package pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package pkg:OpenTelemetry.Extensions.Propagators Issues related to OpenTelemetry.Extensions.Propagators NuGet package pkg:OpenTelemetry.Shims.OpenTracing Issues related to OpenTelemetry.Shims.OpenTracing NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants