Skip to content

Commit

Permalink
feat(slo): delete associated rules when deleting an SLO (elastic#156307)
Browse files Browse the repository at this point in the history
(cherry picked from commit dbedd53)
  • Loading branch information
kdelemme committed May 2, 2023
1 parent 595cc3f commit 0f8e3a3
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 15 deletions.
25 changes: 16 additions & 9 deletions x-pack/plugins/observability/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
Logger,
} from '@kbn/core/server';
import { hiddenTypes as filesSavedObjectTypes } from '@kbn/files-plugin/server/saved_objects';
import { PluginSetupContract } from '@kbn/alerting-plugin/server';
import { PluginSetupContract, PluginStartContract } from '@kbn/alerting-plugin/server';
import { Dataset, RuleRegistryPluginSetupContract } from '@kbn/rule-registry-plugin/server';
import { PluginSetupContract as FeaturesSetup } from '@kbn/features-plugin/server';
import { legacyExperimentalFieldMap } from '@kbn/alerts-as-data-utils';
Expand Down Expand Up @@ -59,6 +59,10 @@ interface PluginSetup {
usageCollection?: UsageCollectionSetup;
}

interface PluginStart {
alerting: PluginStartContract;
}

export class ObservabilityPlugin implements Plugin<ObservabilityPluginSetup> {
private logger: Logger;

Expand All @@ -67,7 +71,7 @@ export class ObservabilityPlugin implements Plugin<ObservabilityPluginSetup> {
this.logger = initContext.logger.get();
}

public setup(core: CoreSetup, plugins: PluginSetup) {
public setup(core: CoreSetup<PluginStart>, plugins: PluginSetup) {
const casesCapabilities = createCasesUICapabilities();
const casesApiTags = getCasesApiTags(observabilityFeatureId);

Expand Down Expand Up @@ -237,13 +241,16 @@ export class ObservabilityPlugin implements Plugin<ObservabilityPluginSetup> {
registerRuleTypes(plugins.alerting, this.logger, ruleDataClient, core.http.basePath);
registerSloUsageCollector(plugins.usageCollection);

registerRoutes({
core,
dependencies: {
ruleDataService,
},
logger: this.logger,
repository: getObservabilityServerRouteRepository(),
core.getStartServices().then(([coreStart, pluginStart]) => {
registerRoutes({
core,
dependencies: {
ruleDataService,
getRulesClientWithRequest: pluginStart.alerting.getRulesClientWithRequest,
},
logger: this.logger,
repository: getObservabilityServerRouteRepository(),
});
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import {
parseEndpoint,
routeValidationObject,
} from '@kbn/server-route-repository';
import { CoreSetup, Logger, RouteRegistrar } from '@kbn/core/server';
import { CoreSetup, KibanaRequest, Logger, RouteRegistrar } from '@kbn/core/server';
import Boom from '@hapi/boom';
import { errors } from '@elastic/elasticsearch';
import { RuleDataPluginService } from '@kbn/rule-registry-plugin/server';
import { RulesClientApi } from '@kbn/alerting-plugin/server/types';

import { ObservabilityRequestHandlerContext } from '../types';
import { AbstractObservabilityServerRouteRepository } from './types';
Expand All @@ -28,6 +29,7 @@ interface RegisterRoutes {

export interface RegisterRoutesDependencies {
ruleDataService: RuleDataPluginService;
getRulesClientWithRequest: (request: KibanaRequest) => RulesClientApi;
}

export function registerRoutes({ repository, core, logger, dependencies }: RegisterRoutes) {
Expand Down
11 changes: 9 additions & 2 deletions x-pack/plugins/observability/server/routes/slo/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,25 @@ const deleteSLORoute = createObservabilityServerRoute({
tags: ['access:slo_write'],
},
params: deleteSLOParamsSchema,
handler: async ({ context, params, logger }) => {
handler: async ({
request,
context,
params,
logger,
dependencies: { getRulesClientWithRequest },
}) => {
if (!isLicenseAtLeastPlatinum(context)) {
throw badRequest('Platinum license or higher is needed to make use of this feature.');
}

const esClient = (await context.core).elasticsearch.client.asCurrentUser;
const soClient = (await context.core).savedObjects.client;
const rulesClient = getRulesClientWithRequest(request);

const repository = new KibanaSavedObjectsSLORepository(soClient);
const transformManager = new DefaultTransformManager(transformGenerators, esClient, logger);

const deleteSLO = new DeleteSLO(repository, transformManager, esClient);
const deleteSLO = new DeleteSLO(repository, transformManager, esClient, rulesClient);

await deleteSLO.execute(params.path.id);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* 2.0.
*/

import { RulesClientApi } from '@kbn/alerting-plugin/server/types';
import { rulesClientMock } from '@kbn/alerting-plugin/server/rules_client.mock';
import { ElasticsearchClient } from '@kbn/core/server';
import { elasticsearchServiceMock } from '@kbn/core/server/mocks';
import { getSLOTransformId, SLO_INDEX_TEMPLATE_NAME } from '../../assets/constants';
Expand All @@ -18,17 +20,19 @@ describe('DeleteSLO', () => {
let mockRepository: jest.Mocked<SLORepository>;
let mockTransformManager: jest.Mocked<TransformManager>;
let mockEsClient: jest.Mocked<ElasticsearchClient>;
let mockRulesClient: jest.Mocked<RulesClientApi>;
let deleteSLO: DeleteSLO;

beforeEach(() => {
mockRepository = createSLORepositoryMock();
mockTransformManager = createTransformManagerMock();
mockEsClient = elasticsearchServiceMock.createElasticsearchClient();
deleteSLO = new DeleteSLO(mockRepository, mockTransformManager, mockEsClient);
mockRulesClient = rulesClientMock.create();
deleteSLO = new DeleteSLO(mockRepository, mockTransformManager, mockEsClient, mockRulesClient);
});

describe('happy path', () => {
it('removes the transform, the roll up data and the SLO from the repository', async () => {
it('removes the transform, the roll up data, the associated rules and the SLO from the repository', async () => {
const slo = createSLO({ indicator: createAPMTransactionErrorRateIndicator() });
mockRepository.findById.mockResolvedValueOnce(slo);

Expand All @@ -51,6 +55,9 @@ describe('DeleteSLO', () => {
},
})
);
expect(mockRulesClient.bulkDeleteRules).toHaveBeenCalledWith({
filter: `alert.attributes.params.sloId:${slo.id}`,
});
expect(mockRepository.deleteById).toHaveBeenCalledWith(slo.id);
});
});
Expand Down
15 changes: 14 additions & 1 deletion x-pack/plugins/observability/server/services/slo/delete_slo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { RulesClientApi } from '@kbn/alerting-plugin/server/types';
import { ElasticsearchClient } from '@kbn/core/server';
import { getSLOTransformId, SLO_INDEX_TEMPLATE_NAME } from '../../assets/constants';

Expand All @@ -15,7 +16,8 @@ export class DeleteSLO {
constructor(
private repository: SLORepository,
private transformManager: TransformManager,
private esClient: ElasticsearchClient
private esClient: ElasticsearchClient,
private rulesClient: RulesClientApi
) {}

public async execute(sloId: string): Promise<void> {
Expand All @@ -26,6 +28,7 @@ export class DeleteSLO {
await this.transformManager.uninstall(sloTransformId);

await this.deleteRollupData(slo.id);
await this.deleteAssociatedRules(slo.id);
await this.repository.deleteById(slo.id);
}

Expand All @@ -40,4 +43,14 @@ export class DeleteSLO {
},
});
}

private async deleteAssociatedRules(sloId: string): Promise<void> {
try {
await this.rulesClient.bulkDeleteRules({
filter: `alert.attributes.params.sloId:${sloId}`,
});
} catch (err) {
// no-op: bulkDeleteRules throws if no rules are found.
}
}
}

0 comments on commit 0f8e3a3

Please sign in to comment.