Skip to content

feat(objectstorage): support minio metrics support#6726

Draft
mlhiter wants to merge 1 commit intolabring:mainfrom
mlhiter:feat/minio-monitor-metrics-adjust
Draft

feat(objectstorage): support minio metrics support#6726
mlhiter wants to merge 1 commit intolabring:mainfrom
mlhiter:feat/minio-monitor-metrics-adjust

Conversation

@mlhiter
Copy link
Copy Markdown
Member

@mlhiter mlhiter commented Feb 27, 2026

No description provided.

Copilot AI review requested due to automatic review settings February 27, 2026 06:26
@mlhiter mlhiter requested a review from a team as a code owner February 27, 2026 06:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Object Storage provider’s monitoring API to query MinIO metrics via the shared sealos-metrics-sdk (VictoriaMetrics/Prometheus-compatible), and updates deployment/env configuration accordingly.

Changes:

  • Replace direct axios calls to MONITOR_URL with MetricsClient.minio.query() calls.
  • Add new env/config wiring for metrics endpoint, MinIO instance label, and optional Kubernetes host whitelist.
  • Add sealos-metrics-sdk as a workspace dependency and update the lockfile.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
frontend/providers/objectstorage/src/pages/api/monitor.ts Switch monitoring endpoint to use sealos-metrics-sdk and add whitelist parsing.
frontend/providers/objectstorage/package.json Add sealos-metrics-sdk workspace dependency.
frontend/providers/objectstorage/deploy/manifests/deploy.yaml.tmpl Replace MONITOR_URL with metrics-related env vars used by the new client.
frontend/providers/objectstorage/deploy/Kubefile Add templated env defaults for metrics configuration.
frontend/providers/objectstorage/.env.template Document new env vars for local/dev configuration.
frontend/pnpm-lock.yaml Workspace link added for sealos-metrics-sdk (plus associated lockfile changes).
Files not reviewed (1)
  • frontend/pnpm-lock.yaml: Language not supported

try {
const client = await initK8s({ req });
const { bucket } = req.body as { bucket?: string };
if (!bucket) return jsonRes(res, { code: 400, data: { error: 'bucketName is invaild' } });
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

Spelling: error message says "bucketName is invaild". Please correct to "bucketName is invalid" (and consider using a consistent error key like "bucket" if the client expects it).

Suggested change
if (!bucket) return jsonRes(res, { code: 400, data: { error: 'bucketName is invaild' } });
if (!bucket) return jsonRes(res, { code: 400, data: { error: 'bucketName is invalid' } });

Copilot uses AI. Check for mistakes.
Comment on lines +81 to +85
const result = response.data.result as typeof response.data.result & {
name?: string;
};
result.name = queries[i];
return result;
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

response.data.result is an array, and adding a name property to the array won’t survive JSON serialization (res.json ignores non-index array properties). If the client needs the metric name, return an object like { name: queries[i], result: response.data.result } (and update the client typing/usage accordingly) or include the name in each element instead of mutating the array.

Suggested change
const result = response.data.result as typeof response.data.result & {
name?: string;
};
result.name = queries[i];
return result;
return {
name: queries[i],
result: response.data.result
};

Copilot uses AI. Check for mistakes.
- name: OBJECT_STORAGE_INSTANCE
value: {{ default "object-storage.objectstorage-system.svc.cluster.local:80" .objectStorageInstance }}
- name: WHITELIST_KUBERNETES_HOSTS
value: "{{ .whitelistKubernetesHosts }}"
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

WHITELIST_KUBERNETES_HOSTS is templated as "{{ .whitelistKubernetesHosts }}" without a default. In other manifests, optional values use default "" ...; without it this may render as <no value>/null depending on the templater and then be treated as a non-empty whitelist string by the API. Use {{ default "" .whitelistKubernetesHosts }} (or omit the env var when empty) for consistent behavior.

Suggested change
value: "{{ .whitelistKubernetesHosts }}"
value: {{ default "" .whitelistKubernetesHosts }}

Copilot uses AI. Check for mistakes.
@mlhiter mlhiter marked this pull request as draft February 28, 2026 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants