Skip to content

ref(profiling): Merge profile provider for both modes #83338

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

Merged
merged 1 commit into from
Jan 14, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export function SpanEvidenceSection({event, organization, projectSlug}: Props) {
<ProfilesProvider
orgSlug={organization.slug}
projectSlug={projectSlug}
profileId={profileId || ''}
profileMeta={profileId || ''}
>
<ProfileContext.Consumer>
{profiles => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ import {
import {formatTo} from 'sentry/utils/profiling/units/units';
import {useDevicePixelRatio} from 'sentry/utils/useDevicePixelRatio';
import {useMemoWithPrevious} from 'sentry/utils/useMemoWithPrevious';
import {
useContinuousProfile,
useContinuousProfileSegment,
} from 'sentry/views/profiling/continuousProfileProvider';
import {useProfileGroup} from 'sentry/views/profiling/profileGroupProvider';
import {
useProfiles,
useProfileTransaction,
} from 'sentry/views/profiling/profilesProvider';

import {FlamegraphDrawer} from './flamegraphDrawer/flamegraphDrawer';
import {FlamegraphWarnings} from './flamegraphOverlays/FlamegraphWarnings';
Expand Down Expand Up @@ -246,9 +246,9 @@ export function ContinuousFlamegraph(): ReactElement {
const devicePixelRatio = useDevicePixelRatio();
const dispatch = useDispatchFlamegraphState();

const profiles = useContinuousProfile();
const profiles = useProfiles();
const profileGroup = useProfileGroup();
const segment = useContinuousProfileSegment();
const segment = useProfileTransaction();

const configSpaceQueryParam = useMemo(() => decodeConfigSpace(), []);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {act, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
import ProjectsStore from 'sentry/stores/projectsStore';
import {useParams} from 'sentry/utils/useParams';
import ProfileFlamegraph from 'sentry/views/profiling/profileFlamechart';
import ProfilesAndTransactionProvider from 'sentry/views/profiling/profilesProvider';
import ProfilesAndTransactionProvider from 'sentry/views/profiling/transactionProfileProvider';

jest.mock('sentry/utils/useParams', () => ({
useParams: jest.fn(),
Expand Down
9 changes: 1 addition & 8 deletions static/app/components/profiling/flamegraph/flamegraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ import {useProfileGroup} from 'sentry/views/profiling/profileGroupProvider';
import {
useProfiles,
useProfileTransaction,
useSetProfiles,
} from 'sentry/views/profiling/profilesProvider';

import {FlamegraphDrawer} from './flamegraphDrawer/flamegraphDrawer';
Expand Down Expand Up @@ -219,7 +218,6 @@ function Flamegraph(): ReactElement {
const dispatch = useDispatchFlamegraphState();

const profiles = useProfiles();
const setProfiles = useSetProfiles();
const profileGroup = useProfileGroup();

const flamegraphTheme = useFlamegraphTheme();
Expand Down Expand Up @@ -1314,12 +1312,7 @@ function Flamegraph(): ReactElement {
[dispatch]
);

const onImport = useCallback(
(p: Profiling.ProfileInput) => {
setProfiles({type: 'resolved', data: p});
},
[setProfiles]
);
const onImport = useCallback(() => {}, []);
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 is actually broken, so l disabled it for now and we'll see if we can add it back later or remove it entirely.

Copy link
Member

Choose a reason for hiding this comment

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

Safe to remove imo, I can take care of it


useEffect(() => {
if (defined(flamegraphProfiles.threadId)) {
Expand Down
4 changes: 3 additions & 1 deletion static/app/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2183,7 +2183,9 @@ function buildRoutes() {
</Route>
<Route
path="profile/:projectId/:eventId/"
component={make(() => import('sentry/views/profiling/profilesProvider'))}
component={make(
() => import('sentry/views/profiling/transactionProfileProvider')
)}
>
<Route
path="flamegraph/"
Expand Down
2 changes: 1 addition & 1 deletion static/app/views/discover/eventDetails/content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ function EventDetailsContent(props: Props) {
<ProfilesProvider
orgSlug={organization.slug}
projectSlug={projectId}
profileId={profileId || ''}
profileMeta={profileId || ''}
>
<ProfileContext.Consumer>
{profiles => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export function MissingInstrumentationNodeDetails(
<ProfilesProvider
orgSlug={organization.slug}
projectSlug={event?.projectSlug ?? ''}
profileId={profileId || ''}
profileMeta={profileId || ''}
>
<ProfileContext.Consumer>
{profiles => (
Expand Down Expand Up @@ -191,7 +191,7 @@ function LegacyMissingInstrumentationNodeDetails({
<ProfilesProvider
orgSlug={organization.slug}
projectSlug={node.event?.projectSlug ?? ''}
profileId={profileId || ''}
profileMeta={profileId || ''}
>
<ProfileContext.Consumer>
{profiles => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export function SpanNodeDetails({
<ProfilesProvider
orgSlug={organization.slug}
projectSlug={node.event?.projectSlug}
profileId={profileId || ''}
profileMeta={profileId || ''}
>
<ProfileContext.Consumer>
{profiles => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ function NewTraceDetailsTransactionBar(props: Props) {
<ProfilesProvider
orgSlug={organization.slug}
projectSlug={embeddedChildren.projectSlug ?? ''}
profileId={profileId || ''}
profileMeta={profileId || ''}
>
<ProfileContext.Consumer>
{profiles => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ function SpanDetailsBody({
<ProfilesProvider
orgSlug={organization.slug}
projectSlug={detail.event.projectSlug}
profileId={profileId || ''}
profileMeta={profileId || ''}
>
<ProfileContext.Consumer>
{profiles => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ function EventDetailsContent(props: Props) {
<ProfilesProvider
orgSlug={organization.slug}
projectSlug={projectId}
profileId={profileId || ''}
profileMeta={profileId || ''}
>
<ProfileContext.Consumer>
{profiles => (
Expand Down
4 changes: 2 additions & 2 deletions static/app/views/profiling/continuousProfileFlamegraph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ import useOrganization from 'sentry/utils/useOrganization';
import {useParams} from 'sentry/utils/useParams';
import {ProfileGroupProvider} from 'sentry/views/profiling/profileGroupProvider';

import {useContinuousProfile} from './continuousProfileProvider';
import {useProfiles} from './profilesProvider';

function ContinuousProfileFlamegraph(): React.ReactElement {
const organization = useOrganization();
const profiles = useContinuousProfile();
const profiles = useProfiles();
const params = useParams();

const [storedPreferences] = useLocalStorageState<DeepPartial<FlamegraphState>>(
Expand Down
137 changes: 15 additions & 122 deletions static/app/views/profiling/continuousProfileProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,69 +1,15 @@
import {createContext, useContext, useLayoutEffect, useMemo, useState} from 'react';
import * as Sentry from '@sentry/react';
import {useMemo, useState} from 'react';

import type {Client} from 'sentry/api';
import {ContinuousProfileHeader} from 'sentry/components/profiling/continuousProfileHeader';
import type {RequestState} from 'sentry/types/core';
import type {EventTransaction} from 'sentry/types/event';
import type {Organization} from 'sentry/types/organization';
import type {Project} from 'sentry/types/project';
import {useSentryEvent} from 'sentry/utils/profiling/hooks/useSentryEvent';
import {decodeScalar} from 'sentry/utils/queryString';
import useApi from 'sentry/utils/useApi';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import {useParams} from 'sentry/utils/useParams';
import useProjects from 'sentry/utils/useProjects';

interface ContinuousProfileQueryParams {
end: string;
profiler_id: string;
start: string;
}

function fetchContinuousProfileFlamegraph(
api: Client,
query: ContinuousProfileQueryParams,
projectSlug: Project['slug'],
orgSlug: Organization['slug']
): Promise<Profiling.ProfileInput> {
return api
.requestPromise(`/organizations/${orgSlug}/profiling/chunks/`, {
method: 'GET',
query: {
...query,
project: projectSlug,
},
includeAllArgs: true,
})
.then(([data]) => data.chunk);
}

type ContinuousProfileProviderValue = RequestState<Profiling.ProfileInput>;
export const ContinuousProfileContext =
createContext<ContinuousProfileProviderValue | null>(null);

export function useContinuousProfile() {
const context = useContext(ContinuousProfileContext);
if (!context) {
throw new Error('useContinuousProfile was called outside of ProfileProvider');
}
return context;
}

type ContinuousProfileSegmentProviderValue = RequestState<EventTransaction>;
export const ContinuousProfileSegmentContext =
createContext<ContinuousProfileSegmentProviderValue | null>(null);

export function useContinuousProfileSegment() {
const context = useContext(ContinuousProfileSegmentContext);
if (!context) {
throw new Error(
'useContinuousProfileSegment was called outside of ContinuousProfileSegmentProvider'
);
}
return context;
}
import {ContinuousProfileProvider, ProfileTransactionContext} from './profilesProvider';

function isValidDate(date: string): boolean {
return !isNaN(Date.parse(date));
Expand All @@ -73,7 +19,9 @@ interface FlamegraphViewProps {
children: React.ReactNode;
}

function ProfilesAndTransactionProvider(props: FlamegraphViewProps): React.ReactElement {
export default function ProfileAndTransactionProvider(
props: FlamegraphViewProps
): React.ReactElement {
const organization = useOrganization();
const params = useParams();
const location = useLocation();
Expand All @@ -100,88 +48,33 @@ function ProfilesAndTransactionProvider(props: FlamegraphViewProps): React.React
};
}, [location.query.start, location.query.end, location.query.profilerId]);

const [profile, setProfile] = useState<RequestState<Profiling.ProfileInput>>({
type: 'initial',
});

const profileTransaction = useSentryEvent<EventTransaction>(
organization.slug,
projectSlug!,
decodeScalar(location.query.eventId) || null
);

return (
<ProfilesProvider
<ContinuousProfileProvider
orgSlug={organization.slug}
profileMeta={profileMeta}
projectSlug={projectSlug}
profile={profile}
setProfile={setProfile}
>
<ContinuousProfileSegmentContext.Provider value={profileTransaction}>
<ProfileTransactionContext.Provider value={profileTransaction}>
<ContinuousProfileHeader
projectId={projectSlug}
transaction={
profileTransaction.type === 'resolved' ? profileTransaction.data : null
}
/>
{props.children}
</ContinuousProfileSegmentContext.Provider>
</ProfilesProvider>
</ProfileTransactionContext.Provider>
</ContinuousProfileProvider>
);
}

interface ProfilesProviderProps {
children: React.ReactNode;
orgSlug: Organization['slug'];
profileMeta: ContinuousProfileQueryParams | null;
projectSlug: Project['slug'];
onUpdateProfiles?: (profiles: RequestState<Profiling.ProfileInput>) => void;
}

export function ProfilesProvider({
children,
onUpdateProfiles,
orgSlug,
profileMeta,
projectSlug,
}: ProfilesProviderProps) {
const api = useApi();
const {projects} = useProjects();

const [profiles, setProfiles] = useState<RequestState<Profiling.ProfileInput>>({
type: 'initial',
});

useLayoutEffect(() => {
if (!profileMeta) {
Sentry.captureMessage(
'Failed to fetch continuous profile - invalid chunk parameters.'
);
return undefined;
}

const project = projects.find(p => p.slug === projectSlug);
if (!project) {
Sentry.captureMessage('Failed to fetch continuous profile - project not found.');
return undefined;
}

setProfiles({type: 'loading'});

fetchContinuousProfileFlamegraph(api, profileMeta, project.id, orgSlug)
.then(p => {
setProfiles({type: 'resolved', data: p});
onUpdateProfiles?.({type: 'resolved', data: p});
})
.catch(err => {
setProfiles({type: 'errored', error: 'Failed to fetch profiles'});
onUpdateProfiles?.({type: 'errored', error: 'Failed to fetch profiles'});
Sentry.captureException(err);
});

return () => api.clear();
}, [api, onUpdateProfiles, profileMeta, orgSlug, projectSlug, projects]);

return (
<ContinuousProfileContext.Provider value={profiles}>
{children}
</ContinuousProfileContext.Provider>
);
}

export default ProfilesAndTransactionProvider;
Loading
Loading