AlexStocks commented on code in PR #3605:
URL: https://github.com/apache/dubbo-go/pull/3605#discussion_r3740274465
##########
metrics/metadata/collector_test.go:
##########
@@ -62,3 +65,175 @@ func TestNewMetadataMetricTimeEvent(t *testing.T) {
assert.NotNil(t, event.Attachment)
assert.Empty(t, event.Attachment)
}
+
+func TestMetadataMetricCollectorHandleMapping(t *testing.T) {
+ tests := []struct {
+ name string
+ eventName MetricName
+ handler func(*MetadataMetricCollector, *MetadataMetricEvent)
+ prefix string
+ }{
+ {
+ name: "register",
+ eventName: MetadataMappingRegister,
+ handler:
(*MetadataMetricCollector).handleMetadataMappingRegister,
+ prefix: "dubbo_metadata_mapping_register",
+ },
+ {
+ name: "get",
+ eventName: MetadataMappingGet,
+ handler:
(*MetadataMetricCollector).handleMetadataMappingGet,
+ prefix: "dubbo_metadata_mapping_get",
+ },
+ {
+ name: "listen",
+ eventName: MetadataMappingListen,
+ handler:
(*MetadataMetricCollector).handleMetadataMappingListen,
+ prefix: "dubbo_metadata_mapping_listen",
+ },
+ {
+ name: "remove",
+ eventName: MetadataMappingRemove,
+ handler:
(*MetadataMetricCollector).handleMetadataMappingRemove,
+ prefix: "dubbo_metadata_mapping_remove",
+ },
+ }
+
+ for _, tt := range tests {
+ for _, succ := range []bool{true, false} {
+ t.Run(fmt.Sprintf("%s/succ=%v", tt.name, succ), func(t
*testing.T) {
+ registry := newMockMetricRegistry()
+ collector :=
&MetadataMetricCollector{BaseCollector: metrics.BaseCollector{R: registry}}
+ event :=
NewMetadataMetricTimeEvent(tt.eventName)
+ event.End = event.Start.Add(10 *
time.Millisecond)
+ event.Succ = succ
+ event.Attachment[constant.InterfaceKey] =
"interfaceName"
+ event.Attachment[constant.GroupKey] = "group"
+ event.Attachment[constant.ApplicationKey] =
"application"
+
+ tt.handler(collector, event)
+
+ assert.InDelta(t, 1.0,
registry.counters[tt.prefix+"_num_total"], 0.000001)
+ if succ {
+ assert.InDelta(t, 1.0,
registry.counters[tt.prefix+"_num_succeed_total"], 0.000001)
+ assert.NotContains(t,
registry.counters, tt.prefix+"_num_failed_total")
+ } else {
+ assert.InDelta(t, 1.0,
registry.counters[tt.prefix+"_num_failed_total"], 0.000001)
+ assert.NotContains(t,
registry.counters, tt.prefix+"_num_succeed_total")
+ }
+ assert.Equal(t, []float64{10.0},
registry.rts[tt.prefix+"_rt_milliseconds"])
+
+ id := registry.ids[tt.prefix+"_num_total"]
+ assert.Equal(t, "interfaceName",
id.Tags[constant.TagInterface])
+ assert.Equal(t, "group",
id.Tags[constant.TagGroup])
+ assert.Equal(t, "application",
id.Tags[constant.TagApplicationName])
+ })
+ }
+ }
+}
+
+type mockMetricRegistry struct {
+ counters map[string]float64
+ rts map[string][]float64
+ ids map[string]*metrics.MetricId
+}
+
+func newMockMetricRegistry() *mockMetricRegistry {
+ return &mockMetricRegistry{
+ counters: make(map[string]float64),
+ rts: make(map[string][]float64),
+ ids: make(map[string]*metrics.MetricId),
+ }
+}
+
+func (m *mockMetricRegistry) Counter(id *metrics.MetricId)
metrics.CounterMetric {
+ m.ids[id.Name] = id
+ return &mockCounterMetric{m: m, name: id.Name}
+}
+
+func (m *mockMetricRegistry) Rt(id *metrics.MetricId, _ *metrics.RtOpts)
metrics.ObservableMetric {
+ m.ids[id.Name] = id
+ return &mockRtMetric{m: m, name: id.Name}
+}
+
+func (m *mockMetricRegistry) Gauge(id *metrics.MetricId) metrics.GaugeMetric {
+ return nil
+}
+
+func (m *mockMetricRegistry) Histogram(id *metrics.MetricId)
metrics.ObservableMetric {
+ return nil
+}
+
+func (m *mockMetricRegistry) Summary(id *metrics.MetricId)
metrics.ObservableMetric {
+ return nil
+}
+
+func (m *mockMetricRegistry) Export() {}
+
+type mockCounterMetric struct {
+ m *mockMetricRegistry
+ name string
+}
+
+func (c *mockCounterMetric) Inc() { c.m.counters[c.name]++ }
+func (c *mockCounterMetric) Add(v float64) { c.m.counters[c.name] += v }
+
+type mockRtMetric struct {
+ m *mockMetricRegistry
+ name string
+}
+
+func (r *mockRtMetric) Observe(v float64) { r.m.rts[r.name] =
append(r.m.rts[r.name], v) }
+
+// TestMetadataMetricCollectorPublishChain covers the production dispatch path:
+// a started collector subscribes to the event bus, and events published via
+// metrics.Publish must reach the registry. Removing any of the mapping switch
+// cases in start() must make this test fail.
+func TestMetadataMetricCollectorPublishChain(t *testing.T) {
+ registry := newMockMetricRegistry()
+ collector := &MetadataMetricCollector{BaseCollector:
metrics.BaseCollector{R: registry}}
+ collector.start()
+ defer metrics.Unsubscribe(constant.MetricsMetadata)
Review Comment:
[P1] 不要关闭包级 channel，并同步异步 registry 断言
`collector.start()` 使用包级 channel，这里的 `Unsubscribe` 会把它永久关闭；当前用例执行 `-count=2`
时第二轮稳定 panic：`send on closed channel` / `close of closed channel`。同时 collector
goroutine 在写 mock maps，而 `Eventually` 并发读取，`go test -race` 也稳定失败。请给测试提供可重建的专用
channel/teardown seam，不要关闭生产包级 channel，并给 mock registry 加锁或等待完成信号后再断言；修复后补
`-race` 与 `-count=2` 门禁。
--
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]
