[fix]: remove unused func #1895
Conversation
Signed-off-by: Sam Yuan <[email protected]>
SamYuan1990
requested review from
sthaha,
dave-tucker,
maryamtahhan,
marceloamaral,
rootfs and
sunya-ch
|
🤖 SeineSailor Here's a concise summary of the pull request changes: Summary: This pull request refactors the metric collector and watcher components, improving code readability and reducing complexity. Key Changes:
Impact: These changes improve code maintainability, reduce unnecessary complexity, and clean up unused code. Some changes affect the external interface and behavior of the code. Observations: The refactoring efforts are commendable, as they enhance code readability and simplify the logic. However, it's essential to ensure that the updated |
| @@ -99,15 +99,15 @@ func newK8sClient() *kubernetes.Clientset { | |||
| } | |||
|
|
|||
| func NewObjListWatcher(bpfSupportedMetrics bpf.SupportedMetrics) (*ObjListWatcher, error) { | |||
| w := &ObjListWatcher{ | |||
| w := ObjListWatcher{ | |||
There was a problem hiding this comment.
| @@ -63,15 +62,15 @@ type Collector struct { | |||
|
|
|||
| func NewCollector(bpfExporter bpf.Exporter) *Collector { | |||
| bpfSupportedMetrics := bpfExporter.SupportedMetrics() | |||
| c := &Collector{ | |||
| c := Collector{ | |||
There was a problem hiding this comment.
| // aggregate processes' resource utilization metrics to containers, virtual machines and nodes | ||
| c.AggregateProcessResourceUtilizationMetrics() | ||
| } | ||
|
|
||
| // update the node metrics that are not related to aggregated resource utilization of processes | ||
| func (c *Collector) updateNodeResourceUtilizationMetrics(wg *sync.WaitGroup) { |
There was a problem hiding this comment.
Do we want to keep this for TODO list when we have metrics those are not related to aggregated resource utilization of processes. @rootfs. Do you recall who introduce this function?
Note that, I have no objection on removing this.
|
can you split into two PRs? one for heap optimization and one for unused functions @SamYuan1990 |
it seems we don't need updateProcessResourceUtilizationMetrics function any more as it just done a wait group.
and if so, we can remove that wait group, as the wait group has 2 funcs to wait, and we removed one.