Skip to content
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

fix: issues with resource query builder w.r.t quotes #6318

Merged
merged 2 commits into from
Nov 1, 2024

Conversation

nityanandagohain
Copy link
Member

@nityanandagohain nityanandagohain commented Oct 30, 2024

Fixes #6307

if the resource label value is

{"arr":"[\"a\",\"b\",\"c\"]","num":"123"}

then now the corresponding queries for like will be

labels like '%arr%[\\\\"s\\\\",\\\\"ds\\\\",\\\\"sdf\\\\"]%'

Important

Fixes query building for resource labels with quotes by updating string escaping logic in query_builder.go and resource_query_builder.go.

  • Behavior:
    • Fixes query building for resource labels with quotes in query_builder.go and resource_query_builder.go.
    • Updates QuoteEscapedStringForContains in format.go to accept a boolean isIndex parameter for handling index-specific escaping.
  • Functions:
    • Modifies buildAttributeFilter() in query_builder.go to use QuoteEscapedStringForContains with isIndex=false.
    • Modifies buildResourceFilter() and buildIndexFilterForInOperator() in resource_query_builder.go to use QuoteEscapedStringForContains with appropriate isIndex values.
  • Tests:
    • Updates test cases in resource_query_builder_test.go to validate new escaping behavior for queries with quotes.

This description was created by Ellipsis for f00319f. It will automatically update as commits are pushed.

@github-actions github-actions bot added the bug Something isn't working label Oct 30, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to f00319f in 27 seconds

More details
  • Looked at 144 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. pkg/query-service/app/logs/v4/query_builder.go:152
  • Draft comment:
    Ensure that the isIndex parameter is correctly set based on the context where QuoteEscapedStringForContains is used. This change is crucial for correct escaping of quotes in different contexts.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. pkg/query-service/app/resource/resource_query_builder.go:52
  • Draft comment:
    Ensure that the isIndex parameter is correctly set based on the context where QuoteEscapedStringForContains is used. This change is crucial for correct escaping of quotes in different contexts.
  • Reason this comment was not posted:
    Marked as duplicate.
3. pkg/query-service/app/resource/resource_query_builder.go:95
  • Draft comment:
    Ensure that the isIndex parameter is correctly set based on the context where QuoteEscapedStringForContains is used. This change is crucial for correct escaping of quotes in different contexts.
  • Reason this comment was not posted:
    Marked as duplicate.
4. pkg/query-service/app/resource/resource_query_builder.go:113
  • Draft comment:
    Ensure that the isIndex parameter is correctly set based on the context where QuoteEscapedStringForContains is used. This change is crucial for correct escaping of quotes in different contexts.
  • Reason this comment was not posted:
    Marked as duplicate.
5. pkg/query-service/utils/format.go:159
  • Draft comment:
    Consider using constants or enums for the isIndex parameter in QuoteEscapedStringForContains to improve readability and maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The function QuoteEscapedStringForContains is used in multiple places with hardcoded boolean values for the isIndex parameter. This could be improved by using a more descriptive approach, such as constants or enums, to enhance readability and maintainability.

Workflow ID: wflow_cuI6TVTMo9PyJOfp


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@nityanandagohain nityanandagohain merged commit 580f0b8 into develop Nov 1, 2024
13 checks passed
@nityanandagohain nityanandagohain deleted the issue_6307 branch November 1, 2024 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LOGS context doesn't work with process.command_args and process.pid
2 participants