-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
Pyroscope: Remove support for old pyroscope #74683
Conversation
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.
@joey-grafana thanks for noticing the docs and devenv stuff, should be all fixed now. |
return err | ||
} | ||
|
||
res, err := d.client.LabelNames(ctx, query["query"][0], start, end) |
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.
Curious about this. Why remove query
, start
and end
? Is it because we were not actually using them on the frontend?
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.
Because the phlare client (which is also new pyroscope client) does not use it so we would just have unused args there. We actually send it from frontend so it will be easy to add them again once pyroscope supports it.
What is this feature?
Removes support for old pyroscope backend and relevant client code. Also removes the backend type switch from the config page as it is not needed anymore.
Why do we need this feature?
For one there was a bit of confusion around the backend type selection and as we have Grafana Pyroscope v1 release the old pyroscope support is less relevant.
Who is this feature for?
[Add information on what kind of user the feature is for.]
Which issue(s) does this PR fix?:
Fixes #
Special notes for your reviewer:
Please check that: