Skip to content

Commit

Permalink
Merging in main
Browse files Browse the repository at this point in the history
  • Loading branch information
ymao1 committed Sep 9, 2024
2 parents feb2b59 + e02e8fc commit e6da1db
Show file tree
Hide file tree
Showing 23 changed files with 430 additions and 132 deletions.
4 changes: 4 additions & 0 deletions .buildkite/scripts/steps/capture_oas_snapshot.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,9 @@ if is_pr && ! is_auto_commit_disabled; then
cmd="$cmd --update"
fi

if [[ $BUILDKITE_PULL_REQUEST != "false" && "$BUILDKITE_PULL_REQUEST_BASE_BRANCH" != "main" ]] || [[ $BUILDKITE_PULL_REQUEST == "false" && "$BUILDKITE_BRANCH" != "main" ]]; then
cmd="$cmd --no-serverless"
fi

eval "$cmd"
check_for_changed_files "$cmd" true
Original file line number Diff line number Diff line change
Expand Up @@ -550,5 +550,109 @@ describe('autocomplete.suggest', () => {
{ triggerCharacter: ' ' }
);
});

test('case', async () => {
const { assertSuggestions } = await setup();
const comparisonOperators = ['==', '!=', '>', '<', '>=', '<=']
.map((op) => `${op} `)
.concat(',');

// case( / ) suggest any field/eval function in this position as first argument

const allSuggestions = [
// With extra space after field name to open suggestions
...getFieldNamesByType('any').map((field) => `${field} `),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }, undefined, ['case']),
];
await assertSuggestions('from a | eval case(/)', allSuggestions, {
triggerCharacter: ' ',
});
await assertSuggestions('from a | eval case(/)', allSuggestions);

// case( field /) suggest comparison operators at this point to converge to a boolean
await assertSuggestions('from a | eval case( textField /)', comparisonOperators, {
triggerCharacter: ' ',
});
await assertSuggestions('from a | eval case( doubleField /)', comparisonOperators, {
triggerCharacter: ' ',
});
await assertSuggestions('from a | eval case( booleanField /)', comparisonOperators, {
triggerCharacter: ' ',
});

// case( field > /) suggest field/function of the same type of the right hand side to complete the boolean expression
await assertSuggestions(
'from a | eval case( keywordField != /)',
[
// Notice no extra space after field name
...getFieldNamesByType(['keyword', 'text', 'boolean']).map((field) => `${field}`),
...getFunctionSignaturesByReturnType(
'eval',
['keyword', 'text', 'boolean'],
{ scalar: true },
undefined,
[]
),
],
{
triggerCharacter: ' ',
}
);

const expectedNumericSuggestions = [
// Notice no extra space after field name
...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES).map((field) => `${field}`),
...getFunctionSignaturesByReturnType(
'eval',
ESQL_COMMON_NUMERIC_TYPES,
{ scalar: true },
undefined,
[]
),
];
await assertSuggestions(
'from a | eval case( integerField != /)',
expectedNumericSuggestions,
{
triggerCharacter: ' ',
}
);
await assertSuggestions('from a | eval case( integerField != /)', [
// Notice no extra space after field name
...getFieldNamesByType('any').map((field) => `${field}`),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }, undefined, []),
'var0 = ',
]);

// case( field > 0, >) suggests fields like normal
await assertSuggestions(
'from a | eval case( integerField != doubleField, /)',
[
// With extra space after field name to open suggestions
...getFieldNamesByType('any').map((field) => `${field}`),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }, undefined, [
'case',
]),
],
{
triggerCharacter: ' ',
}
);

// case( multiple conditions ) suggests fields like normal
await assertSuggestions(
'from a | eval case(integerField < 0, "negative", integerField > 0, "positive", /)',
[
// With extra space after field name to open suggestions
...getFieldNamesByType('any').map((field) => `${field} `),
...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }, undefined, [
'case',
]),
],
{
triggerCharacter: ' ',
}
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type {
} from '@kbn/esql-ast';
import { i18n } from '@kbn/i18n';
import { ESQL_NUMBER_TYPES, isNumericType } from '../shared/esql_types';
import type { EditorContext, SuggestionRawDefinition } from './types';
import type { EditorContext, ItemKind, SuggestionRawDefinition } from './types';
import {
getColumnForASTNode,
getCommandDefinition,
Expand Down Expand Up @@ -111,6 +111,7 @@ import {
isParameterType,
isReturnType,
} from '../definitions/types';
import { comparisonFunctions } from '../definitions/builtin';

type GetSourceFn = () => Promise<SuggestionRawDefinition[]>;
type GetDataStreamsForIntegrationFn = (
Expand Down Expand Up @@ -1307,6 +1308,7 @@ async function getFunctionArgsSuggestions(
fieldsMap,
innerText
);

// pick the type of the next arg
const shouldGetNextArgument = node.text.includes(EDITOR_MARKER);
let argIndex = Math.max(node.args.length, 0);
Expand Down Expand Up @@ -1334,8 +1336,18 @@ async function getFunctionArgsSuggestions(
// Whether to prepend comma to suggestion string
// E.g. if true, "fieldName" -> "fieldName, "
const alreadyHasComma = fullText ? fullText[offset] === ',' : false;
const canBeBooleanCondition =
// For `CASE()`, there can be multiple conditions, so keep suggesting fields and functions if possible
fnDefinition.name === 'case' ||
// If the type is explicitly a boolean condition
typesToSuggestNext.some((t) => t && t.type === 'boolean' && t.name === 'condition');

const shouldAddComma =
hasMoreMandatoryArgs && fnDefinition.type !== 'builtin' && !alreadyHasComma;
hasMoreMandatoryArgs &&
fnDefinition.type !== 'builtin' &&
!alreadyHasComma &&
!canBeBooleanCondition;
const shouldAdvanceCursor = hasMoreMandatoryArgs && fnDefinition.type !== 'builtin';

const suggestedConstants = uniq(
typesToSuggestNext
Expand Down Expand Up @@ -1420,27 +1432,35 @@ async function getFunctionArgsSuggestions(
);

// Fields

suggestions.push(
...pushItUpInTheList(
await getFieldsByType(
// @TODO: have a way to better suggest constant only params
getTypesFromParamDefs(typesToSuggestNext.filter((d) => !d.constantOnly)) as string[],
// For example, in case() where we are expecting a boolean condition
// we can accept any field types (field1 !== field2)
canBeBooleanCondition
? ['any']
: // @TODO: have a way to better suggest constant only params
(getTypesFromParamDefs(
typesToSuggestNext.filter((d) => !d.constantOnly)
) as string[]),
[],
{
addComma: shouldAddComma,
advanceCursor: shouldAddComma,
openSuggestions: shouldAddComma,
advanceCursor: shouldAdvanceCursor,
openSuggestions: shouldAdvanceCursor,
}
),
true
)
);

// Functions
suggestions.push(
...getCompatibleFunctionDefinition(
command.name,
option?.name,
getTypesFromParamDefs(typesToSuggestNext) as string[],
canBeBooleanCondition ? ['any'] : (getTypesFromParamDefs(typesToSuggestNext) as string[]),
fnToIgnore
).map((suggestion) => ({
...suggestion,
Expand Down Expand Up @@ -1475,9 +1495,20 @@ async function getFunctionArgsSuggestions(
);
}
}

// Suggest comparison functions for boolean conditions
if (canBeBooleanCondition) {
suggestions.push(
...comparisonFunctions.map<SuggestionRawDefinition>(({ name, description }) => ({
label: name,
text: name + ' ',
kind: 'Function' as ItemKind,
detail: description,
command: TRIGGER_SUGGESTION_COMMAND,
}))
);
}
if (hasMoreMandatoryArgs) {
// suggest a comma if there's another argument for the function
// Suggest a comma if there's another argument for the function
suggestions.push(commaCompleteItem);
}
}
Expand Down Expand Up @@ -1663,6 +1694,7 @@ async function getOptionArgsSuggestions(
suggestions.push(...buildFieldsDefinitions(policyMetadata.enrichFields));
}
}

if (
assignFn &&
hasSameArgBothSides(assignFn) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export const mathFunctions: FunctionDefinition[] = [
),
];

const comparisonFunctions: FunctionDefinition[] = [
export const comparisonFunctions: FunctionDefinition[] = [
{
name: '==',
description: i18n.translate('kbn-esql-validation-autocomplete.esql.definition.equalToDoc', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import {
ALL_DATASETS_LOCATOR_ID,
} from '@kbn/deeplinks-observability/locators';
import { getLogsLocatorsFromUrlService } from '@kbn/logs-shared-plugin/common';
import {
ASSET_DETAILS_LOCATOR_ID,
type AssetDetailsLocatorParams,
} from '@kbn/observability-shared-plugin/common';
import { isJavaAgentName } from '../../../../../../common/agent_name';
import { SERVICE_NODE_NAME } from '../../../../../../common/es_fields/apm';
import { useApmPluginContext } from '../../../../../context/apm_plugin/use_apm_plugin_context';
Expand Down Expand Up @@ -55,6 +59,8 @@ export function InstanceActionsMenu({ serviceName, serviceNodeName, kuery, onClo
const allDatasetsLocator =
share.url.locators.get<AllDatasetsLocatorParams>(ALL_DATASETS_LOCATOR_ID)!;
const { nodeLogsLocator } = getLogsLocatorsFromUrlService(share.url);
const assetDetailsLocator =
share.url.locators.get<AssetDetailsLocatorParams>(ASSET_DETAILS_LOCATOR_ID);

if (isPending(status)) {
return (
Expand Down Expand Up @@ -95,6 +101,7 @@ export function InstanceActionsMenu({ serviceName, serviceNodeName, kuery, onClo
metricsHref,
allDatasetsLocator,
nodeLogsLocator,
assetDetailsLocator,
});

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { AllDatasetsLocatorParams } from '@kbn/deeplinks-observability/locators'
import type { LocatorPublic } from '@kbn/share-plugin/public';
import { NodeLogsLocatorParams } from '@kbn/logs-shared-plugin/common';
import { findInventoryFields } from '@kbn/metrics-data-access-plugin/common';
import { type AssetDetailsLocator } from '@kbn/observability-shared-plugin/common';
import { APIReturnType } from '../../../../../services/rest/create_call_apm_api';
import { getInfraHref } from '../../../../shared/links/infra_link';
import {
Action,
getNonEmptySections,
Expand All @@ -25,14 +25,14 @@ type InstaceDetails =

function getInfraMetricsQuery(timestamp?: string) {
if (!timestamp) {
return { from: 0, to: 0 };
return undefined;
}
const timeInMilliseconds = new Date(timestamp).getTime();
const fiveMinutes = moment.duration(5, 'minutes').asMilliseconds();

return {
from: timeInMilliseconds - fiveMinutes,
to: timeInMilliseconds + fiveMinutes,
from: new Date(timeInMilliseconds - fiveMinutes).toISOString(),
to: new Date(timeInMilliseconds + fiveMinutes).toISOString(),
};
}

Expand All @@ -43,13 +43,15 @@ export function getMenuSections({
metricsHref,
allDatasetsLocator,
nodeLogsLocator,
assetDetailsLocator,
}: {
instanceDetails: InstaceDetails;
basePath: IBasePath;
onFilterByInstanceClick: () => void;
metricsHref: string;
allDatasetsLocator: LocatorPublic<AllDatasetsLocatorParams>;
nodeLogsLocator: LocatorPublic<NodeLogsLocatorParams>;
assetDetailsLocator?: AssetDetailsLocator;
}) {
const podId = instanceDetails.kubernetes?.pod?.uid;
const containerId = instanceDetails.container?.id;
Expand All @@ -70,6 +72,9 @@ export function getMenuSections({
time,
});

const hasPodLink = !!podId && !!assetDetailsLocator;
const hasContainerLink = !!containerId && !!assetDetailsLocator;

const podActions: Action[] = [
{
key: 'podLogs',
Expand All @@ -84,13 +89,14 @@ export function getMenuSections({
label: i18n.translate('xpack.apm.serviceOverview.instancesTable.actionMenus.podMetrics', {
defaultMessage: 'Pod metrics',
}),
href: getInfraHref({
app: 'metrics',
basePath,
path: `/link-to/pod-detail/${podId}`,
query: infraMetricsQuery,
}),
condition: !!podId,
href: hasPodLink
? assetDetailsLocator.getRedirectUrl({
assetId: podId,
assetType: 'pod',
assetDetails: { dateRange: infraMetricsQuery },
})
: undefined,
condition: hasPodLink,
},
];

Expand All @@ -109,13 +115,14 @@ export function getMenuSections({
'xpack.apm.serviceOverview.instancesTable.actionMenus.containerMetrics',
{ defaultMessage: 'Container metrics' }
),
href: getInfraHref({
app: 'metrics',
basePath,
path: `/link-to/container-detail/${containerId}`,
query: infraMetricsQuery,
}),
condition: !!containerId,
href: hasContainerLink
? assetDetailsLocator.getRedirectUrl({
assetId: containerId,
assetType: 'container',
assetDetails: { dateRange: infraMetricsQuery },
})
: undefined,
condition: hasContainerLink,
},
];

Expand Down
Loading

0 comments on commit e6da1db

Please sign in to comment.