-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(applaunchpad): support metrics-sdk #6720
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,91 +3,46 @@ import { getK8s } from '@/services/backend/kubernetes'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { jsonRes } from '@/services/backend/response'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ApiResp } from '@/services/kubernet'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { monitorFetch } from '@/services/monitorFetch'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { MonitorServiceResult, MonitorDataResult, MonitorQueryKey } from '@/types/monitor'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { MonitorDataResult, MonitorQueryKey } from '@/types/monitor'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { NextApiRequest, NextApiResponse } from 'next'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LaunchpadMetricType, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LaunchpadQueryParams, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LaunchpadQueryResult | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from 'sealos-metrics-sdk'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const normalizeQueryParam = (value: string | string[] | undefined) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Array.isArray(value) ? value[0] : value; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+14
to
+15
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const normalizeQueryParam = (value: string | string[] | undefined) => | |
| Array.isArray(value) ? value[0] : value; | |
| const normalizeQueryParam = (value: string | string[] | undefined) => { | |
| if (value === undefined) return value; | |
| return Array.isArray(value) ? value[0] : value; | |
| }; |
Copilot
AI
Feb 26, 2026
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.
The adaptChartData function uses Number.parseFloat(value[1]).toFixed(2) which returns a string. Since yData is defined as string[] in the MonitorDataResult type, this is technically correct. However, the original code used parseFloat(value[1]).toFixed(2) which has the same behavior. The change to Number.parseFloat is fine, but ensure this is intentional as it adds an extra level of namespace access without functional benefit.
Copilot
AI
Feb 26, 2026
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.
The fallback logic in adaptChartData (line 22) uses item.metric[metricName] || item.metric.pod || item.metric.persistentvolumeclaim to ensure a name is always provided. This is good defensive programming. However, ensure that this fallback is tested, as it could mask issues where the expected metric name is missing. Consider logging a warning when falling back to avoid silent failures in production.
| data.data.result.map((item) => ({ | |
| name: item.metric[metricName] || item.metric.pod || item.metric.persistentvolumeclaim, | |
| xData: item.values?.map((value) => Number(value[0])) ?? [], | |
| yData: item.values?.map((value) => Number.parseFloat(value[1]).toFixed(2)) ?? [] | |
| })); | |
| data.data.result.map((item) => { | |
| const primaryName = item.metric[metricName]; | |
| const fallbackName = item.metric.pod || item.metric.persistentvolumeclaim; | |
| const name = primaryName || fallbackName || 'unknown'; | |
| if (!primaryName && fallbackName) { | |
| console.warn( | |
| `[monitor] adaptChartData: missing expected metric "${metricName}" in item.metric, using fallback name "${fallbackName}".` | |
| ); | |
| } else if (!primaryName && !fallbackName) { | |
| console.warn( | |
| `[monitor] adaptChartData: missing "${metricName}", "pod", and "persistentvolumeclaim" in item.metric; using "unknown" as name.` | |
| ); | |
| } | |
| return { | |
| name, | |
| xData: item.values?.map((value) => Number(value[0])) ?? [], | |
| yData: item.values?.map((value) => Number.parseFloat(value[1]).toFixed(2)) ?? [] | |
| }; | |
| }); |
Copilot
AI
Feb 26, 2026
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.
The queryTypeMap maps storage to disk, but the SDK's LaunchpadMetricType may not distinguish between these two. Verify that the SDK correctly handles the disk type for storage queries, especially when a pvcName is provided. The mapping seems correct based on line 45 where both use disk, but ensure this is documented in the SDK.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,29 +1,13 @@ | ||||||||||||||||||||||||||||||||
| import { AxiosRequestConfig } from 'axios'; | ||||||||||||||||||||||||||||||||
| import { MetricsClient } from 'sealos-metrics-sdk'; | ||||||||||||||||||||||||||||||||
| import type { LaunchpadQueryParams } from 'sealos-metrics-sdk'; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export const monitorFetch = async (props: AxiosRequestConfig, kubeconfig: string) => { | ||||||||||||||||||||||||||||||||
| const { url, params } = props; | ||||||||||||||||||||||||||||||||
| const queryString = typeof params === 'object' ? new URLSearchParams(params).toString() : params; | ||||||||||||||||||||||||||||||||
| const requestOptions = { | ||||||||||||||||||||||||||||||||
| method: 'GET', | ||||||||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||||||||
| Authorization: encodeURIComponent(kubeconfig) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| const doMain = | ||||||||||||||||||||||||||||||||
| global.AppConfig.launchpad.components.monitor.url || | ||||||||||||||||||||||||||||||||
| 'http://launchpad-monitor.sealos.svc.cluster.local:8428'; | ||||||||||||||||||||||||||||||||
| export const monitorFetch = async (params: LaunchpadQueryParams, kubeconfig: string) => { | ||||||||||||||||||||||||||||||||
| const metricsConfig = global.AppConfig?.launchpad?.components?.metrics; | ||||||||||||||||||||||||||||||||
| const client = new MetricsClient({ | ||||||||||||||||||||||||||||||||
| kubeconfig, | ||||||||||||||||||||||||||||||||
| metricsURL: metricsConfig?.url, | ||||||||||||||||||||||||||||||||
| whitelistKubernetesHosts: metricsConfig?.whitelistKubernetesHosts | ||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+9
|
||||||||||||||||||||||||||||||||
| const client = new MetricsClient({ | |
| kubeconfig, | |
| metricsURL: metricsConfig?.url, | |
| whitelistKubernetesHosts: metricsConfig?.whitelistKubernetesHosts | |
| if (!metricsConfig || (metricsConfig.url === undefined && metricsConfig.whitelistKubernetesHosts === undefined)) { | |
| throw new Error('Metrics configuration for launchpad is missing or invalid: expected metrics.url or metrics.whitelistKubernetesHosts to be defined.'); | |
| } | |
| const client = new MetricsClient({ | |
| kubeconfig, | |
| metricsURL: metricsConfig.url, | |
| whitelistKubernetesHosts: metricsConfig.whitelistKubernetesHosts |
Copilot
AI
Feb 26, 2026
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.
The new monitorFetch function removes all explicit error handling that was present in the original implementation. The original code had a try-catch block with environment-specific error messages. While the SDK may handle errors internally, consider wrapping the SDK call in a try-catch block to provide more context-specific error messages for debugging, especially during development. This will help identify whether issues are with the SDK configuration, network connectivity, or authentication.
| return client.launchpad.query(params); | |
| try { | |
| return await client.launchpad.query(params); | |
| } catch (error) { | |
| if (process.env.NODE_ENV !== 'production') { | |
| // Provide more context during development to help debug SDK, network, or auth issues | |
| // Do not change the error shape; just add logging. | |
| // eslint-disable-next-line no-console | |
| console.error('monitorFetch: failed to query launchpad metrics', { | |
| error, | |
| metricsURL: metricsConfig?.url | |
| }); | |
| } | |
| throw error; | |
| } |
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.
The deployment template uses
{{ default "..." .metricsUrl }}which provides a sensible default. However, ensure that the.metricsUrlvariable is documented in the deployment instructions or README, as this is a new configuration parameter that replaces the old.monitorUrl. Consider adding migration documentation for users upgrading from the old monitor configuration to the new metrics configuration.