Skip to content

Commit

Permalink
feat(eap): Adds enabled prop to SpanTagsProvider (getsentry#81188)
Browse files Browse the repository at this point in the history
Adds an `enabled` prop to `SpanTagsProvider` to control fetching tags
  • Loading branch information
edwardgou-sentry authored Nov 22, 2024
1 parent c758f93 commit 327f975
Show file tree
Hide file tree
Showing 11 changed files with 188 additions and 176 deletions.
14 changes: 12 additions & 2 deletions static/app/views/alerts/rules/metric/eapField.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import {OrganizationFixture} from 'sentry-fixture/organization';

import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';

import {DiscoverDatasets} from 'sentry/utils/discover/types';
import EAPField from 'sentry/views/alerts/rules/metric/eapField';
import {SpanTagsProvider} from 'sentry/views/explore/contexts/spanTagsContext';

describe('EAPField', () => {
const organization = OrganizationFixture();
Expand All @@ -16,7 +18,11 @@ describe('EAPField', () => {
});

it('renders', () => {
render(<EAPField aggregate={'count(span.duration)'} onChange={() => {}} />);
render(
<SpanTagsProvider dataset={DiscoverDatasets.SPANS_EAP} enabled>
<EAPField aggregate={'count(span.duration)'} onChange={() => {}} />
</SpanTagsProvider>
);
expect(fieldsMock).toHaveBeenCalledWith(
`/organizations/${organization.slug}/spans/fields/`,
expect.objectContaining({
Expand All @@ -35,7 +41,11 @@ describe('EAPField', () => {

it('should call onChange with the new aggregate string when switching aggregates', async () => {
const onChange = jest.fn();
render(<EAPField aggregate={'count(span.duration)'} onChange={onChange} />);
render(
<SpanTagsProvider dataset={DiscoverDatasets.SPANS_EAP} enabled>
<EAPField aggregate={'count(span.duration)'} onChange={onChange} />
</SpanTagsProvider>
);
expect(fieldsMock).toHaveBeenCalledWith(
`/organizations/${organization.slug}/spans/fields/`,
expect.objectContaining({
Expand Down
12 changes: 2 additions & 10 deletions static/app/views/alerts/rules/metric/eapField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,12 @@ import SelectControl from 'sentry/components/forms/controls/selectControl';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import {parseFunction} from 'sentry/utils/discover/fields';
import {DiscoverDatasets} from 'sentry/utils/discover/types';
import {ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields';
import {
DEFAULT_EAP_FIELD,
DEFAULT_EAP_METRICS_ALERT_FIELD,
} from 'sentry/utils/metrics/mri';
import {
SpanTagsProvider,
useSpanTags,
} from 'sentry/views/explore/contexts/spanTagsContext';
import {useSpanTags} from 'sentry/views/explore/contexts/spanTagsContext';

interface Props {
aggregate: string;
Expand All @@ -30,11 +26,7 @@ const OPERATIONS = [
];

function EAPFieldWrapper({aggregate, onChange}: Props) {
return (
<SpanTagsProvider dataset={DiscoverDatasets.SPANS_EAP}>
<EAPField aggregate={aggregate} onChange={onChange} />
</SpanTagsProvider>
);
return <EAPField aggregate={aggregate} onChange={onChange} />;
}

function EAPField({aggregate, onChange}: Props) {
Expand Down
306 changes: 156 additions & 150 deletions static/app/views/alerts/rules/metric/ruleConditionsForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -678,73 +678,79 @@ class RuleConditionsForm extends PureComponent<Props, State> {
</Fragment>
) : (
<Fragment>
{isExtrapolatedChartData && (
<OnDemandMetricAlert
message={t(
'The chart data above is an estimate based on the stored transactions that match the filters specified.'
)}
/>
)}
{hasActivatedAlerts && this.renderMonitorTypeSelect()}
{!isErrorMigration && this.renderInterval()}
<StyledListItem>{t('Filter events')}</StyledListItem>
<FormRow noMargin columns={1 + (allowChangeEventTypes ? 1 : 0) + 1}>
{this.renderProjectSelector()}
<SelectField
name="environment"
placeholder={t('All Environments')}
style={{
...this.formElemBaseStyle,
minWidth: 230,
flex: 1,
}}
styles={{
singleValue: (base: any) => ({
...base,
}),
option: (base: any) => ({
...base,
}),
}}
options={environmentOptions}
isDisabled={
disabled || this.state.environments === null || isErrorMigration
}
isClearable
inline={false}
flexibleControlStateSize
/>
{allowChangeEventTypes && this.renderEventTypeFilter()}
</FormRow>
<FormRow noMargin>
<FormField
name="query"
inline={false}
style={{
...this.formElemBaseStyle,
flex: '6 0 500px',
}}
flexibleControlStateSize
>
{({onChange, onBlur, initialData, value}) => {
return (hasCustomMetrics(organization) &&
alertType === 'custom_metrics') ||
alertType === 'insights_metrics' ? (
<MetricSearchBar
mri={getMRI(aggregate)}
projectIds={[project.id]}
placeholder={this.searchPlaceholder}
query={initialData.query}
defaultQuery={initialData?.query ?? ''}
useFormWrapper={false}
searchSource="alert_builder"
onChange={query => {
onFilterSearch(query, true);
onChange(query, {});
}}
/>
) : alertType === 'eap_metrics' ? (
<SpanTagsProvider dataset={DiscoverDatasets.SPANS_EAP}>
<SpanTagsProvider
dataset={DiscoverDatasets.SPANS_EAP}
enabled={
organization.features.includes('alerts-eap') &&
alertType === 'eap_metrics'
}
>
{isExtrapolatedChartData && (
<OnDemandMetricAlert
message={t(
'The chart data above is an estimate based on the stored transactions that match the filters specified.'
)}
/>
)}
{hasActivatedAlerts && this.renderMonitorTypeSelect()}
{!isErrorMigration && this.renderInterval()}
<StyledListItem>{t('Filter events')}</StyledListItem>
<FormRow noMargin columns={1 + (allowChangeEventTypes ? 1 : 0) + 1}>
{this.renderProjectSelector()}
<SelectField
name="environment"
placeholder={t('All Environments')}
style={{
...this.formElemBaseStyle,
minWidth: 230,
flex: 1,
}}
styles={{
singleValue: (base: any) => ({
...base,
}),
option: (base: any) => ({
...base,
}),
}}
options={environmentOptions}
isDisabled={
disabled || this.state.environments === null || isErrorMigration
}
isClearable
inline={false}
flexibleControlStateSize
/>
{allowChangeEventTypes && this.renderEventTypeFilter()}
</FormRow>
<FormRow noMargin>
<FormField
name="query"
inline={false}
style={{
...this.formElemBaseStyle,
flex: '6 0 500px',
}}
flexibleControlStateSize
>
{({onChange, onBlur, initialData, value}) => {
return (hasCustomMetrics(organization) &&
alertType === 'custom_metrics') ||
alertType === 'insights_metrics' ? (
<MetricSearchBar
mri={getMRI(aggregate)}
projectIds={[project.id]}
placeholder={this.searchPlaceholder}
query={initialData.query}
defaultQuery={initialData?.query ?? ''}
useFormWrapper={false}
searchSource="alert_builder"
onChange={query => {
onFilterSearch(query, true);
onChange(query, {});
}}
/>
) : alertType === 'eap_metrics' ? (
<SpanTagsContext.Consumer>
{tags => (
<EAPSpanSearchQueryBuilder
Expand All @@ -765,92 +771,92 @@ class RuleConditionsForm extends PureComponent<Props, State> {
/>
)}
</SpanTagsContext.Consumer>
</SpanTagsProvider>
) : (
<SearchContainer>
<SearchQueryBuilder
initialQuery={initialData?.query ?? ''}
getTagValues={this.getEventFieldValues}
placeholder={this.searchPlaceholder}
searchSource="alert_builder"
filterKeys={filterKeys}
disabled={disabled || isErrorMigration}
onChange={onChange}
invalidMessages={{
[InvalidReason.WILDCARD_NOT_ALLOWED]: t(
'The wildcard operator is not supported here.'
),
[InvalidReason.FREE_TEXT_NOT_ALLOWED]: t(
'Free text search is not allowed. If you want to partially match transaction names, use glob patterns like "transaction:*transaction-name*"'
),
}}
onSearch={query => {
onFilterSearch(query, true);
onChange(query, {});
}}
onBlur={(query, {parsedQuery}) => {
onFilterSearch(query, parsedQuery);
onBlur(query);
}}
// We only need strict validation for Transaction queries, everything else is fine
disallowUnsupportedFilters={
organization.features.includes('alert-allow-indexed') ||
(hasOnDemandMetricAlertFeature(organization) &&
isOnDemandQueryString(value))
? false
: dataset === Dataset.GENERIC_METRICS
}
/>
{isExtrapolatedChartData && isOnDemandQueryString(value) && (
<OnDemandWarningIcon
color="gray500"
msg={tct(
`We don’t routinely collect metrics from [fields]. However, we’ll do so [strong:once this alert has been saved.]`,
{
fields: (
<strong>
{getOnDemandKeys(value)
.map(key => `"${key}"`)
.join(', ')}
</strong>
),
strong: <strong />,
}
)}
) : (
<SearchContainer>
<SearchQueryBuilder
initialQuery={initialData?.query ?? ''}
getTagValues={this.getEventFieldValues}
placeholder={this.searchPlaceholder}
searchSource="alert_builder"
filterKeys={filterKeys}
disabled={disabled || isErrorMigration}
onChange={onChange}
invalidMessages={{
[InvalidReason.WILDCARD_NOT_ALLOWED]: t(
'The wildcard operator is not supported here.'
),
[InvalidReason.FREE_TEXT_NOT_ALLOWED]: t(
'Free text search is not allowed. If you want to partially match transaction names, use glob patterns like "transaction:*transaction-name*"'
),
}}
onSearch={query => {
onFilterSearch(query, true);
onChange(query, {});
}}
onBlur={(query, {parsedQuery}) => {
onFilterSearch(query, parsedQuery);
onBlur(query);
}}
// We only need strict validation for Transaction queries, everything else is fine
disallowUnsupportedFilters={
organization.features.includes('alert-allow-indexed') ||
(hasOnDemandMetricAlertFeature(organization) &&
isOnDemandQueryString(value))
? false
: dataset === Dataset.GENERIC_METRICS
}
/>
)}
</SearchContainer>
);
}}
</FormField>
</FormRow>
<FormRow noMargin>
<FormField
name="query"
inline={false}
style={{
...this.formElemBaseStyle,
flex: '6 0 500px',
}}
flexibleControlStateSize
>
{args => {
if (
args.value?.includes('is:unresolved') &&
comparisonType === AlertRuleComparisonType.DYNAMIC
) {
return (
<OnDemandMetricAlert
message={t(
"'is:unresolved' queries are not supported by Anomaly Detection alerts."
{isExtrapolatedChartData && isOnDemandQueryString(value) && (
<OnDemandWarningIcon
color="gray500"
msg={tct(
`We don’t routinely collect metrics from [fields]. However, we’ll do so [strong:once this alert has been saved.]`,
{
fields: (
<strong>
{getOnDemandKeys(value)
.map(key => `"${key}"`)
.join(', ')}
</strong>
),
strong: <strong />,
}
)}
/>
)}
/>
</SearchContainer>
);
}
return null;
}}
</FormField>
</FormRow>
}}
</FormField>
</FormRow>
<FormRow noMargin>
<FormField
name="query"
inline={false}
style={{
...this.formElemBaseStyle,
flex: '6 0 500px',
}}
flexibleControlStateSize
>
{args => {
if (
args.value?.includes('is:unresolved') &&
comparisonType === AlertRuleComparisonType.DYNAMIC
) {
return (
<OnDemandMetricAlert
message={t(
"'is:unresolved' queries are not supported by Anomaly Detection alerts."
)}
/>
);
}
return null;
}}
</FormField>
</FormRow>
</SpanTagsProvider>
</Fragment>
)}
</Fragment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function renderWithProvider({
onSearch,
}: ComponentProps<typeof SpansSearchBar>) {
return render(
<SpanTagsProvider dataset={DiscoverDatasets.SPANS_EAP}>
<SpanTagsProvider dataset={DiscoverDatasets.SPANS_EAP} enabled>
<SpansSearchBar widgetQuery={widgetQuery} onSearch={onSearch} />
</SpanTagsProvider>
);
Expand Down
Loading

0 comments on commit 327f975

Please sign in to comment.