-
Notifications
You must be signed in to change notification settings - Fork 554
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
Add actor metrics #9294
Add actor metrics #9294
Conversation
Add counter for actorTask execution Add histogram to observe actorTask execution
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.
🚀 No blockers, but please check the comments
import io.prometheus.client.Counter; | ||
import io.prometheus.client.Histogram; | ||
|
||
public class ActorMetrics { |
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.
public class ActorMetrics { | |
final class ActorMetrics { |
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.
🔧 As a rule of thumb, I would generally keep the lowest visibility required if only to make sure when other classes depend on one (i.e. it must be made public) hopefully we think about it and what it means to couple them.
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.
I guess I shouldn't just copy old branches 😅 BTW why we still have no check for class needs to be final ? 🤔 I guess it is not easy to detect ? 🤔 Or what is the reason?
util/src/main/java/io/camunda/zeebe/util/sched/ActorMetrics.java
Outdated
Show resolved
Hide resolved
util/src/main/java/io/camunda/zeebe/util/sched/ActorMetrics.java
Outdated
Show resolved
Hide resolved
util/src/main/java/io/camunda/zeebe/util/sched/ActorThread.java
Outdated
Show resolved
Hide resolved
bors r+ |
Description
As discussed here https://camunda.slack.com/archives/C037RS2JHB8/p1651668160788749 add new actor metrics but no new panels for now.
Details:
Currently starting a benchmark to verify whether metrics are exported as expected.
I will create a separate PR for the atomix executors.
@npepinpe I'm not sure whether it fulfills all requirements for #9282 I will remove my assignment then.
Related issues
related #9282
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/1.3
) to the PR, in case that fails you need to create backports manually.Testing:
Documentation:
Please refer to our review guidelines.