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

Pyroscope: Remove support for old pyroscope #74683

Merged
merged 10 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@
/devenv/docker/blocks/mysql_opendata/ @grafana/oss-big-tent
/devenv/docker/blocks/mysql_tests/ @grafana/oss-big-tent
/devenv/docker/blocks/opentsdb/ @grafana/observability-metrics
/devenv/docker/blocks/phlare/ @grafana/observability-traces-and-profiling
/devenv/docker/blocks/postgres/ @grafana/oss-big-tent
/devenv/docker/blocks/postgres_tests/ @grafana/oss-big-tent
/devenv/docker/blocks/prometheus/ @grafana/observability-metrics
Expand Down
8 changes: 0 additions & 8 deletions devenv/docker/blocks/phlare/docker-compose.yaml

This file was deleted.

5 changes: 0 additions & 5 deletions devenv/docker/blocks/phlare/phlare.yaml

This file was deleted.

4 changes: 1 addition & 3 deletions devenv/docker/blocks/pyroscope/docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
pyroscope:
image: "pyroscope/pyroscope:latest"
command:
- "server"
image: "grafana/pyroscope:latest"
ports:
- "4040:4040"
16 changes: 7 additions & 9 deletions docs/sources/datasources/grafana-pyroscope.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ weight: 1150

# Grafana Pyroscope data source

Formerly Phlare data source, it supports both Phlare and Pyroscope, a horizontally scalable, highly-available, multi-tenant, OSS, continuous profiling aggregation systems. Add it as a data source, and you are ready to query your profiles in [Explore][explore].
Formerly Phlare data source, now Grafana Pyroscope, a horizontally scalable, highly-available, multi-tenant, OSS, continuous profiling aggregation system. Add it as a data source, and you are ready to query your profiles in [Explore][explore].

## Configure the Grafana Pyroscope data source

Expand All @@ -45,7 +45,6 @@ To configure basic settings for the data source, complete the following steps:
| `User` | User name for basic authentication. |
| `Password` | Password for basic authentication. |
| `Minimal step` | Used for queries returning timeseries data. Phlare backend, similar to Prometheus, scrapes profiles at certain intervals. To prevent querying at smaller interval use Minimal step same or higher than your Phlare scrape interval. For Pyroscope backend this prevents returning too many data points to the front end. |
| `Backend type` | Select a backend type between Phlare and Pyroscope. It is autodetected if not set but once set you have to change it manually. |

## Querying

Expand All @@ -55,13 +54,13 @@ To configure basic settings for the data source, complete the following steps:

Query editor gives you access to a profile type selector, a label selector, and collapsible options.

![Profile or App selector](/static/img/docs/phlare/select-profile.png 'Profile or App selector')
![Profile selector](/static/img/docs/phlare/select-profile.png 'Profile selector')

Select a profile type or app from the drop-down menu. While the label selector can be left empty to query all profiles without filtering by labels, the profile type or app must be selected for the query to be valid. Grafana does not show any data if the profile type or app isn’t selected when a query is run.
Select a profile type from the drop-down menu. While the label selector can be left empty to query all profiles without filtering by labels, the profile type or app must be selected for the query to be valid. Grafana does not show any data if the profile type or app isn’t selected when a query is run.

![Labels selector](/static/img/docs/phlare/labels-selector.png 'Labels selector')

Use the labels selector input to filter by labels. Phlare and Pyroscope uses similar syntax to Prometheus to filter labels. Refer to [Phlare documentation](https://grafana.com/docs/phlare/latest/) for available operators and syntax.
Use the labels selector input to filter by labels. Pyroscope uses similar syntax to Prometheus to filter labels. Refer to [Pyroscope documentation](https://grafana.com/docs/pyroscope/latest/) for available operators and syntax.

![Options section](/static/img/docs/phlare/options-section.png 'Options section')

Expand All @@ -77,7 +76,7 @@ Profiles can be visualized in a flame graph. See the [Flame Graph documentation]

![Flame graph](/static/img/docs/phlare/flame-graph.png 'Flame graph')

Phlare and Pyroscope returns profiles aggregated over a selected time range, and the absolute values in the flame graph grow as the time range gets bigger while keeping the relative values meaningful. You can zoom in on the time range to get a higher granularity profile up to the point of a single scrape interval.
Pyroscope returns profiles aggregated over a selected time range, and the absolute values in the flame graph grow as the time range gets bigger while keeping the relative values meaningful. You can zoom in on the time range to get a higher granularity profile up to the point of a single scrape interval.

### Metrics query results

Expand All @@ -98,11 +97,10 @@ apiVersion: 1

datasources:
- name: Grafana Pyroscope
type: phlare
url: http://localhost:4100
type: grafana-pyroscope-datasource
url: http://localhost:4040
jsonData:
minStep: '15s'
backendType: 'pyroscope'
```

{{% docs/reference %}}
Expand Down
4 changes: 2 additions & 2 deletions pkg/tests/api/plugins/data/expectedListResp.json
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@
"name": "Grafana Labs",
"url": "https://www.grafana.com"
},
"description": "Supports Phlare and Pyroscope backends, horizontally-scalable, highly-available, multi-tenant continuous profiling aggregation systems.",
"description": "Data source for Grafana Pyroscope, horizontally-scalable, highly-available, multi-tenant continuous profiling aggregation system.",
"links": [
{
"name": "GitHub Project",
Expand Down Expand Up @@ -1896,4 +1896,4 @@
"signatureOrg": "",
"angularDetected": false
}
]
]
66 changes: 0 additions & 66 deletions pkg/tsdb/grafana-pyroscope-datasource/client.go

This file was deleted.

120 changes: 11 additions & 109 deletions pkg/tsdb/grafana-pyroscope-datasource/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,13 @@ import (
"fmt"
"net/http"
"net/url"
"strconv"
"time"

"github.com/grafana/grafana-plugin-sdk-go/backend"
"github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt"
"github.com/grafana/grafana-plugin-sdk-go/data"
"github.com/grafana/grafana/pkg/infra/httpclient"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/contexthandler"
"github.com/grafana/grafana/pkg/services/datasources"
)

var (
Expand All @@ -25,6 +22,14 @@ var (
_ backend.StreamHandler = (*PhlareDatasource)(nil)
)

type ProfilingClient interface {
ProfileTypes(context.Context) ([]*ProfileType, error)
LabelNames(ctx context.Context) ([]string, error)
LabelValues(ctx context.Context, label string) ([]string, error)
GetSeries(ctx context.Context, profileTypeID string, labelSelector string, start int64, end int64, groupBy []string, step float64) (*SeriesResponse, error)
GetProfile(ctx context.Context, profileTypeID string, labelSelector string, start int64, end int64, maxNodes *int64) (*ProfileResponse, error)
}

// PhlareDatasource is a datasource for querying application performance profiles.
type PhlareDatasource struct {
httpClient *http.Client
Expand All @@ -33,10 +38,6 @@ type PhlareDatasource struct {
ac accesscontrol.AccessControl
}

type JsonData struct {
BackendType string `json:"backendType"`
}

// NewPhlareDatasource creates a new datasource instance.
func NewPhlareDatasource(httpClientProvider httpclient.Provider, settings backend.DataSourceInstanceSettings, ac accesscontrol.AccessControl) (instancemgmt.Instance, error) {
opt, err := settings.HTTPClientOptions()
Expand All @@ -48,15 +49,9 @@ func NewPhlareDatasource(httpClientProvider httpclient.Provider, settings backen
return nil, err
}

var jsonData *JsonData
err = json.Unmarshal(settings.JSONData, &jsonData)
if err != nil {
return nil, err
}

return &PhlareDatasource{
httpClient: httpClient,
client: getClient(jsonData.BackendType, httpClient, settings.URL),
client: NewPhlareClient(httpClient, settings.URL),
joey-grafana marked this conversation as resolved.
Show resolved Hide resolved
settings: settings,
ac: ac,
}, nil
Expand All @@ -73,9 +68,6 @@ func (d *PhlareDatasource) CallResource(ctx context.Context, req *backend.CallRe
if req.Path == "labelValues" {
return d.labelValues(ctx, req, sender)
}
if req.Path == "backendType" {
return d.backendType(ctx, req, sender)
}
return sender.Send(&backend.CallResourceResponse{
Status: 404,
})
Expand All @@ -98,21 +90,7 @@ func (d *PhlareDatasource) profileTypes(ctx context.Context, req *backend.CallRe
}

func (d *PhlareDatasource) labelNames(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error {
u, err := url.Parse(req.URL)
if err != nil {
return err
}
query := u.Query()
start, err := strconv.ParseInt(query["start"][0], 10, 64)
if err != nil {
return err
}
end, err := strconv.ParseInt(query["end"][0], 10, 64)
if err != nil {
return err
}

res, err := d.client.LabelNames(ctx, query["query"][0], start, end)
Copy link
Member

@petethepig petethepig Sep 19, 2023

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?

Copy link
Member Author

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.

res, err := d.client.LabelNames(ctx)
if err != nil {
return fmt.Errorf("error calling LabelNames: %v", err)
}
Expand Down Expand Up @@ -140,16 +118,8 @@ func (d *PhlareDatasource) labelValues(ctx context.Context, req *backend.CallRes
return err
}
query := u.Query()
start, err := strconv.ParseInt(query["start"][0], 10, 64)
if err != nil {
return err
}
end, err := strconv.ParseInt(query["end"][0], 10, 64)
if err != nil {
return err
}

res, err := d.client.LabelValues(ctx, query["query"][0], query["label"][0], start, end)
res, err := d.client.LabelValues(ctx, query["label"][0])
if err != nil {
return fmt.Errorf("error calling LabelValues: %v", err)
}
Expand All @@ -164,74 +134,6 @@ func (d *PhlareDatasource) labelValues(ctx context.Context, req *backend.CallRes
return nil
}

type BackendTypeRespBody struct {
BackendType string `json:"backendType"` // "phlare" or "pyroscope"
}

// backendType is a simplistic test to figure out if we are speaking to phlare or pyroscope backend
func (d *PhlareDatasource) backendType(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error {
// To prevent any user sending arbitrary URL for us to test with we allow this only for users who can edit the datasource
// as config page is where this is meant to be used.
ok, err := d.isUserAllowedToEditDatasource(ctx)
if err != nil {
return err
}

if !ok {
return sender.Send(&backend.CallResourceResponse{Headers: req.Headers, Status: 401})
}

u, err := url.Parse(req.URL)
if err != nil {
return err
}
query := u.Query()
body := &BackendTypeRespBody{BackendType: "unknown"}

// We take the url from the request query because the data source may not yet be saved in DB with the URL we want
// to test with (like when filling in the confgi page for the first time)
url := query["url"][0]

pyroClient := getClient("pyroscope", d.httpClient, url)
_, err = pyroClient.ProfileTypes(ctx)

if err == nil {
body.BackendType = "pyroscope"
} else {
phlareClient := getClient("phlare", d.httpClient, url)
_, err := phlareClient.ProfileTypes(ctx)
if err == nil {
body.BackendType = "phlare"
}
}

data, err := json.Marshal(body)
if err != nil {
return err
}

return sender.Send(&backend.CallResourceResponse{Body: data, Headers: req.Headers, Status: 200})
}

func (d *PhlareDatasource) isUserAllowedToEditDatasource(ctx context.Context) (bool, error) {
reqCtx := contexthandler.FromContext(ctx)
uidScope := datasources.ScopeProvider.GetResourceScopeUID(accesscontrol.Parameter(":uid"))

if reqCtx == nil || reqCtx.SignedInUser == nil {
return false, nil
}

ok, err := d.ac.Evaluate(ctx, reqCtx.SignedInUser, accesscontrol.EvalPermission(datasources.ActionWrite, uidScope))
if err != nil {
return false, err
}
if !ok {
return false, nil
}

return true, nil
}

// QueryData handles multiple queries and returns multiple responses.
// req contains the queries []DataQuery (where each query contains RefID as a unique identifier).
// The QueryDataResponse contains a map of RefID to the response for each query, and each response
Expand Down
Loading
Loading