feat: metric for dropped messages in Shard Region buffer#32887
feat: metric for dropped messages in Shard Region buffer#32887sebastian-alfers merged 4 commits intosharding-telemetryfrom
Conversation
f44207d to
0fbb370
Compare
4e144a0 to
b0970f8
Compare
| /** | ||
| * Drop a mesage send to a Shard Region as the buffer is full | ||
| */ | ||
| def dropMessagesShardRegion(selfAddress: Address, self: ActorRef, typeName: String): Unit |
There was a problem hiding this comment.
Could the name reflect what the doc says better? shardRegionBufferOverflowMessageDrop is a mouthful but says what it does right away.
Also wondering about the imperative names here in general, is that style we have used for other instrumentation rather than facts that something happened? dropMessage vs messageDropped. The method here is reporting that it happened, rather than dropping anything.
There was a problem hiding this comment.
Looking at CircuitBreakerTelemetry we have onOpen, onClose. In projections Telemetry we have stopped, failed, beforeProcess, afterProcess (but also error rather than onError) so not quite consistent.
It-happened-naming seems more clear, to me, at least.
There was a problem hiding this comment.
The naming we use in cinnamon is messageDropped.
johanandren
left a comment
There was a problem hiding this comment.
Looks good but a naming question/discussion
...-sharding/src/main/scala/akka/cluster/sharding/internal/ClusterShardingInstrumentation.scala
Outdated
Show resolved
Hide resolved
| * Drop a message send to a Shard Region if the buffer is full. | ||
| */ | ||
| def shardRegionMessageDropped(selfAddress: Address, self: ActorRef, typeName: String): Unit | ||
| def messageDropped(selfAddress: Address, self: ActorRef, typeName: String): Unit |
There was a problem hiding this comment.
Another scaladoc to drop
409a0ab to
356fb3e
Compare
9c2e143 to
b0272ea
Compare
356fb3e to
5cde605
Compare
johanandren
left a comment
There was a problem hiding this comment.
LGTM but drop that scaladoc
No description provided.