Copilot commented on code in PR #857:
URL:
https://github.com/apache/skywalking-banyandb/pull/857#discussion_r2548477268
##########
banyand/liaison/grpc/metrics.go:
##########
@@ -43,26 +43,50 @@ type metrics struct {
totalRegistryFinished meter.Counter
totalRegistryErr meter.Counter
totalRegistryLatency meter.Counter
+
+ memoryLoadSheddingRejections meter.Counter
+ grpcBufferSizeConn meter.Gauge
+ grpcBufferSizeStream meter.Gauge
+ memoryState meter.Gauge
}
func newMetrics(factory *observability.Factory) *metrics {
return &metrics{
- totalStarted: factory.NewCounter("total_started",
"group", "service", "method"),
- totalFinished: factory.NewCounter("total_finished",
"group", "service", "method"),
- totalErr: factory.NewCounter("total_err",
"group", "service", "method"),
- totalPanic: factory.NewCounter("total_panic"),
- totalLatency: factory.NewCounter("total_latency",
"group", "service", "method"),
- totalStreamStarted:
factory.NewCounter("total_stream_started", "service", "method"),
- totalStreamFinished:
factory.NewCounter("total_stream_finished", "service", "method"),
- totalStreamErr:
factory.NewCounter("total_stream_err", "service", "method"),
- totalStreamLatency:
factory.NewCounter("total_stream_latency", "service", "method"),
- totalStreamMsgReceived:
factory.NewCounter("total_stream_msg_received", "group", "service", "method"),
- totalStreamMsgReceivedErr:
factory.NewCounter("total_stream_msg_received_err", "group", "service",
"method"),
- totalStreamMsgSent:
factory.NewCounter("total_stream_msg_sent", "group", "service", "method"),
- totalStreamMsgSentErr:
factory.NewCounter("total_stream_msg_sent_err", "group", "service", "method"),
- totalRegistryStarted:
factory.NewCounter("total_registry_started", "group", "service", "method"),
- totalRegistryFinished:
factory.NewCounter("total_registry_finished", "group", "service", "method"),
- totalRegistryErr:
factory.NewCounter("total_registry_err", "group", "service", "method"),
- totalRegistryLatency:
factory.NewCounter("total_registry_latency", "group", "service", "method"),
+ totalStarted:
factory.NewCounter("total_started", "group", "service", "method"),
+ totalFinished:
factory.NewCounter("total_finished", "group", "service", "method"),
+ totalErr: factory.NewCounter("total_err",
"group", "service", "method"),
+ totalPanic: factory.NewCounter("total_panic"),
+ totalLatency:
factory.NewCounter("total_latency", "group", "service", "method"),
+ totalStreamStarted:
factory.NewCounter("total_stream_started", "service", "method"),
+ totalStreamFinished:
factory.NewCounter("total_stream_finished", "service", "method"),
+ totalStreamErr:
factory.NewCounter("total_stream_err", "service", "method"),
+ totalStreamLatency:
factory.NewCounter("total_stream_latency", "service", "method"),
+ totalStreamMsgReceived:
factory.NewCounter("total_stream_msg_received", "group", "service", "method"),
+ totalStreamMsgReceivedErr:
factory.NewCounter("total_stream_msg_received_err", "group", "service",
"method"),
+ totalStreamMsgSent:
factory.NewCounter("total_stream_msg_sent", "group", "service", "method"),
+ totalStreamMsgSentErr:
factory.NewCounter("total_stream_msg_sent_err", "group", "service", "method"),
+ totalRegistryStarted:
factory.NewCounter("total_registry_started", "group", "service", "method"),
+ totalRegistryFinished:
factory.NewCounter("total_registry_finished", "group", "service", "method"),
+ totalRegistryErr:
factory.NewCounter("total_registry_err", "group", "service", "method"),
+ totalRegistryLatency:
factory.NewCounter("total_registry_latency", "group", "service", "method"),
+ memoryLoadSheddingRejections:
factory.NewCounter("memory_load_shedding_rejections_total", "service"),
+ grpcBufferSizeConn:
factory.NewGauge("grpc_buffer_size_bytes", "type"),
+ grpcBufferSizeStream:
factory.NewGauge("grpc_buffer_size_bytes", "type"),
+ memoryState: factory.NewGauge("memory_state"),
+ }
+}
+
+// updateBufferSizeMetrics updates the buffer size metrics.
+func (m *metrics) updateBufferSizeMetrics(connSize, streamSize int32) {
+ if connSize > 0 {
+ m.grpcBufferSizeConn.Set(float64(connSize), "conn")
}
+ if streamSize > 0 {
+ m.grpcBufferSizeStream.Set(float64(streamSize), "stream")
+ }
Review Comment:
The `updateBufferSizeMetrics` method expects to use separate gauge instances
(`grpcBufferSizeConn` and `grpcBufferSizeStream`), but they should be
consolidated into a single gauge with label values. Currently, the method calls
`Set` with a "type" label value ("conn" or "stream"), but the gauges were
created with the "type" label specified in the constructor.
Since both gauges have the same name and same labels in line 73-74, they
should be a single gauge instance. Update this method to use a single gauge
field like:
```go
m.grpcBufferSize.Set(float64(connSize), "conn")
m.grpcBufferSize.Set(float64(streamSize), "stream")
```
##########
banyand/liaison/grpc/metrics.go:
##########
@@ -43,26 +43,50 @@ type metrics struct {
totalRegistryFinished meter.Counter
totalRegistryErr meter.Counter
totalRegistryLatency meter.Counter
+
+ memoryLoadSheddingRejections meter.Counter
+ grpcBufferSizeConn meter.Gauge
+ grpcBufferSizeStream meter.Gauge
+ memoryState meter.Gauge
}
func newMetrics(factory *observability.Factory) *metrics {
return &metrics{
- totalStarted: factory.NewCounter("total_started",
"group", "service", "method"),
- totalFinished: factory.NewCounter("total_finished",
"group", "service", "method"),
- totalErr: factory.NewCounter("total_err",
"group", "service", "method"),
- totalPanic: factory.NewCounter("total_panic"),
- totalLatency: factory.NewCounter("total_latency",
"group", "service", "method"),
- totalStreamStarted:
factory.NewCounter("total_stream_started", "service", "method"),
- totalStreamFinished:
factory.NewCounter("total_stream_finished", "service", "method"),
- totalStreamErr:
factory.NewCounter("total_stream_err", "service", "method"),
- totalStreamLatency:
factory.NewCounter("total_stream_latency", "service", "method"),
- totalStreamMsgReceived:
factory.NewCounter("total_stream_msg_received", "group", "service", "method"),
- totalStreamMsgReceivedErr:
factory.NewCounter("total_stream_msg_received_err", "group", "service",
"method"),
- totalStreamMsgSent:
factory.NewCounter("total_stream_msg_sent", "group", "service", "method"),
- totalStreamMsgSentErr:
factory.NewCounter("total_stream_msg_sent_err", "group", "service", "method"),
- totalRegistryStarted:
factory.NewCounter("total_registry_started", "group", "service", "method"),
- totalRegistryFinished:
factory.NewCounter("total_registry_finished", "group", "service", "method"),
- totalRegistryErr:
factory.NewCounter("total_registry_err", "group", "service", "method"),
- totalRegistryLatency:
factory.NewCounter("total_registry_latency", "group", "service", "method"),
+ totalStarted:
factory.NewCounter("total_started", "group", "service", "method"),
+ totalFinished:
factory.NewCounter("total_finished", "group", "service", "method"),
+ totalErr: factory.NewCounter("total_err",
"group", "service", "method"),
+ totalPanic: factory.NewCounter("total_panic"),
+ totalLatency:
factory.NewCounter("total_latency", "group", "service", "method"),
+ totalStreamStarted:
factory.NewCounter("total_stream_started", "service", "method"),
+ totalStreamFinished:
factory.NewCounter("total_stream_finished", "service", "method"),
+ totalStreamErr:
factory.NewCounter("total_stream_err", "service", "method"),
+ totalStreamLatency:
factory.NewCounter("total_stream_latency", "service", "method"),
+ totalStreamMsgReceived:
factory.NewCounter("total_stream_msg_received", "group", "service", "method"),
+ totalStreamMsgReceivedErr:
factory.NewCounter("total_stream_msg_received_err", "group", "service",
"method"),
+ totalStreamMsgSent:
factory.NewCounter("total_stream_msg_sent", "group", "service", "method"),
+ totalStreamMsgSentErr:
factory.NewCounter("total_stream_msg_sent_err", "group", "service", "method"),
+ totalRegistryStarted:
factory.NewCounter("total_registry_started", "group", "service", "method"),
+ totalRegistryFinished:
factory.NewCounter("total_registry_finished", "group", "service", "method"),
+ totalRegistryErr:
factory.NewCounter("total_registry_err", "group", "service", "method"),
+ totalRegistryLatency:
factory.NewCounter("total_registry_latency", "group", "service", "method"),
+ memoryLoadSheddingRejections:
factory.NewCounter("memory_load_shedding_rejections_total", "service"),
+ grpcBufferSizeConn:
factory.NewGauge("grpc_buffer_size_bytes", "type"),
+ grpcBufferSizeStream:
factory.NewGauge("grpc_buffer_size_bytes", "type"),
Review Comment:
Both `grpcBufferSizeConn` and `grpcBufferSizeStream` are using the same
metric name `"grpc_buffer_size_bytes"` but are initialized as separate gauge
instances. This creates duplicate metrics with the same name. They should
either:
1. Be the same gauge instance with different label values passed when
setting (e.g., `m.grpcBufferSize.Set(float64(connSize), "conn")`), or
2. Use different metric names (e.g., `"grpc_buffer_size_bytes_conn"` and
`"grpc_buffer_size_bytes_stream"`).
The current implementation in `updateBufferSizeMetrics` shows that option 1
is intended, so only one gauge should be created in `newMetrics`.
--
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]
