-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
514 - bring back mailbox size metric #514
Conversation
ff3ebfe
to
cdd3365
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@skipper1982 I think I have an idea on how to use an async instrument for this metric (alternative approach). LMKWDYT. :)
|
||
import java.util.Objects; | ||
|
||
public class MailboxEnqueueAdvice { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a method in ActorCell
:
/**
* If the actor isLocal, returns the number of "user messages" currently queued,
* which may be a costly operation, 0 otherwise.
*/
def numberOfMessages: Int
We could (maybe?) pass a reference to that field every time an actor is created.
object LocalActorRefProviderAdvice {
@OnMethodExit
def actorOf(@Return ref: ActorRef, @Argument(0) system: ActorSystem): Unit =
??? // get the reference, the path and create the instrument for the actorRef (that contains the actorcell, which has the numberOfMessages method).
}
that advice could be attached to:
private val localActorRefProviderInstrumentation: AgentInstrumentation =
instrument("akka.actor.LocalActorRefProvider").visit(LocalActorRefProviderAdvice, "actorOf")
(taken from v0.7.0) Not sure if it's enough to instrument the localActorRefProvider
everytime we create a new actor maybe we can setup an async instrument that would periodically poll for the value from numberOfMessages
, thus taking the mailbox size. Gauge seems a good fit. Not sure about the performance of this (probably awful, but maybe not?).
@skipper1982 lmk what do you think. I realize this can be unclear so let's have a call if needed. :)
cdd3365
to
898c525
Compare
898c525
to
d6c797e
Compare
da090cc
to
2d45281
Compare
@adamgadomski I created an issue for a potential improvement of this metric: #615 For now merging as is. |
2d45281
to
76a8877
Compare
Closes #514
Summary:
(please input the summary)