Skip to content

Commit

Permalink
Merge branch 'main' into unskip-detail-page-tests
Browse files Browse the repository at this point in the history
  • Loading branch information
elasticmachine authored Oct 17, 2024
2 parents d63a46d + abc8f68 commit 0fc73ef
Show file tree
Hide file tree
Showing 11 changed files with 179 additions and 20 deletions.
6 changes: 3 additions & 3 deletions examples/feature_flags_example/common/feature_flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

export const FeatureFlagExampleBoolean = 'example-boolean';
export const FeatureFlagExampleString = 'example-string';
export const FeatureFlagExampleNumber = 'example-number';
export const FeatureFlagExampleBoolean = 'featureFlagsExample.exampleBoolean';
export const FeatureFlagExampleString = 'featureFlagsExample.exampleString';
export const FeatureFlagExampleNumber = 'featureFlagsExample.exampleNumber';
16 changes: 11 additions & 5 deletions packages/core/feature-flags/README.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ id: kibFeatureFlagsService
slug: /kibana-dev-docs/tutorials/feature-flags-service
title: Feature Flags service
description: The Feature Flags service provides the necessary APIs to evaluate dynamic feature flags.
date: 2024-07-26
date: 2024-10-16
tags: ['kibana', 'dev', 'contributor', 'api docs', 'a/b testing', 'feature flags', 'flags']
---

Expand All @@ -12,7 +12,13 @@ tags: ['kibana', 'dev', 'contributor', 'api docs', 'a/b testing', 'feature flags
The Feature Flags service provides the necessary APIs to evaluate dynamic feature flags.

The service is always enabled, however, it will return the fallback value if a feature flags provider hasn't been attached.
Kibana only registers a provider when running on Elastic Cloud Hosted/Serverless.
Kibana only registers a provider when running on Elastic Cloud Hosted/Serverless. And even in those scenarios, we expect that some customers might
have network restrictions that might not allow the flags to evaluate. The fallback value must provide a non-broken experience to users.

:warning: Feature Flags are considered dynamic configuration and cannot be used for settings that require restarting Kibana.
One example of invalid use cases are settings used during the `setup` lifecycle of the plugin, such as settings that define
if an HTTP route is registered or not. Instead, you should always register the route, and return `404 - Not found` in the route
handler if the feature flag returns a _disabled_ state.

For a code example, refer to the [Feature Flags Example plugin](../../../examples/feature_flags_example)

Expand All @@ -32,7 +38,7 @@ import type { PluginInitializerContext } from '@kbn/core-plugins-server';

export const featureFlags: FeatureFlagDefinitions = [
{
key: 'my-cool-feature',
key: 'myPlugin.myCoolFeature',
name: 'My cool feature',
description: 'Enables the cool feature to auto-hide the navigation bar',
tags: ['my-plugin', 'my-service', 'ui'],
Expand Down Expand Up @@ -118,7 +124,7 @@ async (context, request, response) => {
const { featureFlags } = await context.core;
return response.ok({
body: {
number: await featureFlags.getNumberValue('example-number', 1),
number: await featureFlags.getNumberValue('myPlugin.exampleNumber', 1),
},
});
}
Expand All @@ -142,7 +148,7 @@ provider. In the `kibana.yml`, the following config sets the overrides:

```yaml
feature_flags.overrides:
my-feature-flag: 'my-forced-value'
myPlugin.myFeatureFlag: 'my-forced-value'
```
> [!WARNING]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,11 @@ describe('FeatureFlagsService Browser', () => {
beforeEach(async () => {
addHandlerSpy = jest.spyOn(featureFlagsClient, 'addHandler');
injectedMetadata.getFeatureFlags.mockReturnValue({
overrides: { 'my-overridden-flag': true },
overrides: {
'my-overridden-flag': true,
'myPlugin.myOverriddenFlag': true,
myDestructuredObjPlugin: { myOverriddenFlag: true },
},
});
featureFlagsService.setup({ injectedMetadata });
startContract = await featureFlagsService.start();
Expand Down Expand Up @@ -288,5 +292,14 @@ describe('FeatureFlagsService Browser', () => {
expect(getBooleanValueSpy).toHaveBeenCalledTimes(1);
expect(getBooleanValueSpy).toHaveBeenCalledWith('another-flag', false);
});

test('overrides with dotted names', async () => {
const getBooleanValueSpy = jest.spyOn(featureFlagsClient, 'getBooleanValue');
expect(startContract.getBooleanValue('myPlugin.myOverriddenFlag', false)).toEqual(true);
expect(
startContract.getBooleanValue('myDestructuredObjPlugin.myOverriddenFlag', false)
).toEqual(true);
expect(getBooleanValueSpy).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { apm } from '@elastic/apm-rum';
import { type Client, ClientProviderEvents, OpenFeature } from '@openfeature/web-sdk';
import deepMerge from 'deepmerge';
import { filter, map, startWith, Subject } from 'rxjs';
import { get } from 'lodash';

/**
* setup method dependencies
Expand Down Expand Up @@ -172,9 +173,10 @@ export class FeatureFlagsService {
flagName: string,
fallbackValue: T
): T {
const override = get(this.overrides, flagName); // using lodash get because flagName can come with dots and the config parser might structure it in objects.
const value =
typeof this.overrides[flagName] !== 'undefined'
? (this.overrides[flagName] as T)
typeof override !== 'undefined'
? (override as T)
: // We have to bind the evaluation or the client will lose its internal context
evaluationFn.bind(this.featureFlagsClient)(flagName, fallbackValue);
apm.addLabels({ [`flag_${flagName}`]: value });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ describe('FeatureFlagsService Server', () => {
atPath: {
overrides: {
'my-overridden-flag': true,
'myPlugin.myOverriddenFlag': true,
myDestructuredObjPlugin: { myOverriddenFlag: true },
},
},
}),
Expand Down Expand Up @@ -251,10 +253,25 @@ describe('FeatureFlagsService Server', () => {
expect(getBooleanValueSpy).toHaveBeenCalledTimes(1);
expect(getBooleanValueSpy).toHaveBeenCalledWith('another-flag', false);
});

test('overrides with dotted names', async () => {
const getBooleanValueSpy = jest.spyOn(featureFlagsClient, 'getBooleanValue');
await expect(
startContract.getBooleanValue('myPlugin.myOverriddenFlag', false)
).resolves.toEqual(true);
await expect(
startContract.getBooleanValue('myDestructuredObjPlugin.myOverriddenFlag', false)
).resolves.toEqual(true);
expect(getBooleanValueSpy).not.toHaveBeenCalled();
});
});

test('returns overrides', () => {
const { getOverrides } = featureFlagsService.setup();
expect(getOverrides()).toStrictEqual({ 'my-overridden-flag': true });
expect(getOverrides()).toStrictEqual({
'my-overridden-flag': true,
'myPlugin.myOverriddenFlag': true,
myDestructuredObjPlugin: { myOverriddenFlag: true },
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from '@openfeature/server-sdk';
import deepMerge from 'deepmerge';
import { filter, switchMap, startWith, Subject } from 'rxjs';
import { get } from 'lodash';
import { type FeatureFlagsConfig, featureFlagsConfig } from './feature_flags_config';

/**
Expand Down Expand Up @@ -165,9 +166,10 @@ export class FeatureFlagsService {
flagName: string,
fallbackValue: T
): Promise<T> {
const override = get(this.overrides, flagName); // using lodash get because flagName can come with dots and the config parser might structure it in objects.
const value =
typeof this.overrides[flagName] !== 'undefined'
? (this.overrides[flagName] as T)
typeof override !== 'undefined'
? (override as T)
: // We have to bind the evaluation or the client will lose its internal context
await evaluationFn.bind(this.featureFlagsClient)(flagName, fallbackValue);
apm.addLabels({ [`flag_${flagName}`]: value });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ export interface UrlDrilldownCollectConfigProps {
variablesHelpDocsLink?: string;
}

const isCursorBetweenDoubleCurlyBrackets = (editor: monaco.editor.IStandaloneCodeEditor) => {
const model = editor.getModel();
const position = editor.getPosition();
if (!model || !position) return false;

const offset = model.getOffsetAt(position);
const text = model.getValue();
const twoCharactersBeforeOffset = text.slice(offset - 2, offset);
const twoCharactersAfterOffset = text.slice(offset, offset + 2);

return twoCharactersBeforeOffset === '{{' && twoCharactersAfterOffset === '}}';
};

export const UrlDrilldownCollectConfig: React.FC<UrlDrilldownCollectConfigProps> = ({
config,
variables,
Expand Down Expand Up @@ -64,9 +77,10 @@ export const UrlDrilldownCollectConfig: React.FC<UrlDrilldownCollectConfigProps>
onSelect={(variable: string) => {
const editor = editorRef.current;
if (!editor) return;
const text = isCursorBetweenDoubleCurlyBrackets(editor) ? variable : `{{${variable}}}`;

editor.trigger('keyboard', 'type', {
text: '{{' + variable + '}}',
text,
});
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
*/

import * as React from 'react';
import { Suspense } from 'react';
import { shallow } from 'enzyme';
import { waitFor } from '@testing-library/react';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { mountWithIntl, nextTick } from '@kbn/test-jest-helpers';
import { act } from 'react-dom/test-utils';
Expand All @@ -21,6 +23,10 @@ import { ruleTypeRegistryMock } from '../../../rule_type_registry.mock';
import { useKibana } from '../../../../common/lib/kibana';
import { useBulkGetMaintenanceWindows } from '../../alerts_table/hooks/use_bulk_get_maintenance_windows';
import { getMaintenanceWindowMockMap } from '../../alerts_table/maintenance_windows/index.mock';
import { loadRuleTypes } from '../../../lib/rule_api/rule_types';

jest.mock('../../../lib/rule_api/rule_types');
jest.mocked(loadRuleTypes).mockResolvedValue([]);

const mockUseKibanaReturnValue = createStartServicesMock();
jest.mock('../../../../common/lib/kibana', () => ({
Expand All @@ -37,6 +43,15 @@ jest.mock('../../../lib/rule_api/load_execution_log_aggregations', () => ({
loadExecutionLogAggregations: jest.fn(),
}));

const mockAlertsTable = jest.fn(() => {
return <div data-test-subj="alertsTable" />;
});
jest.mock('../../alerts_table/alerts_table_state', () => ({
__esModule: true,
AlertsTableState: mockAlertsTable,
default: mockAlertsTable,
}));

const { loadExecutionLogAggregations } = jest.requireMock(
'../../../lib/rule_api/load_execution_log_aggregations'
);
Expand All @@ -60,7 +75,6 @@ const useBulkGetMaintenanceWindowsMock = useBulkGetMaintenanceWindows as jest.Mo
const ruleTypeRegistry = ruleTypeRegistryMock.create();

import { getIsExperimentalFeatureEnabled } from '../../../../common/get_experimental_features';
import { waitFor } from '@testing-library/react';
import { createStartServicesMock } from '../../../../common/lib/kibana/kibana_react.mock';

const fakeNow = new Date('2020-02-09T23:15:41.941Z');
Expand Down Expand Up @@ -118,9 +132,11 @@ const queryClient = new QueryClient({

const RuleComponentWithProvider = (props: RuleComponentProps) => {
return (
<QueryClientProvider client={queryClient}>
<RuleComponent {...props} />
</QueryClientProvider>
<Suspense fallback={null}>
<QueryClientProvider client={queryClient}>
<RuleComponent {...props} />
</QueryClientProvider>
</Suspense>
);
};

Expand Down Expand Up @@ -277,6 +293,48 @@ describe('rules', () => {
alertToListItem(fakeNow.getTime(), 'us-east', ruleUsEast),
]);
});

it('requests a table refresh when the refresh token changes', async () => {
jest.useFakeTimers();
const rule = mockRule({
enabled: false,
});
const ruleType = mockRuleType({
hasFieldsForAAD: true,
});
const ruleSummary = mockRuleSummary();
jest.setSystemTime(fake2MinutesAgo);

const wrapper = mountWithIntl(
<RuleComponentWithProvider
{...mockAPIs}
rule={rule}
ruleType={ruleType}
ruleSummary={ruleSummary}
readOnly={false}
/>
);

await waitFor(() => wrapper.find('[data-test-subj="alertsTable"]'));

jest.setSystemTime(fakeNow);

wrapper.setProps({
refreshToken: {
resolve: () => undefined,
reject: () => undefined,
},
});

expect(mockAlertsTable).toHaveBeenCalledWith(
expect.objectContaining({
lastReloadRequestTime: fakeNow.getTime(),
}),
expect.anything()
);

jest.useRealTimers();
});
});

describe('alertToListItem', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { lazy, useCallback } from 'react';
import React, { lazy, useCallback, useMemo } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiSpacer, EuiFlexGroup, EuiFlexItem, EuiTabbedContent } from '@elastic/eui';
import { AlertStatusValues, ALERTING_FEATURE_ID } from '@kbn/alerting-plugin/common';
Expand Down Expand Up @@ -71,6 +71,9 @@ export function RuleComponent({
}: RuleComponentProps) {
const { ruleTypeRegistry, actionTypeRegistry, alertsTableConfigurationRegistry } =
useKibana().services;
// The lastReloadRequestTime should be updated when the refreshToken changes
// eslint-disable-next-line react-hooks/exhaustive-deps
const lastReloadRequestTime = useMemo(() => new Date().getTime(), [refreshToken]);

const alerts = Object.entries(ruleSummary.alerts)
.map(([alertId, alert]) => alertToListItem(durationEpoch, alertId, alert))
Expand Down Expand Up @@ -110,6 +113,7 @@ export function RuleComponent({
}
query={{ bool: { filter: { term: { [ALERT_RULE_UUID]: rule.id } } } }}
showAlertStatusWithFlapping
lastReloadRequestTime={lastReloadRequestTime}
/>
);
}
Expand All @@ -124,6 +128,7 @@ export function RuleComponent({
}, [
alerts,
alertsTableConfigurationRegistry,
lastReloadRequestTime,
onMuteAction,
readOnly,
rule.consumer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ import {
riskEngineRouteHelpersFactory,
waitForRiskScoresToBePresent,
createAndSyncRuleAndAlertsFactory,
waitForRiskEngineTaskToBeGone,
waitForSavedObjectToBeGone,
waitForRiskScoresToBeGone,
} from '../../utils';
import { dataGeneratorFactory } from '../../../detections_response/utils';

Expand All @@ -22,6 +25,7 @@ export default ({ getService }: FtrProviderContext) => {
const es = getService('es');
const log = getService('log');
const esArchiver = getService('esArchiver');
const kibanaServer = getService('kibanaServer');

describe('@ess @ serverless @serverless QA risk_engine_cleanup_api', () => {
const createAndSyncRuleAndAlerts = createAndSyncRuleAndAlertsFactory({ supertest, log });
Expand Down Expand Up @@ -59,6 +63,10 @@ export default ({ getService }: FtrProviderContext) => {
cleanup_successful: true,
});

await waitForRiskEngineTaskToBeGone({ es, log });
await waitForSavedObjectToBeGone({ log, kibanaServer });
await waitForRiskScoresToBeGone({ es, log });

const status3 = await riskEngineRoutes.getStatus();
expect(status3.body.risk_engine_status).to.be('NOT_INSTALLED');
expect(status3.body.legacy_risk_engine_status).to.be('NOT_INSTALLED');
Expand Down
Loading

0 comments on commit 0fc73ef

Please sign in to comment.