-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[PLINT-542] Add machines metrics #19285
base: sarah/add-octopus-integration
Are you sure you want to change the base?
[PLINT-542] Add machines metrics #19285
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
] | ||
machine_tags += roles | ||
self.gauge("machine.count", 1, tags=self._base_tags + machine_tags) | ||
self.gauge("machine.is_healthy", is_healthy, tags=self._base_tags + machine_tags) |
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.
What is the purpose of also reporting it as a metric if we already report it as a tag?
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.
It's easier to monitor on, and the tag gives more specific/detailed information
roles = machine.get("Roles", []) | ||
health_status = machine.get("HealthStatus", None) | ||
is_healthy = health_status.startswith("Healthy") | ||
machine_tags = [ |
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.
What do you think about adding OperatingSystem
as a machine_os
tag?
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.
Sure!
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.
Added!
"Uri": "hostname-test:10933/", | ||
"IsDisabled": false, | ||
"MachinePolicyId": "MachinePolicies-1", | ||
"HealthStatus": "Healthy", |
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.
We're only testing HealthStatus
with Healthy
value
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.
Added multiple health status values!
What does this PR do?
Add support for collecting metrics about machines and deployment targets
Motivation
Review checklist (to be filled by reviewers)
qa/skip-qa
label if the PR doesn't need to be tested during QA.backport/<branch-name>
label to the PR and it will automatically open a backport PR once this one is merged