Skip to content

fix(middleware/tracing): replace strings.TrimLeft with strings.CutPrefix for correct fullMethod parsing #3668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

1911860538
Copy link
Contributor

Replaced strings.TrimLeft with strings.CutPrefix to correctly handle fullMethod input.
strings.CutPrefix not only removes the leading '/', but also allows us to check whether fullMethod starts with a '/'.
This ensures that only the correct prefix is removed and provides a clear indication when the input doesn't follow the expected /package.service/method format.

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label May 16, 2025
@1911860538 1911860538 closed this May 16, 2025
@1911860538 1911860538 reopened this May 16, 2025
@kagaya85 kagaya85 requested a review from Copilot July 14, 2025 03:32
Copy link

@Copilot 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 refines the parsing of gRPC fullMethod strings by using strings.CutPrefix instead of strings.TrimLeft, enabling detection of invalid formats and ensuring only a leading slash is removed.

  • Use strings.CutPrefix to remove exactly one leading / and detect when it’s missing
  • Introduce an early return with an attribute for inputs not following /package.service/method
  • Keep downstream splitting logic unchanged
Comments suppressed due to low confidence (2)

middleware/tracing/span.go:105

  • [nitpick] The boolean variable cut is ambiguous. Consider renaming it to something like hasPrefix or removedPrefix to clarify its purpose.
	name, cut := strings.CutPrefix(fullMethod, "/")

middleware/tracing/span.go:106

  • The new branch handling inputs without a leading slash should be covered by unit tests to ensure correct behavior when fullMethod doesn't start with /.
	if !cut {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant