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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMPROVEMENT] Add PVC namespace to longhorn_volume metrics #7077

Closed
antoninferrand opened this issue Nov 11, 2023 · 2 comments
Closed

[IMPROVEMENT] Add PVC namespace to longhorn_volume metrics #7077

antoninferrand opened this issue Nov 11, 2023 · 2 comments
Assignees
Labels
area/monitoring System (cluster, node) or volume metrics, logs, stats backport/1.4.5 backport/1.5.3 component/longhorn-manager Longhorn manager (control plane) kind/improvement Request for improvement of existing function require/auto-e2e-test Require adding/updating auto e2e test cases if they can be automated require/backport Require backport. Only used when the specific versions to backport have not been definied. require/doc Require updating the longhorn.io documentation
Milestone

Comments

@antoninferrand
Copy link

antoninferrand commented Nov 11, 2023

Is your improvement request related to a feature? Please describe (馃憤 if you like this request)

In [IMPROVEMENT] Add pvc name to longhorn_volume metrics you added the PVC name to the volume metrics, however to entirely define a PVC we also need the namespace (multiple PVC can have the same name if they are in different namespaces).

Describe the solution you'd like

Add a "namespace" label to all longhorn_volume metrics, and fill it with the PVC namespace from v.Status.KubernetesStatus.Namespace.

Describe alternatives you've considered

n/a

Additional context

n/a

PRs

As the change is very similar to what you did a few months ago, I created the same PRs, let me know if this is a mistake.

@antoninferrand antoninferrand added kind/improvement Request for improvement of existing function require/auto-e2e-test Require adding/updating auto e2e test cases if they can be automated require/backport Require backport. Only used when the specific versions to backport have not been definied. require/doc Require updating the longhorn.io documentation labels Nov 11, 2023
@innobead innobead added this to the v1.6.0 milestone Nov 12, 2023
@longhorn-io-github-bot
Copy link

longhorn-io-github-bot commented Nov 12, 2023

Pre Ready-For-Testing Checklist

  • Where is the reproduce steps/test steps documented?
    The reproduce steps/test steps are at:

  • Is there a workaround for the issue? If so, where is it documented?
    The workaround is at:

  • Does the PR include the explanation for the fix or the feature?

  • Does the PR include deployment change (YAML/Chart)? If so, where are the PRs for both YAML file and Chart?
    The PR for the YAML change is at:
    The PR for the chart change is at:

  • Have the backend code been merged (Manager, Engine, Instance Manager, BackupStore etc) (including backport-needed/*)?
    The PR is at
    feat(metrics): add pvc namespace to volume metrics聽longhorn-manager#2284

  • Which areas/issues this PR might have potential impacts on?
    Area
    Issues

  • If labeled: require/LEP Has the Longhorn Enhancement Proposal PR submitted?
    The LEP PR is at

  • If labeled: area/ui Has the UI issue filed or ready to be merged (including backport-needed/*)?
    The UI issue/PR is at

  • If labeled: require/doc Has the necessary document PR submitted or merged (including backport-needed/*)?
    The documentation issue/PR is at
    doc(metrics): add pvc namespace to volume metrics聽website#796

  • If labeled: require/automation-e2e Has the end-to-end test plan been merged? Have QAs agreed on the automation test case? If only test case skeleton w/o implementation, have you created an implementation issue (including backport-needed/*)
    The automation skeleton PR is at
    The automation test case PR is at
    The issue of automation test case implementation is at (please create by the template)
    test(metrics): PVC namespace included in volume metrics聽longhorn-tests#1585

  • If labeled: require/automation-engine Has the engine integration test been merged (including backport-needed/*)?
    The engine automation PR is at

  • If labeled: require/manual-test-plan Has the manual test plan been documented?
    The updated manual test plan is at

  • If the fix introduces the code for backward compatibility Has a separate issue been filed with the label release/obsolete-compatibility?
    The compatibility issue is filed at

@roger-ryao
Copy link

roger-ryao commented Nov 15, 2023

Verified on master-head 20231115

Test Steps:

  1. Create vol-0 with PV/PVC in the longhorn-system namespace.
  2. Create vol-1 with PV/PVC in the local namespace.
  3. Create vol-2 with PV/PVC in the default namespace.
  4. Create vol-3 without PV/PVC
  5. Attach volumes to the node.
  6. Execute the command curl -sSL http://$longhorn_manager_ip:$longhorn_backend_service_port/metrics | grep longhorn_volume | grep pvc_namespace

Result Passed

Screenshot_20231115_180404

@roger-ryao roger-ryao added require/auto-e2e-test Require adding/updating auto e2e test cases if they can be automated and removed require/auto-e2e-test Require adding/updating auto e2e test cases if they can be automated labels Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/monitoring System (cluster, node) or volume metrics, logs, stats backport/1.4.5 backport/1.5.3 component/longhorn-manager Longhorn manager (control plane) kind/improvement Request for improvement of existing function require/auto-e2e-test Require adding/updating auto e2e test cases if they can be automated require/backport Require backport. Only used when the specific versions to backport have not been definied. require/doc Require updating the longhorn.io documentation
Projects
Status: Resolved/Scheduled
Development

No branches or pull requests

6 participants