-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Improve logging in API integration test and replace pRetry with a method from retryService #178515
Merged
maryam-saeidi
merged 11 commits into
elastic:main
from
maryam-saeidi:176401-rate-custom-threshold-flaky-test
Mar 19, 2024
Merged
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
f0ad217
Improve logging, use retryService instead of pRetry, fix flakyness in…
maryam-saeidi a03e1bc
Adjust timeout
maryam-saeidi 796fd58
Merge branch 'main' into 176401-rate-custom-threshold-flaky-test
maryam-saeidi aceaf5f
Generate data after now in test and wait for documents in metric thre…
maryam-saeidi 86ab613
Add refreshSavedObjectIndices and increase timeout
maryam-saeidi 38cff83
Merge branch 'main' into 176401-rate-custom-threshold-flaky-test
maryam-saeidi 1168ecc
Merge branch 'main' into 176401-rate-custom-threshold-flaky-test
maryam-saeidi 95711e0
Fix type
maryam-saeidi d948c58
Merge branch 'main' into 176401-rate-custom-threshold-flaky-test
maryam-saeidi 535dab2
Remove unnecessary JSON.stringify and adjust timeout
maryam-saeidi 40e2018
Merge branch 'main' into 176401-rate-custom-threshold-flaky-test
maryam-saeidi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { RetryService } from '@kbn/ftr-common-functional-services'; | ||
import type { ToolingLog } from '@kbn/tooling-log'; | ||
|
||
/** | ||
* Copied from x-pack/test/security_solution_api_integration/test_suites/detections_response/utils/retry.ts | ||
* | ||
* Retry wrapper for async supertests, with a maximum number of retries. | ||
* You can pass in a function that executes a supertest test, and make assertions | ||
* on the response. If the test fails, it will retry the test the number of retries | ||
* that are passed in. | ||
* | ||
* Example usage: | ||
* ```ts | ||
const fleetResponse = await retry<InstallPackageResponse>({ | ||
test: async () => { | ||
const testResponse = await supertest | ||
.post(`/api/fleet/epm/packages/security_detection_engine`) | ||
.set('kbn-xsrf', 'xxxx') | ||
.set('elastic-api-version', '2023-10-31') | ||
.type('application/json') | ||
.send({ force: true }) | ||
.expect(200); | ||
expect((testResponse.body as InstallPackageResponse).items).toBeDefined(); | ||
expect((testResponse.body as InstallPackageResponse).items.length).toBeGreaterThan(0); | ||
|
||
return testResponse.body; | ||
}, | ||
retryService, | ||
retries: MAX_RETRIES, | ||
timeout: ATTEMPT_TIMEOUT, | ||
}); | ||
* ``` | ||
* @param test The function containing a test to run | ||
* @param retryService The retry service to use | ||
* @param retries The maximum number of retries | ||
* @param timeout The timeout for each retry | ||
* @param retryDelay The delay between each retry | ||
* @returns The response from the test | ||
*/ | ||
export const retry = async <T>({ | ||
test, | ||
retryService, | ||
utilityName, | ||
retries = 3, | ||
timeout = 30000, | ||
retryDelay = 200, | ||
logger, | ||
}: { | ||
test: () => Promise<T>; | ||
utilityName: string; | ||
retryService: RetryService; | ||
retries?: number; | ||
timeout?: number; | ||
retryDelay?: number; | ||
logger: ToolingLog; | ||
}): Promise<T> => { | ||
let retryAttempt = 0; | ||
const response = await retryService.tryForTime( | ||
timeout, | ||
async () => { | ||
if (retryAttempt > retries) { | ||
// Log error message if we reached the maximum number of retries | ||
// but don't throw an error, return it to break the retry loop. | ||
const errorMessage = `Reached maximum number of retries for test ${utilityName}: ${ | ||
retryAttempt - 1 | ||
}/${retries}`; | ||
logger.error(errorMessage); | ||
return new Error(JSON.stringify(errorMessage)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably don't need to |
||
} | ||
|
||
retryAttempt = retryAttempt + 1; | ||
|
||
return await test(); | ||
}, | ||
undefined, | ||
retryDelay | ||
); | ||
|
||
// Now throw the error in order to fail the test. | ||
if (response instanceof Error) { | ||
throw response; | ||
} | ||
|
||
return response; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ export default function ({ getService }: FtrProviderContext) { | |
const supertest = getService('supertest'); | ||
const esDeleteAllIndices = getService('esDeleteAllIndices'); | ||
const logger = getService('log'); | ||
const retryService = getService('retry'); | ||
|
||
describe('Custom Threshold rule - CUSTOM_EQ - AVG - BYTES - FIRED', () => { | ||
const CUSTOM_THRESHOLD_RULE_ALERT_INDEX = '.alerts-observability.threshold.alerts-default'; | ||
|
@@ -67,12 +68,15 @@ export default function ({ getService }: FtrProviderContext) { | |
esClient, | ||
indexName: DATA_VIEW, | ||
docCountTarget: 75, | ||
retryService, | ||
logger, | ||
}); | ||
await createDataView({ | ||
supertest, | ||
name: DATA_VIEW, | ||
id: DATA_VIEW_ID, | ||
title: DATA_VIEW, | ||
logger, | ||
}); | ||
}); | ||
|
||
|
@@ -90,22 +94,25 @@ export default function ({ getService }: FtrProviderContext) { | |
await deleteDataView({ | ||
supertest, | ||
id: DATA_VIEW_ID, | ||
logger, | ||
}); | ||
await esDeleteAllIndices([ALERT_ACTION_INDEX, ...dataForgeIndices]); | ||
await cleanup({ client: esClient, config: dataForgeConfig, logger }); | ||
}); | ||
|
||
// FLAKY: https://github.com/elastic/kibana/issues/175360 | ||
describe.skip('Rule creation', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was fixed in #175479 |
||
describe('Rule creation', () => { | ||
it('creates rule successfully', async () => { | ||
actionId = await createIndexConnector({ | ||
supertest, | ||
name: 'Index Connector: Threshold API test', | ||
indexName: ALERT_ACTION_INDEX, | ||
logger, | ||
}); | ||
|
||
const createdRule = await createRule({ | ||
supertest, | ||
logger, | ||
esClient, | ||
tags: ['observability'], | ||
consumer: 'logs', | ||
name: 'Threshold rule', | ||
|
@@ -165,6 +172,8 @@ export default function ({ getService }: FtrProviderContext) { | |
id: ruleId, | ||
expectedStatus: 'active', | ||
supertest, | ||
retryService, | ||
logger, | ||
}); | ||
expect(executionStatus.status).to.be('active'); | ||
}); | ||
|
@@ -174,6 +183,8 @@ export default function ({ getService }: FtrProviderContext) { | |
esClient, | ||
indexName: CUSTOM_THRESHOLD_RULE_ALERT_INDEX, | ||
ruleId, | ||
retryService, | ||
logger, | ||
}); | ||
alertId = (resp.hits.hits[0]._source as any)['kibana.alert.uuid']; | ||
|
||
|
@@ -230,16 +241,18 @@ export default function ({ getService }: FtrProviderContext) { | |
const resp = await waitForDocumentInIndex<ActionDocument>({ | ||
esClient, | ||
indexName: ALERT_ACTION_INDEX, | ||
retryService, | ||
logger, | ||
}); | ||
|
||
expect(resp.hits.hits[0]._source?.ruleType).eql('observability.rules.custom_threshold'); | ||
expect(resp.hits.hits[0]._source?.alertDetailsUrl).eql( | ||
`https://localhost:5601/app/observability/alerts/${alertId}` | ||
); | ||
expect(resp.hits.hits[0]._source?.reason).eql( | ||
`Custom equation is 1, above the threshold of 0.9. (duration: 1 min, data view: ${DATA_VIEW})` | ||
`Custom equation is 1 B, above the threshold of 0.9 B. (duration: 1 min, data view: ${DATA_VIEW})` | ||
); | ||
expect(resp.hits.hits[0]._source?.value).eql('1'); | ||
expect(resp.hits.hits[0]._source?.value).eql('1 B'); | ||
}); | ||
}); | ||
}); | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a ticket to share this logic via retryService or a package or ...