-
Notifications
You must be signed in to change notification settings - Fork 242
*: HasProfileData endpoint and time range support for ProfileTypes query
#6003
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
Conversation
pkg/query/columnquery.go
Outdated
| } | ||
|
|
||
| func (q *ColumnQueryAPI) HasProfileData(ctx context.Context, req *pb.HasProfileDataRequest) (*pb.HasProfileDataResponse, error) { | ||
| res, err := q.ProfileTypes(ctx, &pb.ProfileTypesRequest{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should call q.querier.HasProfileData which is defined differently for querier.go and querier_sql.go. Basically ProfileTypes for querier.go but the SELECT 1 FROM v1.stacktraces LIMIT 1 query in querier_sql.go that we were talking about
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was just about to comment the exact same thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concretely for parca I think this means we should make HasProfileData part of the Querier interface too and this should call q.querier.HasProfileData
|
🔴 Meticulous spotted visual differences in 460 of 493 screens tested: view and approve differences detected. Meticulous evaluated ~5 hours of user flows against your PR. Last updated for commit 7e6b665. This comment will update as new commits are pushed. |
This decouples the need for
ProfileTypesendpoint to check if there is profile data available.