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.

Sign up for GitHub

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

Jump to bottom

feat: traces QB #2571

Merged
merged 17 commits into from
Apr 25, 2023
Merged

feat: traces QB #2571

merged 17 commits into from
Apr 25, 2023

Conversation

makeavish
Copy link
Member

No description provided.

@request-info
Copy link

request-info bot commented Apr 13, 2023

We would appreciate it if you could provide us with more info about this issue/pr!

@github-actions github-actions bot added the enhancement New feature or request label Apr 13, 2023
This was linked to issues Apr 13, 2023
(traces): query range v3 #2249
Closed
(traces): Parse Query builder query #2391
Closed
(traces): Prepare clickhouse builder queries from payload #2392
Closed
(traces): Handle Multiple queries #2393
Closed
(traces): Run clickhouse queries #2394
Closed
(traces): Convert CH query result to respective format #2396
Closed
@makeavish makeavish removed a link to an issue Apr 13, 2023
(traces): query range v3 #2249
Closed
7 tasks
@makeavish makeavish changed the title feat: traces QB checkpoint feat: traces QB Apr 13, 2023
@makeavish makeavish marked this pull request as ready for review April 14, 2023 13:29
srikanthccv
srikanthccv reviewed Apr 17, 2023
View reviewed changes
Copy link
Member

@srikanthccv srikanthccv left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This largely resembles the logs PR so I don't have as many comments since they were taken care. A couple of questions otherwise LGTM overall. I will take another close look.

pkg/query-service/app/traces/v3/query_builder.go Outdated Show resolved Hide resolved
pkg/query-service/app/traces/v3/query_builder.go Outdated Show resolved Hide resolved
srikanthccv
srikanthccv reviewed Apr 18, 2023
View reviewed changes
pkg/query-service/app/http_handler.go Show resolved Hide resolved
pkg/query-service/app/traces/v3/query_builder.go Outdated Show resolved Hide resolved
pkg/query-service/app/traces/v3/query_builder.go Outdated Show resolved Hide resolved
pkg/query-service/app/traces/v3/query_builder.go Outdated Show resolved Hide resolved
pkg/query-service/app/traces/v3/query_builder.go Outdated Show resolved Hide resolved
pkg/query-service/app/traces/v3/query_builder.go Outdated Show resolved Hide resolved
pkg/query-service/app/traces/v3/query_builder.go Outdated Show resolved Hide resolved
pkg/query-service/app/traces/v3/query_builder_test.go Outdated Show resolved Hide resolved
@nityanandagohain nityanandagohain mentioned this pull request Apr 20, 2023
Closed
@CLAassistant
Copy link

CLAassistant commented Apr 21, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@makeavish makeavish mentioned this pull request Apr 24, 2023
Open
@makeavish
Copy link
Member Author

For now, we are not supporting exists/nexists operators for booleans and numbers as they are not nullable.
We can handle this case in GetTraceAttributeKeys API when operator is exists/nexists, depends on results of #2594.

@makeavish makeavish mentioned this pull request Apr 25, 2023
Closed
@srikanthccv
Copy link
Member

we are not supporting exists/nexists operators for booleans and numbers as they are not nullable

Are you referring to top-level columns? The exists/nexists is supposed to check if the key exists in the map, right?

@makeavish
Copy link
Member Author

we are not supporting exists/nexists operators for booleans and numbers as they are not nullable

Are you referring to top-level columns? The exists/nexists is supposed to check if the key exists in the map, right?

Yes, this was for only top-level columns.

@srikanthccv srikanthccv merged commit bd248c4 into develop Apr 25, 2023
@srikanthccv srikanthccv deleted the feat/trace-query-range branch April 25, 2023 16:23
@srikanthccv srikanthccv mentioned this pull request Jul 4, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nityanandagohain nityanandagohain nityanandagohain approved these changes

@srikanthccv srikanthccv srikanthccv approved these changes

Assignees
No one assigned
Labels
codeball:needs-careful-review Codeball enhancement New feature or request request-more-info
Projects
None yet
Milestone
No milestone
4 participants
@makeavish @CLAassistant @srikanthccv @nityanandagohain