Copilot commented on code in PR #1390:
URL: https://github.com/apache/dubbo-admin/pull/1390#discussion_r2712089991
##########
pkg/console/counter/manager.go:
##########
@@ -183,36 +205,72 @@ func (cm *counterManager) handleEvent(kind
resmodel.ResourceKind, event events.E
return nil
}
+func (cm *counterManager) getDistributionConfig(kind resmodel.ResourceKind,
metric CounterType) *distributionCounterConfig {
+ configs := cm.distributionConfigs[kind]
+ for _, cfg := range configs {
+ if cfg.counterType == metric {
+ return cfg
+ }
+ }
+ return nil
+}
+
+func extractMeshName(res resmodel.Resource) string {
+ if res == nil {
+ return "default"
+ }
+ mesh := res.ResourceMesh()
+ if mesh == "" {
+ return "default"
+ }
+ return mesh
+}
+
func processSimpleCounter(counter *Counter, event events.Event) {
+ mesh := extractMeshName(event.NewObj())
+ if event.NewObj() == nil {
+ mesh = extractMeshName(event.OldObj())
+ }
+
switch event.Type() {
case cache.Added:
- counter.Increment()
+ counter.Increment(mesh)
+ logger.Debugf("CounterManager: Increment %s for mesh=%s,
current count=%d", counter.name, mesh, counter.GetByGroup(mesh))
case cache.Sync, cache.Replaced:
if isNewResourceEvent(event) {
- counter.Increment()
+ counter.Increment(mesh)
+ logger.Debugf("CounterManager: Increment %s for mesh=%s
(Sync/Replaced), current count=%d", counter.name, mesh,
counter.GetByGroup(mesh))
}
case cache.Deleted:
- counter.Decrement()
+ counter.Decrement(mesh)
+ logger.Debugf("CounterManager: Decrement %s for mesh=%s,
current count=%d", counter.name, mesh, counter.GetByGroup(mesh))
case cache.Updated:
Review Comment:
Simple counters do not handle mesh changes during update events. If a
resource's mesh changes from "meshA" to "meshB" during an update, the counter
for "meshA" should decrement and "meshB" should increment, but currently the
update case is a no-op. This can lead to incorrect counts when resources move
between meshes.
```suggestion
case cache.Updated:
oldMesh := extractMeshName(event.OldObj())
newMesh := extractMeshName(event.NewObj())
if oldMesh != newMesh {
counter.Decrement(oldMesh)
logger.Debugf("CounterManager: Decrement %s for mesh=%s
(Updated), current count=%d", counter.name, oldMesh,
counter.GetByGroup(oldMesh))
counter.Increment(newMesh)
logger.Debugf("CounterManager: Increment %s for mesh=%s
(Updated), current count=%d", counter.name, newMesh,
counter.GetByGroup(newMesh))
}
```
##########
pkg/console/counter/manager.go:
##########
@@ -42,6 +43,8 @@ type CounterManager interface {
RegisterDistributionCounter(kind resmodel.ResourceKind, metric
CounterType, extractor FieldExtractor)
Count(kind resmodel.ResourceKind) int64
Distribution(metric CounterType) map[string]int64
+ CountByMesh(kind resmodel.ResourceKind, mesh string) int64
Review Comment:
The newly added CountByMesh and DistributionByMesh methods lack
documentation comments. Public interface methods should have comments
describing their purpose, parameters, and return values to help API consumers
understand how to use them.
```suggestion
Distribution(metric CounterType) map[string]int64
// CountByMesh returns the number of resources of the given kind that
belong to the specified mesh.
// The mesh parameter identifies the mesh to filter on.
CountByMesh(kind resmodel.ResourceKind, mesh string) int64
// DistributionByMesh returns a distribution of counts for the given
metric limited to the specified mesh.
// The returned map keys are metric values (for example, protocol or
release) and the values are their counts.
```
##########
pkg/console/counter/counter.go:
##########
@@ -19,79 +19,149 @@ package counter
import (
"sync"
- "sync/atomic"
)
type Counter struct {
- name string
- value atomic.Int64
+ name string
+ data map[string]int64
+ mu sync.RWMutex
}
func NewCounter(name string) *Counter {
- return &Counter{name: name}
+ return &Counter{
+ name: name,
+ data: make(map[string]int64),
+ }
}
func (c *Counter) Get() int64 {
- return c.value.Load()
+ c.mu.RLock()
+ defer c.mu.RUnlock()
+ var sum int64
+ for _, v := range c.data {
+ sum += v
+ }
+ return sum
}
-func (c *Counter) Increment() {
- c.value.Add(1)
+func (c *Counter) GetByGroup(group string) int64 {
+ if group == "" {
+ group = "default"
+ }
+ c.mu.RLock()
+ defer c.mu.RUnlock()
+ return c.data[group]
}
Review Comment:
The newly added GetByGroup method lacks documentation. Public methods should
have comments describing their purpose, parameters, and return values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
