kubernetes / kube-state-metrics

Add-on agent to generate and expose cluster-level metrics.
https://kubernetes.io/docs/concepts/cluster-administration/kube-state-metrics/
Apache License 2.0
5.21k stars 1.93k forks source link

Potential cyclic imports #2103

Open CatherineF-dev opened 1 year ago

CatherineF-dev commented 1 year ago

What happened: Encounter some cyclic imports errors after rebasing a WIP PR to master. It means the code structure is lowly cohesive/ highly coupling, or the WIP PR needs refactoring imports.

Compared dependencies and found these newly added dependencies:

v2/pkg/app
├──v2/pkg/util
├──v2/internal/discovery

v2/pkg/customresourcestate
├──v2/pkg/util
├──v2/internal/discovery

v2/internal/store
├──v2/pkg/util

v2/pkg/util
└──v2/pkg/customresource
v2/internal/discovery
├──v2/pkg/metricshandler
├──v2/pkg/util
├──v2/internal/store
├──v2/pkg/customresource
└──v2/pkg/options
# before
godepgraph ./

v2/pkg/app
├──v2/pkg/metricshandler
├──v2/pkg/allowdenylist
├──v2/pkg/customresource
├──v2/pkg/customresourcestate
├──v2/pkg/options
├──v2/internal/store
├──v2/pkg/optin
├──v2/pkg/util/proc
└──v2/pkg/metric_generator

v2/pkg/customresourcestate
├──v2/pkg/metric_generator
├──v2/pkg/customresource
└──v2/pkg/metric

v2/pkg/optin
└──v2/pkg/metric_generator
v2/pkg/sharding
v2/pkg/util/proc
v2/pkg/allowdenylist
└──v2/pkg/metric_generator
v2/pkg/builder/types
├──v2/pkg/metrics_store
├──v2/pkg/customresource
├──v2/pkg/metric_generator
└──v2/pkg/options
v2/pkg/customresource
└──v2/pkg/metric_generator
v2/pkg/watch
v2/tests/e2e/framework
v2/tools
v2/internal
├──v2/pkg/app
└──v2/pkg/options
v2/internal/store
├──v2/pkg/watch
├──v2/pkg/customresource
├──v2/pkg/metric_generator
├──v2/pkg/options
├──v2/pkg/metrics_store
├──v2/pkg/metric
├──v2/pkg/sharding
├──v2/pkg/constant
└──v2/pkg/builder/types

v2/pkg/allow
v2/pkg/options
v2
├──v2/internal
└──v2/pkg/options
v2/pkg/builder
├──v2/internal/store
├──v2/pkg/builder/types
├──v2/pkg/customresource
├──v2/pkg/metric_generator
├──v2/pkg/metrics_store
└──v2/pkg/options
v2/pkg/metric
v2/pkg/metric_generator
└──v2/pkg/metric
v2/pkg/metrics_store/metricsstore
└──v2/pkg/metric
v2/pkg/metricshandler
├──v2/pkg/builder/types
├──v2/pkg/metrics_store
└──v2/pkg/options
# now
godepgraph ./

v2/pkg/app
├──v2/pkg/util
├──v2/internal/discovery
├──v2/pkg/optin
├──v2/pkg/customresourcestate
├──v2/pkg/metricshandler
├──v2/pkg/options
├──v2/pkg/allowdenylist
├──v2/internal/store
├──v2/pkg/metric_generator
├──v2/pkg/util/proc
└──v2/pkg/customresource
v2/pkg/customresourcestate
├──v2/pkg/util
├──v2/internal/discovery
└──v2/internal/store
├──v2/pkg/customresource
├──v2/pkg/metric
├──v2/pkg/metric_generator
v2/pkg/optin
└──v2/pkg/metric_generator
v2/pkg/sharding
v2/pkg/util/proc
v2/pkg/allowdenylist
└──v2/pkg/metric_generator
v2/pkg/builder/types
├──v2/pkg/options
├──v2/pkg/metrics_store
├──v2/pkg/customresource
└──v2/pkg/metric_generator
v2/pkg/customresource
└──v2/pkg/metric_generator
v2/pkg/watch
v2/tests/e2e/framework
v2/tools
v2/internal
├──v2/pkg/app
└──v2/pkg/options
v2/internal/store
├──v2/pkg/util
├──v2/pkg/sharding
├──v2/pkg/builder/types
├──v2/pkg/options
├──v2/pkg/metric
├──v2/pkg/metric_generator
├──v2/pkg/constant
├──v2/pkg/metrics_store
├──v2/pkg/watch
└──v2/pkg/customresource
v2/pkg/allow
v2/pkg/options
v2
├──v2/internal
└──v2/pkg/options
v2/pkg/builder
├──v2/internal/store
├──v2/pkg/builder/types
├──v2/pkg/metric_generator
├──v2/pkg/options
├──v2/pkg/customresource
└──v2/pkg/metrics_store
v2/pkg/metric
v2/pkg/metric_generator
└──v2/pkg/metric
v2/pkg/metrics_store/metricsstore
└──v2/pkg/metric
v2/pkg/metricshandler
├──v2/pkg/builder/types
├──v2/pkg/options
└──v2/pkg/metrics_store

v2/pkg/util
└──v2/pkg/customresource
v2/internal/discovery
├──v2/pkg/metricshandler
├──v2/pkg/util
├──v2/internal/store
├──v2/pkg/customresource
└──v2/pkg/options

What you expected to happen: Refactor some codes to make sure it's highly cohesive and lowly coupling.

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know?:

Environment:

CatherineF-dev commented 1 year ago

/assign

CatherineF-dev commented 1 year ago
  1. discovery imports many packages, which is similar to app package. Will move this under app.

  2. util are highly coupled with custom resource state feature. Will move this under customresource.

It's better to have less many imports in an independent package

dgrisonnet commented 1 year ago

/triage accepted

k8s-triage-robot commented 4 days ago

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted