-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(auto_source_config): Add a few more metrics #86598
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
Conversation
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #86598 +/- ##
==========================================
+ Coverage 87.73% 87.86% +0.13%
==========================================
Files 9863 9802 -61
Lines 558910 557037 -1873
Branches 22052 21640 -412
==========================================
- Hits 490363 489460 -903
+ Misses 68117 67206 -911
+ Partials 430 371 -59 |
def get_trees_for_org(self) -> dict[str, RepoTree]: | ||
trees = {} | ||
repositories = self._populate_repositories() | ||
with metrics.timer("integrations.source_code_management.populate_repositories.duration"): |
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.
This is the root of all slowdowns. I want to verify it.
] | ||
|
||
metrics.incr( | ||
"integrations.source_code_management.populate_repositories", |
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 want to calculate how often the cache is hit.
|
||
metrics.incr( | ||
"integrations.source_code_management.populate_trees", | ||
tags={"cached": use_cache, "integration": self.integration_name}, |
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.
Same here.
break | ||
with metrics.timer( | ||
f"{METRIC_PREFIX}.generate_code_mappings.duration", tags={"platform": platform} | ||
): |
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.
integration_id=organization_integration.integration_id, | ||
with metrics.timer( | ||
f"{METRIC_PREFIX}.create_configurations.duration", tags={"platform": platform} | ||
): |
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.
|
||
metrics.incr( | ||
"integrations.source_code_management.populate_repositories", | ||
tags={"cached": use_cache, "integration": self.integration_name}, |
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.
The cardinality of self.integration_name
is pretty low, right? It'll just be the SCM?
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.
Correct
use_cache = False | ||
if not repositories: | ||
use_cache = True |
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.
Is this inverted? If repositories is present didn't we use cache?
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.
At least one of us is paying attention! 😅 Fixed now
Add the following perf metrics:
Add the following counting metrics: