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

[data / data views] Disable rollup functionality when rollup plugin is disabled #162674

Merged
merged 35 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
cb358ca
partial progress
mattkime Jul 28, 2023
e15fc4d
undo some unneeded changes
mattkime Jul 28, 2023
0e4bc04
Update ese_search_strategy.ts
mattkime Jul 28, 2023
48ba21c
rollup plugin enables functionality in data views
mattkime Jul 28, 2023
f8d537a
Merge branch 'disable_rollups_on_serverless' of github.com:mattkime/k…
mattkime Jul 28, 2023
f72101f
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Jul 28, 2023
83c3afd
address data plugin
mattkime Jul 28, 2023
f4a880c
Merge branch 'disable_rollups_on_serverless' of github.com:mattkime/k…
mattkime Jul 28, 2023
5741d45
fix data plugin
mattkime Jul 28, 2023
8efde82
add search strategy tests
mattkime Jul 28, 2023
7c46b6b
add debugging info
mattkime Jul 28, 2023
b57cd65
fix passing of rollupsEnabled
mattkime Jul 29, 2023
946c2be
remove unused var
mattkime Jul 29, 2023
27d261c
Merge branch 'main' into disable_rollups_on_serverless
mattkime Jul 30, 2023
0aa7e2f
Merge branch 'main' of github.com:elastic/kibana into disable_rollups…
mattkime Jul 30, 2023
c638dea
better mock
mattkime Jul 30, 2023
bf869ff
Merge branch 'disable_rollups_on_serverless' of github.com:mattkime/k…
mattkime Jul 30, 2023
3af845f
maybe this will work better
mattkime Jul 31, 2023
d5dd11a
fix registering routes
mattkime Jul 31, 2023
3480ccb
fix registering routes
mattkime Jul 31, 2023
8bd2709
Merge branch 'main' into disable_rollups_on_serverless
mattkime Aug 2, 2023
55488dd
fix jest test
mattkime Aug 2, 2023
f5fe89b
Merge branch 'main' into disable_rollups_on_serverless
mattkime Aug 3, 2023
dc1f75a
remove unused interfaces
mattkime Aug 3, 2023
257e03c
Merge branch 'disable_rollups_on_serverless' of github.com:mattkime/k…
mattkime Aug 3, 2023
a2f10fb
interface cleanup / simplification
mattkime Aug 3, 2023
fad16e6
add functional test
mattkime Aug 7, 2023
485a7fc
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Aug 7, 2023
91832c5
another attempt at including new test
mattkime Aug 8, 2023
836057a
Merge branch 'disable_rollups_on_serverless' of github.com:mattkime/k…
mattkime Aug 8, 2023
e0e5ad2
Merge branch 'main' into disable_rollups_on_serverless
mattkime Aug 8, 2023
a3108cc
another attempt at including new test
mattkime Aug 8, 2023
96eb121
fix functional test
mattkime Aug 8, 2023
299dcb4
cleanup
mattkime Aug 8, 2023
8e67228
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Aug 8, 2023
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
1 change: 0 additions & 1 deletion src/plugins/data/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
],
"optionalPlugins": [
"usageCollection",
"taskManager",
"security"
],
"requiredBundles": [
Expand Down
22 changes: 2 additions & 20 deletions src/plugins/data/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ import { BfetchServerSetup } from '@kbn/bfetch-plugin/server';
import { PluginStart as DataViewsServerPluginStart } from '@kbn/data-views-plugin/server';
import { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server';
import { FieldFormatsSetup, FieldFormatsStart } from '@kbn/field-formats-plugin/server';
import type {
TaskManagerSetupContract,
TaskManagerStartContract,
} from '@kbn/task-manager-plugin/server';
import type { SecurityPluginSetup } from '@kbn/security-plugin/server';
import { ConfigSchema } from '../config';
import type { ISearchSetup, ISearchStart } from './search';
Expand Down Expand Up @@ -55,15 +51,13 @@ export interface DataPluginSetupDependencies {
expressions: ExpressionsServerSetup;
usageCollection?: UsageCollectionSetup;
fieldFormats: FieldFormatsSetup;
taskManager?: TaskManagerSetupContract;
security?: SecurityPluginSetup;
}

export interface DataPluginStartDependencies {
fieldFormats: FieldFormatsStart;
logger: Logger;
dataViews: DataViewsServerPluginStart;
taskManager?: TaskManagerStartContract;
}

export class DataServerPlugin
Expand All @@ -90,14 +84,7 @@ export class DataServerPlugin

public setup(
core: CoreSetup<DataPluginStartDependencies, DataPluginStart>,
{
bfetch,
expressions,
usageCollection,
fieldFormats,
taskManager,
security,
}: DataPluginSetupDependencies
{ bfetch, expressions, usageCollection, fieldFormats, security }: DataPluginSetupDependencies
) {
this.scriptsService.setup(core);
const querySetup = this.queryService.setup(core);
Expand All @@ -110,7 +97,6 @@ export class DataServerPlugin
expressions,
usageCollection,
security,
taskManager,
});

return {
Expand All @@ -120,14 +106,10 @@ export class DataServerPlugin
};
}

public start(
core: CoreStart,
{ fieldFormats, dataViews, taskManager }: DataPluginStartDependencies
) {
public start(core: CoreStart, { fieldFormats, dataViews }: DataPluginStartDependencies) {
const search = this.searchService.start(core, {
fieldFormats,
indexPatterns: dataViews,
taskManager,
});
const datatableUtilities = new DatatableUtilitiesService(
search.aggs,
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/server/search/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export function createSearchSetupMock(): jest.Mocked<ISearchSetup> {
aggs: searchAggsSetupMock(),
registerSearchStrategy: jest.fn(),
searchSource: searchSourceMock.createSetupContract(),
enableRollups: jest.fn(),
};
}

Expand Down
17 changes: 7 additions & 10 deletions src/plugins/data/server/search/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ import { ExpressionsServerSetup } from '@kbn/expressions-plugin/server';
import { FieldFormatsStart } from '@kbn/field-formats-plugin/server';
import { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server';
import { KbnServerError } from '@kbn/kibana-utils-plugin/server';
import type {
TaskManagerSetupContract,
TaskManagerStartContract,
} from '@kbn/task-manager-plugin/server';
import type { SecurityPluginSetup } from '@kbn/security-plugin/server';
import type { DataViewsServerPluginStart } from '@kbn/data-views-plugin/server';
import type {
Expand Down Expand Up @@ -107,15 +103,13 @@ export interface SearchServiceSetupDependencies {
bfetch: BfetchServerSetup;
expressions: ExpressionsServerSetup;
usageCollection?: UsageCollectionSetup;
taskManager?: TaskManagerSetupContract;
security?: SecurityPluginSetup;
}

/** @internal */
export interface SearchServiceStartDependencies {
fieldFormats: FieldFormatsStart;
indexPatterns: DataViewsServerPluginStart;
taskManager?: TaskManagerStartContract;
}

/** @internal */
Expand All @@ -131,6 +125,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
private sessionService: SearchSessionService;
private asScoped!: ISearchStart['asScoped'];
private searchAsInternalUser!: ISearchStrategy;
private rollupsEnabled: boolean = false;

constructor(
private initializerContext: PluginInitializerContext<ConfigSchema>,
Expand All @@ -145,7 +140,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {

public setup(
core: CoreSetup<DataPluginStartDependencies, DataPluginStart>,
{ bfetch, expressions, usageCollection, taskManager, security }: SearchServiceSetupDependencies
mattkime marked this conversation as resolved.
Show resolved Hide resolved
{ bfetch, expressions, usageCollection, security }: SearchServiceSetupDependencies
): ISearchSetup {
core.savedObjects.registerType(searchSessionSavedObjectType);
const usage = usageCollection ? usageProvider(core) : undefined;
Expand Down Expand Up @@ -261,12 +256,13 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
registerSearchStrategy: this.registerSearchStrategy,
usage,
searchSource: this.searchSourceService.setup(),
enableRollups: () => (this.rollupsEnabled = true),
};
}

public start(
core: CoreStart,
{ fieldFormats, indexPatterns, taskManager }: SearchServiceStartDependencies
{ fieldFormats, indexPatterns }: SearchServiceStartDependencies
): ISearchStart {
const { elasticsearch, savedObjects, uiSettings } = core;

Expand All @@ -278,7 +274,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
indexPatterns,
});

this.asScoped = this.asScopedProvider(core);
this.asScoped = this.asScopedProvider(core, this.rollupsEnabled);
return {
aggs,
searchAsInternalUser: this.searchAsInternalUser,
Expand Down Expand Up @@ -516,7 +512,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
return deps.searchSessionsClient.extend(sessionId, expires);
};

private asScopedProvider = (core: CoreStart) => {
private asScopedProvider = (core: CoreStart, rollupsEnabled: boolean = false) => {
const { elasticsearch, savedObjects, uiSettings } = core;
const getSessionAsScoped = this.sessionService.asScopedProvider(core);
return (request: KibanaRequest): IScopedSearchClient => {
Expand All @@ -530,6 +526,7 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
uiSettings.asScopedToClient(savedObjectsClient)
),
request,
rollupsEnabled,
};
return {
search: <
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ describe('ES search strategy', () => {
},
},
searchSessionsClient: createSearchSessionsClientMock(),
rollupsEnabled: true,
} as unknown as SearchStrategyDependencies;
const mockLegacyConfig$ = new BehaviorSubject<any>({
elasticsearch: {
Expand Down Expand Up @@ -233,6 +234,31 @@ describe('ES search strategy', () => {
expect(method).toBe('POST');
expect(path).toBe('/foo-%E7%A8%8B/_rollup_search');
});

it("doesn't call the rollup API if the index is a rollup type BUT rollups are disabled", async () => {
mockApiCaller.mockResolvedValueOnce(mockRollupResponse);
mockSubmitCaller.mockResolvedValueOnce(mockAsyncResponse);

const params = { index: 'foo-程', body: { query: {} } };
Copy link
Member

Choose a reason for hiding this comment

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

out of curiosity, 程 means journey? right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to, it was duplicated from the previous test.

const esSearch = await enhancedEsSearchStrategyProvider(
mockLegacyConfig$,
mockSearchConfig,
mockLogger
);

await esSearch
.search(
{
indexType: 'rollup',
params,
},
{},
{ ...mockDeps, rollupsEnabled: false }
)
.toPromise();

expect(mockApiCaller).toBeCalledTimes(0);
});
});

describe('with sessionId', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export const enhancedEsSearchStrategyProvider = (
throw new KbnServerError('Unknown indexType', 400);
}

if (request.indexType === undefined) {
if (request.indexType === undefined || !deps.rollupsEnabled) {
return asyncSearch(request, options, deps);
} else {
return from(rollupSearch(request, options, deps));
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/data/server/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface SearchStrategyDependencies {
uiSettingsClient: Pick<IUiSettingsClient, 'get'>;
searchSessionsClient: IScopedSearchSessionsClient;
request: KibanaRequest;
rollupsEnabled?: boolean;
}

export interface ISearchSetup {
Expand All @@ -55,7 +56,7 @@ export interface ISearchSetup {
* Used internally for telemetry
*/
usage?: SearchUsage;

enableRollups: () => void;
searchSource: ReturnType<SearchSourceService['setup']>;
}

Expand Down
1 change: 0 additions & 1 deletion src/plugins/data/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
"@kbn/field-formats-plugin",
"@kbn/data-views-plugin",
"@kbn/screenshot-mode-plugin",
"@kbn/task-manager-plugin",
"@kbn/security-plugin",
"@kbn/expressions-plugin",
"@kbn/field-types",
Expand Down
9 changes: 7 additions & 2 deletions src/plugins/data_views/server/data_views_service_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ interface DataViewsServiceFactoryDeps {
uiSettings: UiSettingsServiceStart;
fieldFormats: FieldFormatsStart;
capabilities: CoreStart['capabilities'];
rollupsEnabled: boolean;
}

/**
Expand All @@ -38,14 +39,18 @@ export const dataViewsServiceFactory = (deps: DataViewsServiceFactoryDeps) =>
request?: KibanaRequest,
byPassCapabilities?: boolean
) {
const { logger, uiSettings, fieldFormats, capabilities } = deps;
const { logger, uiSettings, fieldFormats, capabilities, rollupsEnabled } = deps;
const uiSettingsClient = uiSettings.asScopedToClient(savedObjectsClient);
const formats = await fieldFormats.fieldFormatServiceFactory(uiSettingsClient);

return new DataViewsService({
uiSettings: new UiSettingsServerToCommon(uiSettingsClient),
savedObjectsClient: new SavedObjectsClientWrapper(savedObjectsClient),
apiClient: new IndexPatternsApiServer(elasticsearchClient, savedObjectsClient),
apiClient: new IndexPatternsApiServer(
elasticsearchClient,
savedObjectsClient,
rollupsEnabled
),
fieldFormats: formats,
onError: (error) => {
logger.error(error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@ import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { IndexPatternsFetcher } from '.';
import { elasticsearchServiceMock } from '@kbn/core/server/mocks';

const rollupResponse = {
foo: {
rollup_jobs: [
{
index_pattern: 'foo',
job_id: '123',
rollup_index: 'foo',
fields: [],
},
],
},
};

describe('Index Pattern Fetcher - server', () => {
let indexPatterns: IndexPatternsFetcher;
let esClient: ReturnType<typeof elasticsearchServiceMock.createElasticsearchClient>;
Expand All @@ -21,12 +34,40 @@ describe('Index Pattern Fetcher - server', () => {
beforeEach(() => {
jest.clearAllMocks();
esClient = elasticsearchServiceMock.createElasticsearchClient();
indexPatterns = new IndexPatternsFetcher(esClient);
indexPatterns = new IndexPatternsFetcher(esClient, false, true);
});
it('calls fieldcaps once', async () => {
esClient.fieldCaps.mockResponse(response as unknown as estypes.FieldCapsResponse);
indexPatterns = new IndexPatternsFetcher(esClient, true);
indexPatterns = new IndexPatternsFetcher(esClient, true, true);
await indexPatterns.getFieldsForWildcard({ pattern: patternList });
expect(esClient.fieldCaps).toHaveBeenCalledTimes(1);
});

it('calls rollup api when given rollup data view', async () => {
esClient.fieldCaps.mockResponse(response as unknown as estypes.FieldCapsResponse);
esClient.rollup.getRollupIndexCaps.mockResponse(
rollupResponse as unknown as estypes.RollupGetRollupIndexCapsResponse
);
indexPatterns = new IndexPatternsFetcher(esClient, true, true);
await indexPatterns.getFieldsForWildcard({
pattern: patternList,
type: 'rollup',
rollupIndex: 'foo',
});
expect(esClient.rollup.getRollupIndexCaps).toHaveBeenCalledTimes(1);
});

it("doesn't call rollup api when given rollup data view and rollups are disabled", async () => {
esClient.fieldCaps.mockResponse(response as unknown as estypes.FieldCapsResponse);
esClient.rollup.getRollupIndexCaps.mockResponse(
rollupResponse as unknown as estypes.RollupGetRollupIndexCapsResponse
);
indexPatterns = new IndexPatternsFetcher(esClient, true, false);
await indexPatterns.getFieldsForWildcard({
pattern: patternList,
type: 'rollup',
rollupIndex: 'foo',
});
expect(esClient.rollup.getRollupIndexCaps).toHaveBeenCalledTimes(0);
});
});
10 changes: 8 additions & 2 deletions src/plugins/data_views/server/fetcher/index_patterns_fetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,16 @@ interface FieldSubType {
export class IndexPatternsFetcher {
private elasticsearchClient: ElasticsearchClient;
private allowNoIndices: boolean;
private rollupsEnabled: boolean;

constructor(elasticsearchClient: ElasticsearchClient, allowNoIndices: boolean = false) {
constructor(
elasticsearchClient: ElasticsearchClient,
allowNoIndices: boolean = false,
rollupsEnabled: boolean = false
) {
this.elasticsearchClient = elasticsearchClient;
this.allowNoIndices = allowNoIndices;
this.rollupsEnabled = rollupsEnabled;
}

/**
Expand Down Expand Up @@ -81,7 +87,7 @@ export class IndexPatternsFetcher {
fields: options.fields || ['*'],
});

if (type === 'rollup' && rollupIndex) {
if (this.rollupsEnabled && type === 'rollup' && rollupIndex) {
const rollupFields: FieldDescriptor[] = [];
const capabilityCheck = getCapabilitiesForRollupIndices(
await this.elasticsearchClient.rollup.getRollupIndexCaps({
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data_views/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export { getFieldByName, findIndexPatternById } from './utils';
export type { FieldDescriptor, RollupIndexCapability } from './fetcher';
export { IndexPatternsFetcher, getCapabilitiesForRollupIndices } from './fetcher';
export type {
DataViewsServerPluginSetup,
DataViewsServerPluginStart,
DataViewsServerPluginSetupDependencies,
DataViewsServerPluginStartDependencies,
Expand Down
9 changes: 7 additions & 2 deletions src/plugins/data_views/server/index_patterns_api_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ export class IndexPatternsApiServer implements IDataViewsApiClient {
esClient: ElasticsearchClient;
constructor(
elasticsearchClient: ElasticsearchClient,
private readonly savedObjectsClient: SavedObjectsClientContract
private readonly savedObjectsClient: SavedObjectsClientContract,
private readonly rollupsEnabled: boolean
) {
this.esClient = elasticsearchClient;
}
Expand All @@ -29,7 +30,11 @@ export class IndexPatternsApiServer implements IDataViewsApiClient {
indexFilter,
fields,
}: GetFieldsOptions) {
const indexPatterns = new IndexPatternsFetcher(this.esClient, allowNoIndex);
const indexPatterns = new IndexPatternsFetcher(
this.esClient,
allowNoIndex,
this.rollupsEnabled
);
return await indexPatterns
.getFieldsForWildcard({
pattern,
Expand Down
Loading