Add functionality to collect metrics from commit server for argocd #20412
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. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Just have a question for my understanding and a comment to simplify the number of places we need to modify when adding more config options.
EDIT: Just noticed it is in draft, I was curious about how the metrics were being added and reviewed it 😅 . Let me know when it is out of draft and I can approve again if it is not kept.
| NOTIFICATIONS_CONTROLLER_METRICS, | ||
| ) | ||
| ) | ||
| if commit_server_endpoint: | ||
| self.scraper_configs.append( | ||
| self.generate_config(commit_server_endpoint, COMMIT_SERVER_NAMESPACE, COMMIT_SERVER_METRICS) | ||
| ) |
There was a problem hiding this comment.
[Re: lines +44 to +94]
nit: wdyt about simplifying this with a config setup and a dynamic block?
def parse_config(self):
configs_per_endpoint = {
"app_controller_endpoint": {
"namespace": APP_CONTROLLER_NAMESPACE,
"metrics": APPLICATION_CONTROLLER_METRICS,
},
"appset_controller_endpoint": {
"namespace": APPSET_CONTROLLER_NAMESPACE,
"metrics": APPSET_CONTROLLER_METRICS,
},
"api_server_endpoint": {
"namespace": API_SERVER_NAMESPACE,
"metrics": API_SERVER_METRICS,
},
"repo_server_endpoint": {
"namespace": REPO_SERVER_NAMESPACE,
"metrics": REPO_SERVER_METRICS,
},
"notifications_controller_endpoint": {
"namespace": NOTIFICATIONS_CONTROLLER_NAMESPACE,
"metrics": NOTIFICATIONS_CONTROLLER_METRICS,
},
"commit_server_endpoint": {
"namespace": COMMIT_SERVER_NAMESPACE,
"metrics": COMMIT_SERVER_METRICS,
},
}
self.scraper_configs = [
self.generate_config(
endpoint,
configs_per_endpoint[endpoint]["namespace"],
configs_per_endpoint[endpoint]["metrics"],
)
for endpoint_str in configs_per_endpoint
if (endpoint := self.instance.get(endpoint_str))
]
if not self.scraper_configs:
expected_endpoints_str = "`, `".join(configs_per_endpoint.keys())
raise ConfigurationError(
f"Must specify at least one of the following: `{expected_endpoints_str}`"
)See this comment inline on Graphite.
| metric_name,metric_type,interval,unit_name,per_unit_name,description,orientation,integration,short_name,curated_metric,sample_tags |
There was a problem hiding this comment.
Just a question, is this change because there was a different metadata.csv format before?
| @@ -0,0 +1 @@ | |||
| Add functionality to collect metrics from commit server | |||
There was a problem hiding this comment.
nit: Forgot to mention this. Would it be good to add here a link to the ArgoDC commit server feature we are adding support for? Both for future team members that might want to know what this is as well as customers that read about the release where this feature was added.
What does this PR do?
Added functionality to collect metrics from another component that was introduced in argocd.