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] updates to metrics api #195640

Merged
merged 7 commits into from
Oct 10, 2024
Merged
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
change metrics endpoint from GET to POST and update tests
neptunian committed Oct 10, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit a5747279767ce826f7c51121cab7427b0604a49d
45 changes: 16 additions & 29 deletions x-pack/plugins/data_usage/common/rest_types/usage_metrics.test.ts
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ import { UsageMetricsRequestSchema } from './usage_metrics';
describe('usage_metrics schemas', () => {
it('should accept valid request query', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
metricTypes: ['storage_retained'],
@@ -19,20 +19,9 @@ describe('usage_metrics schemas', () => {
).not.toThrow();
});

it('should accept a single `metricTypes` in request query', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
metricTypes: 'ingest_rate',
dataStreams: ['data_stream_1', 'data_stream_2', 'data_stream_3'],
})
).not.toThrow();
});

it('should accept multiple `metricTypes` in request query', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
metricTypes: ['ingest_rate', 'storage_retained', 'index_rate'],
@@ -43,7 +32,7 @@ describe('usage_metrics schemas', () => {

it('should accept `dataStream` list', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
metricTypes: ['storage_retained'],
@@ -54,7 +43,7 @@ describe('usage_metrics schemas', () => {

it('should error if `dataStream` list is empty', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
metricTypes: ['storage_retained'],
@@ -65,7 +54,7 @@ describe('usage_metrics schemas', () => {

it('should error if `dataStream` is given type not array', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
metricTypes: ['storage_retained'],
@@ -76,7 +65,7 @@ describe('usage_metrics schemas', () => {

it('should error if `dataStream` is given an empty item in the list', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
metricTypes: ['storage_retained'],
@@ -87,7 +76,7 @@ describe('usage_metrics schemas', () => {

it('should error if `metricTypes` is empty string', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
dataStreams: ['data_stream_1', 'data_stream_2', 'data_stream_3'],
@@ -98,7 +87,7 @@ describe('usage_metrics schemas', () => {

it('should error if `metricTypes` contains an empty item', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
dataStreams: ['data_stream_1', 'data_stream_2', 'data_stream_3'],
@@ -107,22 +96,20 @@ describe('usage_metrics schemas', () => {
).toThrowError(/list cannot contain empty values/);
});

it('should error if `metricTypes` is not a valid value', () => {
it('should error if `metricTypes` is not a valid type', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
dataStreams: ['data_stream_1', 'data_stream_2', 'data_stream_3'],
metricTypes: 'foo',
})
).toThrow(
'[metricTypes] must be one of storage_retained, ingest_rate, search_vcu, ingest_vcu, ml_vcu, index_latency, index_rate, search_latency, search_rate'
);
).toThrow('[metricTypes]: could not parse array value from json input');
});

it('should error if `metricTypes` is not a valid list', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: new Date().toISOString(),
dataStreams: ['data_stream_1', 'data_stream_2', 'data_stream_3'],
@@ -135,7 +122,7 @@ describe('usage_metrics schemas', () => {

it('should error if `from` is not a valid input', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: 1010,
to: new Date().toISOString(),
dataStreams: ['data_stream_1', 'data_stream_2', 'data_stream_3'],
@@ -146,7 +133,7 @@ describe('usage_metrics schemas', () => {

it('should error if `to` is not a valid input', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: 1010,
dataStreams: ['data_stream_1', 'data_stream_2', 'data_stream_3'],
@@ -157,7 +144,7 @@ describe('usage_metrics schemas', () => {

it('should error if `from` is empty string', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: ' ',
to: new Date().toISOString(),
dataStreams: ['data_stream_1', 'data_stream_2', 'data_stream_3'],
@@ -168,7 +155,7 @@ describe('usage_metrics schemas', () => {

it('should error if `to` is empty string', () => {
expect(() =>
UsageMetricsRequestSchema.query.validate({
UsageMetricsRequestSchema.validate({
from: new Date().toISOString(),
to: ' ',
dataStreams: ['data_stream_1', 'data_stream_2', 'data_stream_3'],
58 changes: 23 additions & 35 deletions x-pack/plugins/data_usage/common/rest_types/usage_metrics.ts
Original file line number Diff line number Diff line change
@@ -37,43 +37,31 @@ const metricTypesSchema = schema.oneOf(
// @ts-expect-error TS2769: No overload matches this call
METRIC_TYPE_VALUES.map((metricType) => schema.literal(metricType)) // Create a oneOf schema for the keys
);
export const UsageMetricsRequestSchema = {
query: schema.object({
from: DateSchema,
to: DateSchema,
metricTypes: schema.oneOf([
schema.arrayOf(schema.string(), {
minSize: 1,
validate: (values) => {
if (values.map((v) => v.trim()).some((v) => !v.length)) {
return '[metricTypes] list cannot contain empty values';
} else if (values.map((v) => v.trim()).some((v) => !isValidMetricType(v))) {
return `[metricTypes] must be one of ${METRIC_TYPE_VALUES.join(', ')}`;
}
},
}),
schema.string({
validate: (v) => {
if (!v.trim().length) {
return '[metricTypes] must have at least one value';
} else if (!isValidMetricType(v)) {
return `[metricTypes] must be one of ${METRIC_TYPE_VALUES.join(', ')}`;
}
},
}),
]),
dataStreams: schema.arrayOf(schema.string(), {
minSize: 1,
validate: (values) => {
if (values.map((v) => v.trim()).some((v) => !v.length)) {
return '[dataStreams] list cannot contain empty values';
}
},
}),
export const UsageMetricsRequestSchema = schema.object({
from: DateSchema,
to: DateSchema,
metricTypes: schema.arrayOf(schema.string(), {
minSize: 1,
validate: (values) => {
const trimmedValues = values.map((v) => v.trim());
if (trimmedValues.some((v) => !v.length)) {
return '[metricTypes] list cannot contain empty values';
} else if (trimmedValues.some((v) => !isValidMetricType(v))) {
return `[metricTypes] must be one of ${METRIC_TYPE_VALUES.join(', ')}`;
}
Comment on lines +47 to +51
Copy link
Member

Choose a reason for hiding this comment

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

Should also validate a string input instead of an array then the error should say that it expects and array and not a string. could not parse array value from json input is not valuable info to fix the request input, IMO.

},
}),
};
dataStreams: schema.arrayOf(schema.string(), {
minSize: 1,
validate: (values) => {
if (values.map((v) => v.trim()).some((v) => !v.length)) {
Copy link
Member

Choose a reason for hiding this comment

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

same validation here for a string intput instead of array

return '[dataStreams] list cannot contain empty values';
}
},
}),
});

export type UsageMetricsRequestSchemaQueryParams = TypeOf<typeof UsageMetricsRequestSchema.query>;
export type UsageMetricsRequestSchemaQueryParams = TypeOf<typeof UsageMetricsRequestSchema>;

export const UsageMetricsResponseSchema = {
body: () =>
20 changes: 8 additions & 12 deletions x-pack/plugins/data_usage/public/app/data_usage.tsx
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ export const DataUsage = () => {
setUrlDateRangeFilter,
} = useDataUsageMetricsUrlParams();

const [queryParams, setQueryParams] = useState<UsageMetricsRequestSchemaQueryParams>({
const [metricsFilters, setMetricsFilters] = useState<UsageMetricsRequestSchemaQueryParams>({
Copy link
Member

Choose a reason for hiding this comment

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

note: This file is renamed in my PR. you can leave it here as it is and I'll resolve conflicts in my PR.

metricTypes: ['storage_retained', 'ingest_rate'],
// TODO: Replace with data streams from /data_streams api
dataStreams: [
@@ -54,28 +54,24 @@ export const DataUsage = () => {

useEffect(() => {
if (!metricTypesFromUrl) {
setUrlMetricTypesFilter(
typeof queryParams.metricTypes !== 'string'
? queryParams.metricTypes.join(',')
: queryParams.metricTypes
);
setUrlMetricTypesFilter(metricsFilters.metricTypes.join(','));
}
if (!startDateFromUrl || !endDateFromUrl) {
setUrlDateRangeFilter({ startDate: queryParams.from, endDate: queryParams.to });
setUrlDateRangeFilter({ startDate: metricsFilters.from, endDate: metricsFilters.to });
}
}, [
endDateFromUrl,
metricTypesFromUrl,
queryParams.from,
queryParams.metricTypes,
queryParams.to,
metricsFilters.from,
metricsFilters.metricTypes,
metricsFilters.to,
setUrlDateRangeFilter,
setUrlMetricTypesFilter,
startDateFromUrl,
]);

useEffect(() => {
setQueryParams((prevState) => ({
setMetricsFilters((prevState) => ({
...prevState,
metricTypes: metricTypesFromUrl?.length ? metricTypesFromUrl : prevState.metricTypes,
dataStreams: dataStreamsFromUrl?.length ? dataStreamsFromUrl : prevState.dataStreams,
@@ -92,7 +88,7 @@ export const DataUsage = () => {
refetch: refetchDataUsageMetrics,
} = useGetDataUsageMetrics(
{
...queryParams,
...metricsFilters,
from: dateRangePickerState.startDate,
to: dateRangePickerState.endDate,
},
18 changes: 9 additions & 9 deletions x-pack/plugins/data_usage/public/hooks/use_get_usage_metrics.ts
Original file line number Diff line number Diff line change
@@ -21,24 +21,24 @@ interface ErrorType {
}

export const useGetDataUsageMetrics = (
query: UsageMetricsRequestSchemaQueryParams,
body: UsageMetricsRequestSchemaQueryParams,
options: UseQueryOptions<UsageMetricsResponseSchemaBody, IHttpFetchError<ErrorType>> = {}
): UseQueryResult<UsageMetricsResponseSchemaBody, IHttpFetchError<ErrorType>> => {
const http = useKibanaContextForPlugin().services.http;

return useQuery<UsageMetricsResponseSchemaBody, IHttpFetchError<ErrorType>>({
queryKey: ['get-data-usage-metrics', query],
queryKey: ['get-data-usage-metrics', body],
...options,
keepPreviousData: true,
queryFn: async () => {
return http.get<UsageMetricsResponseSchemaBody>(DATA_USAGE_METRICS_API_ROUTE, {
return http.post<UsageMetricsResponseSchemaBody>(DATA_USAGE_METRICS_API_ROUTE, {
version: '1',
query: {
from: query.from,
to: query.to,
metricTypes: query.metricTypes,
dataStreams: query.dataStreams,
},
body: JSON.stringify({
from: body.from,
to: body.to,
metricTypes: body.metricTypes,
dataStreams: body.dataStreams,
}),
});
},
});
Original file line number Diff line number Diff line change
@@ -17,15 +17,17 @@ export const registerUsageMetricsRoute = (
) => {
if (dataUsageContext.serverConfig.enabled) {
router.versioned
.get({
.post({
access: 'internal',
path: DATA_USAGE_METRICS_API_ROUTE,
})
.addVersion(
{
version: '1',
validate: {
request: UsageMetricsRequestSchema,
request: {
body: UsageMetricsRequestSchema,
},
response: {
200: UsageMetricsResponseSchema,
},