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

feat(*): Various minor improvements #46

Merged
merged 3 commits into from
Jul 12, 2024
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
9 changes: 9 additions & 0 deletions e2e/config/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,13 @@ export const DEFAULT_URL_PARAMS = ENV_VARS.E2E_BASE_URL.startsWith('http://local
})
: new URLSearchParams();

export const DEFAULT_EXPLORE_PROFILES_URL_PARAMS = ENV_VARS.E2E_BASE_URL.startsWith('http://localhost')
? new URLSearchParams({
// We use static data in local and PR build (where the host is http://localhost):
from: '2024-03-13T18:00:00.000Z',
to: '2024-03-13T18:50:00.000Z',
maxNodes: '16384',
})
: new URLSearchParams();

export const AUTH_FILE = path.join(process.cwd(), 'e2e', 'auth', 'user.json');
10 changes: 9 additions & 1 deletion e2e/fixtures/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { test as base, expect } from '@playwright/test';

import { DEFAULT_URL_PARAMS } from '../config/constants';
import { DEFAULT_EXPLORE_PROFILES_URL_PARAMS, DEFAULT_URL_PARAMS } from '../config/constants';
import { Toolbar } from './components/Toolbar';
import { AdHocViewPage } from './pages/AdHocViewPage';
import { ComparisonDiffViewPage } from './pages/ComparisonDiffViewPage';
import { ComparisonViewPage } from './pages/ComparisonViewPage';
import { ExploreProfilesPage } from './pages/ExploreProfilesPage';
import { SettingsPage } from './pages/SettingsPage';

type Fixtures = {
toolbar: Toolbar;
exploreProfilesPage: ExploreProfilesPage;
comparisonViewPage: ComparisonViewPage;
comparisonDiffViewPage: ComparisonDiffViewPage;
adHocViewPage: AdHocViewPage;
Expand Down Expand Up @@ -41,6 +43,12 @@ export const test = base.extend<Options & Fixtures>({
toolbar: async ({ page }, use) => {
await use(new Toolbar(page));
},
exploreProfilesPage: async ({ page, failOnUncaughtExceptions }, use) => {
await withExceptionsAssertion(
{ page, failOnUncaughtExceptions, use },
new ExploreProfilesPage(page, DEFAULT_EXPLORE_PROFILES_URL_PARAMS)
);
},
comparisonViewPage: async ({ page, failOnUncaughtExceptions }, use) => {
await withExceptionsAssertion(
{ page, failOnUncaughtExceptions, use },
Expand Down
20 changes: 20 additions & 0 deletions e2e/fixtures/pages/ExploreProfilesPage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { type Page } from '@playwright/test';

import { PyroscopePage } from './PyroscopePage';

export class ExploreProfilesPage extends PyroscopePage {
constructor(readonly page: Page, defaultUrlParams: URLSearchParams) {
const urlParams = new URLSearchParams(defaultUrlParams);

super(page, '/a/grafana-pyroscope-app/profiles-explorer', urlParams.toString());
}

getExplorationTypeSelector() {
return this.page.getByTestId('exploration-types');
}

async getSelectedExplorationType() {
const label = await this.getExplorationTypeSelector().locator('input[checked] + label').textContent();
return label?.trim();
}
}
23 changes: 23 additions & 0 deletions e2e/tests/explore-profiles/explore-profiles.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { DEFAULT_EXPLORE_PROFILES_URL_PARAMS } from '../../config/constants';
import { expect, test } from '../../fixtures';

test.describe('Explore Profiles', () => {
test.describe('Smoke tests', () => {
for (const { type, label } of [
{ type: 'all', label: 'All services' },
{ type: 'profiles', label: 'Profile types' },
{ type: 'labels', label: 'Labels' },
{ type: 'flame-graph', label: 'Flame graph' },
{ type: 'favorites', label: 'Favorites' },
]) {
test(label, async ({ exploreProfilesPage }) => {
const urlParams = new URLSearchParams(DEFAULT_EXPLORE_PROFILES_URL_PARAMS);

urlParams.set('explorationType', type);
await exploreProfilesPage.goto(urlParams.toString());

expect(await exploreProfilesPage.getSelectedExplorationType()).toBe(label);
});
}
});
});
80 changes: 28 additions & 52 deletions src/pages/ProfilesExplorerView/SceneProfilesExplorer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { EventViewServiceProfiles } from './events/EventViewServiceProfiles';
import { SceneExploreAllServices } from './exploration-types/SceneExploreAllServices/SceneExploreAllServices';
import { SceneExploreFavorites } from './exploration-types/SceneExploreFavorites/SceneExploreFavorites';
import { SceneExploreServiceLabels } from './exploration-types/SceneExploreServiceLabels/SceneExploreServiceLabels';
import { SceneExploreSingleService } from './exploration-types/SceneExploreSingleService/SceneExploreSingleService';
import { SceneExploreServiceProfileTypes } from './exploration-types/SceneExploreServiceProfileTypes/SceneExploreServiceProfileTypes';
import { SceneServiceFlameGraph } from './exploration-types/SceneServiceFlameGraph/SceneServiceFlameGraph';
import { findSceneObjectByClass } from './helpers/findSceneObjectByClass';
import { FiltersVariable } from './variables/FiltersVariable/FiltersVariable';
Expand All @@ -53,9 +53,9 @@ export interface SceneProfilesExplorerState extends Partial<EmbeddedSceneState>

export enum ExplorationType {
ALL_SERVICES = 'all',
SINGLE_SERVICE = 'single',
SINGLE_SERVICE_LABELS = 'labels',
SINGLE_SERVICE_FLAME_GRAPH = 'flame-graph',
PROFILE_TYPES = 'profiles',
LABELS = 'labels',
FLAME_GRAPH = 'flame-graph',
FAVORITES = 'favorites',
}

Expand All @@ -64,27 +64,27 @@ export class SceneProfilesExplorer extends SceneObjectBase<SceneProfilesExplorer
{
value: ExplorationType.ALL_SERVICES,
label: 'All services',
description: '', // no tooltip (see src/pages/ProfilesExplorerView/SceneProfilesExplorer/ExplorationTypeSelector.tsx)
description: 'Overview of all services, for any given profile type',
},
{
value: ExplorationType.SINGLE_SERVICE,
label: 'Single service',
description: '',
value: ExplorationType.PROFILE_TYPES,
label: 'Profile types',
description: 'Overview of all the profile types for a single service',
},
{
value: ExplorationType.SINGLE_SERVICE_LABELS,
value: ExplorationType.LABELS,
label: 'Labels',
description: '',
description: 'Single service label exploration and filtering',
},
{
value: ExplorationType.SINGLE_SERVICE_FLAME_GRAPH,
value: ExplorationType.FLAME_GRAPH,
label: 'Flame graph',
description: '',
description: 'Single service flame graph',
},
{
value: ExplorationType.FAVORITES,
label: 'Favorites',
description: '',
description: 'Overview of favorited visualizations',
},
];

Expand Down Expand Up @@ -177,19 +177,19 @@ export class SceneProfilesExplorer extends SceneObjectBase<SceneProfilesExplorer
const profilesSub = this.subscribeToEvent(EventViewServiceProfiles, (event) => {
(findSceneObjectByClass(this, SceneQuickFilter) as SceneQuickFilter)?.clear();

this.setExplorationType(ExplorationType.SINGLE_SERVICE, event.payload.item);
this.setExplorationType(ExplorationType.PROFILE_TYPES, event.payload.item);
});

const labelsSub = this.subscribeToEvent(EventViewServiceLabels, (event) => {
(findSceneObjectByClass(this, SceneQuickFilter) as SceneQuickFilter)?.clear();

this.setExplorationType(ExplorationType.SINGLE_SERVICE_LABELS, event.payload.item);
this.setExplorationType(ExplorationType.LABELS, event.payload.item);
});

const flameGraphSub = this.subscribeToEvent(EventViewServiceFlameGraph, (event) => {
(findSceneObjectByClass(this, SceneQuickFilter) as SceneQuickFilter)?.clear();

this.setExplorationType(ExplorationType.SINGLE_SERVICE_FLAME_GRAPH, event.payload.item);
this.setExplorationType(ExplorationType.FLAME_GRAPH, event.payload.item);
});

return {
Expand All @@ -216,17 +216,17 @@ export class SceneProfilesExplorer extends SceneObjectBase<SceneProfilesExplorer
let primary;

switch (explorationType) {
case ExplorationType.SINGLE_SERVICE:
primary = new SceneExploreSingleService();
case ExplorationType.PROFILE_TYPES:
primary = new SceneExploreServiceProfileTypes();

this.updateQuickFilterPlaceholder('Search profile metrics (comma-separated regexes are supported)');
this.updateQuickFilterPlaceholder('Search profile types (comma-separated regexes are supported)');
break;

case ExplorationType.SINGLE_SERVICE_LABELS:
case ExplorationType.LABELS:
primary = new SceneExploreServiceLabels();
break;

case ExplorationType.SINGLE_SERVICE_FLAME_GRAPH:
case ExplorationType.FLAME_GRAPH:
primary = new SceneServiceFlameGraph();
break;

Expand Down Expand Up @@ -303,14 +303,14 @@ export class SceneProfilesExplorer extends SceneObjectBase<SceneProfilesExplorer
gridControls: this.state.gridControls,
};

case ExplorationType.SINGLE_SERVICE:
case ExplorationType.PROFILE_TYPES:
return {
variables: [dataSourceVariable, serviceNameVariable],
gridControls: this.state.gridControls,
};

case ExplorationType.SINGLE_SERVICE_LABELS:
case ExplorationType.SINGLE_SERVICE_FLAME_GRAPH:
case ExplorationType.LABELS:
case ExplorationType.FLAME_GRAPH:
return {
// note that SceneGroupByLabels will directly get groupByVariable and gridControls as the layout is a bit different
variables: [dataSourceVariable, serviceNameVariable, profileMetricVariable, filtersVariable],
Expand All @@ -334,9 +334,7 @@ export class SceneProfilesExplorer extends SceneObjectBase<SceneProfilesExplorer
GroupByVariable.DEFAULT_VALUE
);

if (
![ExplorationType.SINGLE_SERVICE_LABELS, ExplorationType.SINGLE_SERVICE_FLAME_GRAPH].includes(explorationType)
) {
if (![ExplorationType.LABELS, ExplorationType.FLAME_GRAPH].includes(explorationType)) {
(findSceneObjectByClass(this, FiltersVariable) as FiltersVariable)?.setState({
filters: FiltersVariable.DEFAULT_VALUE,
});
Expand Down Expand Up @@ -371,37 +369,15 @@ export class SceneProfilesExplorer extends SceneObjectBase<SceneProfilesExplorer
<>
<div className={styles.header}>
<div className={styles.controls}>
<Stack justifyContent="space-between" wrap="wrap">
<Stack justifyContent="space-between">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This unfortunately does not completely fix #41, the time picker does not shrink on narrow screen. I will have to investigate further why

<Stack>
<div className={styles.dataSourceVariable}>
<InlineLabel width="auto">{dataSourceVariable.state.label}</InlineLabel>
<dataSourceVariable.Component model={dataSourceVariable} />
</div>

<div className={styles.explorationType}>
<InlineLabel
width="auto"
tooltip={
<div className={styles.tooltipContent}>
<h5>Types of exploration</h5>
<dl>
<dt>All services</dt>
<dd>Overview of all services, for any given profile metric</dd>
<dt>Single service</dt>
<dd>Overview of all the profile metrics for a single service</dd>
<dt>Labels</dt>
<dd>Single service label exploration and filtering</dd>
<dt>Flame graph</dt>
<dd>Single service flame graph</dd>
<dt>Favorites</dt>
<dd>Overview of favorited visualizations</dd>
</dl>
</div>
}
>
Exploration type
</InlineLabel>

<div className={styles.explorationType} data-testid="exploration-types">
<InlineLabel width="auto">Exploration type</InlineLabel>
<RadioButtonGroup
options={SceneProfilesExplorer.EXPLORATION_TYPE_OPTIONS}
value={explorationType}
Expand Down
38 changes: 24 additions & 14 deletions src/pages/ProfilesExplorerView/actions/FavAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,30 +35,40 @@ export class FavAction extends SceneObjectBase<FavActionState> {
return FavoritesDataSource.exists(this.buildFavorite());
}

buildFavorite(): Favorite {
const { item, skipVariablesInterpolation } = this.state;

const queryRunnerParams = (
skipVariablesInterpolation ? item.queryRunnerParams : interpolateQueryRunnerVariables(this, item)
) as Favorite['queryRunnerParams'];
static buildFavorite(item: GridItemData): Favorite {
const { index, queryRunnerParams } = item;

const favorite: Favorite = {
index,
queryRunnerParams: {
serviceName: queryRunnerParams.serviceName as string,
profileMetricId: queryRunnerParams.profileMetricId as string,
},
};

if (queryRunnerParams.groupBy) {
queryRunnerParams.groupBy = {
favorite.queryRunnerParams.groupBy = {
label: queryRunnerParams.groupBy.label, // we don't store values, we'll fetch all timeseries by using the `groupBy` parameter
};
} else {
delete queryRunnerParams.groupBy;
}

// we don't store filters if empty
if (!queryRunnerParams.filters?.length) {
delete queryRunnerParams.filters;
if (queryRunnerParams.filters?.length) {
favorite.queryRunnerParams.filters = queryRunnerParams.filters;
}

return {
return favorite;
}

buildFavorite(): Favorite {
const { item, skipVariablesInterpolation } = this.state;

return FavAction.buildFavorite({
index: item.index,
queryRunnerParams,
};
queryRunnerParams: skipVariablesInterpolation
? item.queryRunnerParams
: interpolateQueryRunnerVariables(this, item),
} as GridItemData);
}

public onClick = () => {
Expand Down
Loading
Loading