-
Notifications
You must be signed in to change notification settings - Fork 810
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 allocations metrics #963
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Build Failed 😱 Build Id: c5aca92e-cc39-4d57-a560-5aff291d35c9 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Issue in |
Build Failed 😱 Build Id: c72a940e-9e15-4b60-b154-eb6cdb765116 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Hitting retry! |
Build Succeeded 👏 Build Id: f81b5185-8591-4b53-b5ff-ad8f6cb93574 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
@markmandel - have you had time to review this? Are you still hoping to get it in before the 0.12.0 RC? |
Build Succeeded 👏 Build Id: b80982f4-2e2e-46ad-b8a7-f69cd755796e The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
You can go ahead Robert, so I can start fixing if you find things, I created a structure to embed the tagging/record logic because I didn't wanted to add a lot of nested code in the current allocation implementation. I think that was a good idea but I would love feedback |
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.
Thanks for adding metrics for allocation and liveliness check.
cmd/allocator/main.go
Outdated
var err error | ||
lock := sync.Mutex{} | ||
// force a pod restart if the https server exits. | ||
health.AddLivenessCheck("allocator-https", func() error { |
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.
Can we instead having two place for liveliness check and healthz only use healthz and leverage wait group to end the process if ListenAndServeTLS returns?
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.
yes good idea, I'm doing it, way simpler :)
I only just woke up 😁- so if there is a chance to get this in before I do the RC release in a few hours, that would be awesome, I think. |
Thanks @pooneh-m, I'll look into it. |
@cyriltovena - was going to do a review, but looks like you have some outstanding review items from @pooneh-m , so I'll hold off until they are complete. |
2034569
to
6e3b5e8
Compare
Build Failed 😱 Build Id: 6c91761c-f24f-4c4c-b279-8c7d74367b46 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
@pooneh-m I fixed all your feedbacks, thanks ! |
Build Failed 😱 Build Id: 28b5e835-1d6d-4acf-90e5-c627a53090bd To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Oops, |
I'll check everything one more time. |
it got rollback by a rebase.... |
errr, something went wrong with the rebase.... I lost some new dashboard.... |
Build Failed 😱 Build Id: 4147cd83-c2af-4452-98d6-5884072f035f To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: 36203b24-b4f4-44c6-8558-2002bbd8b1df The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: a4ffeae0-f819-479e-a2b1-4cc24626c0fc The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
This adds gameserver allocations metrics with a dashboard. The metrics tracks allocations count, rates and latency per nodes, fleet and status.
I've also added everything required to handle multi cluster allocations but nothing used in the dashboard yet.
I've fixed some issues with kind cluster and also took the time to add metrics and healthcheck for the allocator service. (with a grafana dashboard)
Documentation has been updated.
Closes #144