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

logs v4 qb refactor #5908

Merged
merged 9 commits into from
Sep 12, 2024
Merged

logs v4 qb refactor #5908

merged 9 commits into from
Sep 12, 2024

Conversation

nityanandagohain
Copy link
Member

@nityanandagohain nityanandagohain commented Sep 10, 2024

For #5555 , second PR . Follow up of #5872

Changes made code wise

  • In v3 folder json_filter.go , json_filter_test.go, query_builder.go
    Some functions and constants are made public in these files and no other change.

  • In v4 folder json_filter.go and json_filter_test.go
    The only change for these two files is that the map jsonLogOperators uses LIKE instead of ILIKE. So the function GetJSONFilter is a local one and no change apart from that.

  • In v4 folder query_builder.go and query_builder_test.go

    • It adds the resource filter caluse
    • new select clauses for map support.
    • refactoring the code as we did for v4 resource qb
    • Only body contains search is case insensitive

@srikanthccv
Copy link
Member

The only change for these two files is that the map jsonLogOperators uses LIKE instead of ILIKE. So the function GetJSONFilter is a local one and no change apart from that.

Can we get rid of v3 in logs after v4 is fully rolled out?

@nityanandagohain
Copy link
Member Author

Yeah once it is fully rolled out, we will stop ingestion for the old table and the entire v3 code can be removed, some functions will have to be moved to the new folder that we are reusing.

Copy link
Member

@srikanthccv srikanthccv left a comment

Choose a reason for hiding this comment

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

Some comments

pkg/query-service/app/logs/v4/query_builder.go Outdated Show resolved Hide resolved
@srikanthccv
Copy link
Member

Mostly LGTM. @raj-k-singh please review

@nityanandagohain nityanandagohain merged commit 6e7f04b into develop Sep 12, 2024
13 checks passed
@nityanandagohain nityanandagohain deleted the feat/logs-v4-qb-refactor branch September 12, 2024 04:18
@nityanandagohain
Copy link
Member Author

@raj-k-singh feel free to leave a comment if you find something, I have merged it as I want to move ahead with other PRs for the feature.

@raj-k-singh
Copy link
Collaborator

@raj-k-singh feel free to leave a comment if you find something, I have merged it as I want to move ahead with other PRs for the feature.

I gave the PR a quick look and everything seemed ok to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants