robocanic commented on code in PR #1345: URL: https://github.com/apache/dubbo-admin/pull/1345#discussion_r2484423827
########## pkg/console/counter/manager.go: ########## @@ -0,0 +1,370 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package counter + +import ( + "fmt" + "math" + + "k8s.io/client-go/tools/cache" + + "github.com/apache/dubbo-admin/pkg/console/model" + "github.com/apache/dubbo-admin/pkg/core/events" + meshresource "github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1" + coremodel "github.com/apache/dubbo-admin/pkg/core/resource/model" +) + +type CounterType string + +const ( + ApplicationCounter CounterType = "application" + ServiceCounter CounterType = "service" + InstanceCounter CounterType = "instance" + + ProtocolCounter CounterType = "protocol" + ReleaseCounter CounterType = "release" + DiscoveryCounter CounterType = "discovery" +) + +type CounterManager interface { + Overview() *model.OverviewResp + Reset() + Increment(counterType CounterType) error + Decrement(counterType CounterType) error + IncrementDistribution(counterType CounterType, key string) error + DecrementDistribution(counterType CounterType, key string) error + Bind(bus events.EventBus) error +} + +type counterManager struct { + simpleCounters map[CounterType]*Counter + distCounters map[CounterType]*DistributionCounter +} + +func NewCounterManager() CounterManager { + return newCounterManager() +} + +func newCounterManager() *counterManager { + return &counterManager{ + simpleCounters: map[CounterType]*Counter{ + ApplicationCounter: NewCounter("application"), + ServiceCounter: NewCounter("service"), + InstanceCounter: NewCounter("instance"), + }, + distCounters: map[CounterType]*DistributionCounter{ + ProtocolCounter: NewDistributionCounter("protocol"), + ReleaseCounter: NewDistributionCounter("release"), + DiscoveryCounter: NewDistributionCounter("discovery"), + }, + } +} + +func (cm *counterManager) Reset() { + for _, counter := range cm.simpleCounters { + counter.Reset() + } + for _, counter := range cm.distCounters { + counter.Reset() + } +} + +func (cm *counterManager) Increment(counterType CounterType) error { + counter, exists := cm.simpleCounters[counterType] + if !exists { + return fmt.Errorf("counter type %s not found", counterType) + } + counter.Increment() + return nil +} + +func (cm *counterManager) Decrement(counterType CounterType) error { + counter, exists := cm.simpleCounters[counterType] + if !exists { + return fmt.Errorf("counter type %s not found", counterType) + } + counter.Decrement() + return nil +} + +func (cm *counterManager) IncrementDistribution(counterType CounterType, key string) error { + counter, exists := cm.distCounters[counterType] + if !exists { + return fmt.Errorf("distribution counter type %s not found", counterType) + } + counter.Increment(normalizeDistributionKey(key)) + return nil +} + +func (cm *counterManager) DecrementDistribution(counterType CounterType, key string) error { + counter, exists := cm.distCounters[counterType] + if !exists { + return fmt.Errorf("distribution counter type %s not found", counterType) + } + counter.Decrement(normalizeDistributionKey(key)) + return nil +} + +func (cm *counterManager) Overview() *model.OverviewResp { Review Comment: suggestion: Overview这个方法里的逻辑应该要放到web层来做，CounterManager只专注于count。暴露给外层的应该只是一个类似GetCount(ResourceKind)这样的方法 ########## pkg/console/counter/manager.go: ########## @@ -0,0 +1,370 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package counter + +import ( + "fmt" + "math" + + "k8s.io/client-go/tools/cache" + + "github.com/apache/dubbo-admin/pkg/console/model" + "github.com/apache/dubbo-admin/pkg/core/events" + meshresource "github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1" + coremodel "github.com/apache/dubbo-admin/pkg/core/resource/model" +) + +type CounterType string + +const ( + ApplicationCounter CounterType = "application" + ServiceCounter CounterType = "service" + InstanceCounter CounterType = "instance" + + ProtocolCounter CounterType = "protocol" + ReleaseCounter CounterType = "release" + DiscoveryCounter CounterType = "discovery" +) + +type CounterManager interface { + Overview() *model.OverviewResp + Reset() + Increment(counterType CounterType) error + Decrement(counterType CounterType) error + IncrementDistribution(counterType CounterType, key string) error + DecrementDistribution(counterType CounterType, key string) error Review Comment: suggestion: Increment,Decrement,IncrementDistribution,DecrementDistribution这几个接口以及接口实现放在CounterManager这个Manager中不太合适。从架构上来说，CounterManager是Counter的管理者，对外提供的应该是类似AddCounter，DeleteCounter()这样的方法 ########## pkg/console/counter/manager.go: ########## @@ -0,0 +1,370 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package counter + +import ( + "fmt" + "math" + + "k8s.io/client-go/tools/cache" + + "github.com/apache/dubbo-admin/pkg/console/model" + "github.com/apache/dubbo-admin/pkg/core/events" + meshresource "github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1" + coremodel "github.com/apache/dubbo-admin/pkg/core/resource/model" +) + +type CounterType string + +const ( + ApplicationCounter CounterType = "application" Review Comment: suggestion：如果要增加某种类型的Counter，需要新增counterType，和对应的事件回调函数。看能不能在易用性上完善一下。可以参考的思路如下： 拆解一下Counter的类型，目前定义的counter有两种，一种是记录某种resource的累计数量(ResourceKind->countValue)，另一种是记录根据resource的某一个字段的累计数量(ResourceKind->{key,value})，这两种都可以关联到ResourceKind上。对于第一种counter，可以以resourceKind为map key，simple counter为map value，不同的resource的事件处理逻辑完全一致，因此可以统一起来。对于第二种counter，可以以resourceKind为map key，distribution counter为map value，每种key的处理逻辑不一样，所以可以把这个处理逻辑抽象成一个func类型，这个func关注一个事件，返回一个数值，然后把这个数值更新到distribution counter的key上。至此，两种counter类型都可以关联到resourcekind上，对其他组件来说，如果想新增一种类型的counter，只需要少量甚至不需要额外代码就能扩展的。 -- 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]
