-
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
[Data Usage] updates to metrics api #195640
Changes from 2 commits
08b6a92
1163d1f
2efd672
5746492
a574727
ba52d99
5ffe666
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,6 @@ import { PLUGIN_NAME } from '../../common'; | |
import { useGetDataUsageMetrics } from '../hooks/use_get_usage_metrics'; | ||
import { DEFAULT_DATE_RANGE_OPTIONS, useDateRangePicker } from './hooks/use_date_picker'; | ||
import { useDataUsageMetricsUrlParams } from './hooks/use_charts_url_params'; | ||
import { MetricsResponse } from './types'; | ||
|
||
export const DataUsage = () => { | ||
const { | ||
|
@@ -44,7 +43,11 @@ export const DataUsage = () => { | |
|
||
const [queryParams, setQueryParams] = useState<UsageMetricsRequestSchemaQueryParams>({ | ||
metricTypes: ['storage_retained', 'ingest_rate'], | ||
dataStreams: [], | ||
// TODO: Replace with data streams from /data_streams api | ||
dataStreams: [ | ||
'.alerts-ml.anomaly-detection-health.alerts-default', | ||
'.alerts-stack.alerts-default', | ||
], | ||
Comment on lines
+47
to
+50
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. Q: With respect to UX, should we use all the datastreams from the API here and have them all selected when we land on the page even if the list is very long, say 100 or more? 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 a question for a PM. Personally, I would say the "top" 10 would suffice. Otherwise we are potentially making the initial experience very slow unnecessarily. 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. Yeah, that is my concern. Also since we're pulling the datastreams list from another API ( 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. At the moment I think the plan is to not handle it and just display them all in the dropdown but only select the largest by storage size of 10. The dropdown with all the streams will ideally scroll and not go off the page. |
||
from: DEFAULT_DATE_RANGE_OPTIONS.startDate, | ||
to: DEFAULT_DATE_RANGE_OPTIONS.endDate, | ||
}); | ||
|
@@ -140,7 +143,7 @@ export const DataUsage = () => { | |
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
<EuiSpacer size="l" /> | ||
{isFetched && data ? <Charts data={data as MetricsResponse} /> : <EuiLoadingElastic />} | ||
{isFetched && data ? <Charts data={data} /> : <EuiLoadingElastic />} | ||
</EuiPageSection> | ||
</> | ||
); | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,10 @@ import { RequestHandler } from '@kbn/core/server'; | |
import { IndicesGetDataStreamResponse } from '@elastic/elasticsearch/lib/api/types'; | ||
import { | ||
MetricTypes, | ||
UsageMetricsAutoOpsResponseSchema, | ||
UsageMetricsAutoOpsResponseSchemaBody, | ||
UsageMetricsRequestSchemaQueryParams, | ||
UsageMetricsResponseSchema, | ||
UsageMetricsResponseSchemaBody, | ||
} from '../../../common/rest_types'; | ||
import { DataUsageContext, DataUsageRequestHandlerContext } from '../../types'; | ||
|
||
|
@@ -34,44 +36,27 @@ export const getUsageMetricsHandler = ( | |
const core = await context.core; | ||
const esClient = core.elasticsearch.client.asCurrentUser; | ||
|
||
// @ts-ignore | ||
const { from, to, metricTypes, dataStreams: dsNames, size } = request.query; | ||
const { from, to, metricTypes, dataStreams: requestDsNames } = request.query; | ||
logger.debug(`Retrieving usage metrics`); | ||
|
||
const { data_streams: dataStreamsResponse }: IndicesGetDataStreamResponse = | ||
await esClient.indices.getDataStream({ | ||
name: '*', | ||
name: requestDsNames, | ||
expand_wildcards: 'all', | ||
}); | ||
ashokaditya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const hasDataStreams = dataStreamsResponse.length > 0; | ||
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.
|
||
let userDsNames: string[] = []; | ||
|
||
if (dsNames?.length) { | ||
userDsNames = typeof dsNames === 'string' ? [dsNames] : dsNames; | ||
} else if (!userDsNames.length && hasDataStreams) { | ||
userDsNames = dataStreamsResponse.map((ds) => ds.name); | ||
} | ||
|
||
// If no data streams are found, return an empty response | ||
if (!userDsNames.length) { | ||
return response.ok({ | ||
body: { | ||
metrics: {}, | ||
}, | ||
}); | ||
} | ||
|
||
const metrics = await fetchMetricsFromAutoOps({ | ||
from, | ||
to, | ||
metricTypes: formatStringParams(metricTypes) as MetricTypes[], | ||
dataStreams: formatStringParams(userDsNames), | ||
dataStreams: formatStringParams(dataStreamsResponse.map((ds) => ds.name)), | ||
ashokaditya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
|
||
const processedMetrics = transformMetricsData(metrics); | ||
|
||
return response.ok({ | ||
body: { | ||
metrics, | ||
...processedMetrics, | ||
neptunian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
}); | ||
} catch (error) { | ||
|
@@ -94,7 +79,7 @@ const fetchMetricsFromAutoOps = async ({ | |
}) => { | ||
// TODO: fetch data from autoOps using userDsNames | ||
/* | ||
const response = await axios.post('https://api.auto-ops.{region}.{csp}.cloud.elastic.co/monitoring/serverless/v1/projects/{project_id}/metrics', { | ||
const response = await axios.post({AUTOOPS_URL}, { | ||
from: Date.parse(from), | ||
to: Date.parse(to), | ||
metric_types: metricTypes, | ||
|
@@ -231,7 +216,25 @@ const fetchMetricsFromAutoOps = async ({ | |
}, | ||
}; | ||
// Make sure data is what we expect | ||
const validatedData = UsageMetricsResponseSchema.body().validate(mockData); | ||
const validatedData = UsageMetricsAutoOpsResponseSchema.body().validate(mockData); | ||
|
||
return validatedData.metrics; | ||
return validatedData; | ||
}; | ||
function transformMetricsData( | ||
data: UsageMetricsAutoOpsResponseSchemaBody | ||
): UsageMetricsResponseSchemaBody { | ||
return { | ||
metrics: Object.fromEntries( | ||
Object.entries(data.metrics).map(([metricType, series]) => [ | ||
metricType, | ||
series.map((metricSeries) => ({ | ||
name: metricSeries.name, | ||
data: (metricSeries.data as Array<[number, number]>).map(([timestamp, value]) => ({ | ||
x: timestamp, | ||
y: value, | ||
})), | ||
})), | ||
]) | ||
), | ||
}; | ||
} |
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.
This schema has to be a oneOf(string, array). It should work with
api/path?dataStreams=xyz
and withapi/path?dataStreams=xyz&dataStreams=pqr
. The first one is a string and the second one is an array. So you can use the previous schema def without themaybe
wrapper.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.
Right. I think in my head I was thinking this should be a POST request and we could simplify it and metricTypes to be arrays only. Given they can pass in as many data streams as they want I think we should probably switch to a POST as I can see this URL getting very large. wdyt?