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 Usage] setup integration tests #197112

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

neptunian
Copy link
Contributor

@neptunian neptunian commented Oct 21, 2024

Summary

Adds serverless api integration tests for data usage plugin

Both are skipped in mki until xpack.dataUsage.enabled is enabled by default in serverless

POST /internal/api/data_usage/data_streams

POST /internal/api/data_usage/metrics

  • skipped in MKI because we'll need to make sure real credentials are being used via the xpack.dataUsage.autoops*
  • we start a mock server at localhost:9000 and set that the config (xpack.dataUsage.autoops.api.url=http://localhost:9000') along with fake credentials for the other xpack.dataUsage.autoops* values. If we're not in MKI these values will be used and the mock server will respond to the request at http://localhost:9000. If we are in MKI, the real values and credentials should be set, otherwise it will fail as these kibana config values in the tests are not passed into the MKI environment.

@@ -134,8 +135,10 @@ export class AutoOpsAPIService {
}
);

const validatedResponse = UsageMetricsAutoOpsResponseSchema.body().validate(response.data);
Copy link
Contributor Author

@neptunian neptunian Oct 21, 2024

Choose a reason for hiding this comment

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

I think we should validate the autoops response at runtime. The error will be caught further up.

@elastic elastic deleted a comment from elasticmachine Oct 22, 2024
@neptunian
Copy link
Contributor Author

/ci

1 similar comment
@neptunian
Copy link
Contributor Author

/ci

@@ -21,5 +22,22 @@ export default createTestConfig({
kbnServerArgs: [
// useful for testing (also enabled in MKI QA)
'--coreApp.allowDynamicConfigOverrides=true',
'--xpack.dataUsage.enabled=true',
Copy link
Contributor Author

@neptunian neptunian Oct 23, 2024

Choose a reason for hiding this comment

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

we need to enable this to run the tests in MKI. once we enable this in serverless only, we can remove this.

@neptunian neptunian force-pushed the data-usage-serverless-api-integration-tests branch 2 times, most recently from 55c96a7 to c786ac9 Compare October 23, 2024 19:13
@neptunian neptunian added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes labels Oct 23, 2024
>;
>['metrics'][MetricTypes][number];

export type UsageMetricsAutoOpsResponseSchemaBody = Omit<
Copy link
Contributor Author

@neptunian neptunian Oct 23, 2024

Choose a reason for hiding this comment

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

I changed this type because I was getting a type error when only sending a subset of metrics (using the type in mock server response), so this allows the subset instead of all. @ashokaditya Hope thats ok.

@neptunian neptunian marked this pull request as ready for review October 23, 2024 19:21
@neptunian neptunian requested review from a team as code owners October 23, 2024 19:21
@neptunian neptunian requested review from a team as code owners October 23, 2024 20:01
@neptunian neptunian force-pushed the data-usage-serverless-api-integration-tests branch from 41e2585 to dff67dc Compare October 23, 2024 20:05
@neptunian neptunian removed request for a team October 23, 2024 20:06
@@ -25,5 +26,11 @@ export default createTestConfig({
'--coreApp.allowDynamicConfigOverrides=true',
`--xpack.securitySolutionServerless.cloudSecurityUsageReportingTaskInterval=5s`,
`--xpack.securitySolutionServerless.usageApi.url=http://localhost:8081`,
'--xpack.dataUsage.enabled=true',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'--xpack.dataUsage.enabled=true',
'--xpack.dataUsage.autoops.enabled=true',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added --xpack.dataUsage.autoops.enabled=true, though left --xpack.dataUsage.enabled=true in place so as not to enable the plugin.

@@ -21,5 +22,11 @@ export default createTestConfig({
kbnServerArgs: [
// useful for testing (also enabled in MKI QA)
'--coreApp.allowDynamicConfigOverrides=true',
'--xpack.dataUsage.enabled=true',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'--xpack.dataUsage.enabled=true',
'--xpack.dataUsage.autoops.enabled=true',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added --xpack.dataUsage.autoops.enabled=true, though left --xpack.dataUsage.enabled=true in place so as not to enable the plugin.

import { DataStreamsResponseBodySchemaBody } from '@kbn/data-usage-plugin/common/rest_types';
import { FtrProviderContext } from '../../../../ftr_provider_context';

const API_PATH = '/internal/api/data_usage/data_streams';
Copy link
Member

Choose a reason for hiding this comment

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

You can import and use DATA_USAGE_METRICS_API_ROUTE instead.

import { DATA_USAGE_METRICS_API_ROUTE } from '@kbn/data-usage-plugin/common';

.set('elastic-api-version', '1')
.send(requestBody);
expect(res.statusCode).to.be(200);
// TODO: decide on how to generate data
Copy link
Member

Choose a reason for hiding this comment

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

I think we should have a function that takes in a date range and spits out some metrics for that range. Maybe a data point every hour?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm thinking to use synthtrace

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

ResponseOps changes (in x-pack/test_serverless/api_integration/test_suites/observability/config.ts) LGTM.

Side note: no idea why Response Ops is marked for these files, which is why we got pinged:

kibana/.github/CODEOWNERS

Lines 1488 to 1489 in c41178d

/x-pack/test_serverless/api_integration/test_suites/observability/config.ts @elastic/response-ops
/x-pack/test_serverless/api_integration/test_suites/observability/index.ts @elastic/response-ops

Perhaps fix that in this PR as well?

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

x-pack/test_serverless/api_integration/test_suites/observability/common_configs/config.group1.ts and x-pack/test_serverless/tsconfig.json changes LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants