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

Pyroscope: Preselect default profile type or app in the query editor dropdown #70624

Merged
merged 3 commits into from
Jun 28, 2023
Merged

Pyroscope: Preselect default profile type or app in the query editor dropdown #70624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3ca5c36
Select first profile type that comes in
aocenas Jun 23, 2023
685dc16
remove console log
aocenas Jun 23, 2023
54d5dbe
Try to select cpu profile
aocenas Jun 26, 2023
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
100 changes: 52 additions & 48 deletions public/app/plugins/datasource/grafana-pyroscope-datasource/QueryEditor/QueryEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { defaults } from 'lodash';
import deepEqual from 'fast-deep-equal';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { useAsync } from 'react-use';

import { CoreApp, QueryEditorProps, TimeRange } from '@grafana/data';
import { ButtonCascader, CascaderOption } from '@grafana/ui';

import { defaultGrafanaPyroscope, defaultPhlareQueryType, GrafanaPyroscope } from '../dataquery.gen';
import { PhlareDataSource } from '../datasource';
import { normalizeQuery, PhlareDataSource } from '../datasource';
import { BackendType, PhlareDataSourceOptions, ProfileTypeMessage, Query } from '../types';

import { EditorRow } from './EditorRow';
Expand All @@ -16,31 +15,18 @@ import { QueryOptions } from './QueryOptions';

export type Props = QueryEditorProps<PhlareDataSource, Query, PhlareDataSourceOptions>;

export const defaultQuery: Partial<GrafanaPyroscope> = {
...defaultGrafanaPyroscope,
queryType: defaultPhlareQueryType,
};

export function QueryEditor(props: Props) {
let query = normalizeQuery(props.query, props.app);
const { onChange, onRunQuery, datasource, query, range, app } = props;

function handleRunQuery(value: string) {
props.onChange({ ...props.query, labelSelector: value });
props.onRunQuery();
onChange({ ...query, labelSelector: value });
onRunQuery();
}

const { profileTypes, onProfileTypeChange, selectedProfileName } = useProfileTypes(
props.datasource,
props.query,
props.onChange,
props.datasource.backendType
);
const { labels, getLabelValues, onLabelSelectorChange } = useLabels(
props.range,
props.datasource,
props.query,
props.onChange
);
const { profileTypes, onProfileTypeChange, selectedProfileName } = useProfileTypes(datasource, query, onChange);
const { labels, getLabelValues, onLabelSelectorChange } = useLabels(range, datasource, query, onChange);
useNormalizeQuery(query, profileTypes, onChange, app);

const cascaderOptions = useCascaderOptions(profileTypes);

return (
Expand All @@ -64,6 +50,42 @@ export function QueryEditor(props: Props) {
);
}

function useNormalizeQuery(
query: Query,
profileTypes: ProfileTypeMessage[],
onChange: (value: Query) => void,
app?: CoreApp
) {
useEffect(() => {
const normalizedQuery = normalizeQuery(query, app);
// Query can be stored with some old type, or we can have query from different pyro datasource
const selectedProfile = query.profileTypeId && profileTypes.find((p) => p.id === query.profileTypeId);
if (profileTypes.length && !selectedProfile) {
normalizedQuery.profileTypeId = defaultProfileType(profileTypes);
}
// Makes sure we don't have an infinite loop updates because the normalization creates a new object
if (!deepEqual(query, normalizedQuery)) {
onChange(normalizedQuery);
}
}, [app, query, profileTypes, onChange]);
}

function defaultProfileType(profileTypes: ProfileTypeMessage[]): string {
const cpuProfiles = profileTypes.filter((p) => p.id.indexOf('cpu') >= 0);
if (cpuProfiles.length) {
// Prefer cpu time profile if available instead of samples
const cpuTimeProfile = cpuProfiles.find((p) => p.id.indexOf('samples') === -1);
if (cpuTimeProfile) {
return cpuTimeProfile.id;
}
// Fallback to first cpu profile type
return cpuProfiles[0].id;
}

// Fallback to first profile type from response data
return profileTypes[0].id;
}

function useLabels(
range: TimeRange | undefined,
datasource: PhlareDataSource,
Expand Down Expand Up @@ -138,12 +160,7 @@ function useCascaderOptions(profileTypes: ProfileTypeMessage[]) {
}, [profileTypes]);
}

function useProfileTypes(
datasource: PhlareDataSource,
query: Query,
onChange: (value: Query) => void,
backendType: BackendType = 'phlare'
) {
function useProfileTypes(datasource: PhlareDataSource, query: Query, onChange: (value: Query) => void) {
const [profileTypes, setProfileTypes] = useState<ProfileTypeMessage[]>([]);

useEffect(() => {
Expand All @@ -160,23 +177,20 @@ function useProfileTypes(
}

const id = selectedOptions[selectedOptions.length - 1].value;

// Probably cannot happen but makes TS happy
joey-grafana marked this conversation as resolved.
Show resolved Hide resolved
if (typeof id !== 'string') {
throw new Error('id is not string');
}

onChange({ ...query, profileTypeId: id });
},
[onChange, query]
);

const selectedProfileName = useProfileName(profileTypes, query.profileTypeId, backendType);

const selectedProfileName = useProfileName(profileTypes, query.profileTypeId, datasource.backendType);
return { profileTypes, onProfileTypeChange, selectedProfileName };
}

function useProfileName(profileTypes: ProfileTypeMessage[], profileTypeId: string, backendType: BackendType) {
function useProfileName(
profileTypes: ProfileTypeMessage[],
profileTypeId: string,
backendType: BackendType = 'phlare'
) {
return useMemo(() => {
if (!profileTypes) {
return 'Loading';
Expand All @@ -192,13 +206,3 @@ function useProfileName(profileTypes: ProfileTypeMessage[], profileTypeId: strin
return profile.label;
}, [profileTypeId, profileTypes, backendType]);
}

export function normalizeQuery(query: Query, app?: CoreApp | string) {
let normalized = defaults(query, defaultQuery);
if (app !== CoreApp.Explore && normalized.queryType === 'both') {
// In dashboards and other places, we can't show both types of graphs at the same time.
// This will also be a default when having 'both' query and adding it from explore to dashboard
normalized.queryType = 'profile';
}
return normalized;
}
22 changes: 21 additions & 1 deletion public/app/plugins/datasource/grafana-pyroscope-datasource/datasource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Observable, of } from 'rxjs';

import {
AbstractQuery,
CoreApp,
DataQueryRequest,
DataQueryResponse,
DataSourceInstanceSettings,
Expand All @@ -12,7 +13,7 @@ import { DataSourceWithBackend, getTemplateSrv, TemplateSrv } from '@grafana/run

import { extractLabelMatchers, toPromLikeExpr } from '../prometheus/language_utils';

import { normalizeQuery } from './QueryEditor/QueryEditor';
import { defaultGrafanaPyroscope, defaultPhlareQueryType } from './dataquery.gen';
import { PhlareDataSourceOptions, Query, ProfileTypeMessage, BackendType } from './types';

export class PhlareDataSource extends DataSourceWithBackend<Query, PhlareDataSourceOptions> {
Expand Down Expand Up @@ -101,6 +102,25 @@ export class PhlareDataSource extends DataSourceWithBackend<Query, PhlareDataSou
labelMatchers: extractLabelMatchers(tokens),
};
}

getDefaultQuery(app: CoreApp): Partial<Query> {
Copy link
Member Author

Choose a reason for hiding this comment

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

This makes initialization in explore a bit better so for the first render of query editor we get this instead of totally empty query object.

return defaultQuery;
}
}

export const defaultQuery: Partial<Query> = {
joey-grafana marked this conversation as resolved.
Show resolved Hide resolved
...defaultGrafanaPyroscope,
queryType: defaultPhlareQueryType,
};

export function normalizeQuery(query: Query, app?: CoreApp | string) {
let normalized = { ...query, ...defaultQuery };
if (app !== CoreApp.Explore && normalized.queryType === 'both') {
// In dashboards and other places, we can't show both types of graphs at the same time.
// This will also be a default when having 'both' query and adding it from explore to dashboard
normalized.queryType = 'profile';
}
return normalized;
}

const grammar: Grammar = {
Expand Down