feat(derived_code_mappings): Add metric for creating a repository and dry_run mode #86387
Conversation
… dry_run mode This will track when a repository is created and when any of the two metrics would have created a repository or code mapping (useful for Java's dry run).
github-actions
bot
added
the
Scope: Backend
| @@ -589,7 +589,7 @@ def get_path_from_module(module: str, abs_path: str) -> tuple[str, str]: | |||
| raise DoesNotFollowJavaPackageNamingConvention | |||
|
|
|||
| # If module has a dot, take everything before the last dot | |||
| # com.example.foo.Bar$InnerClass -> com/example/foo/ | |||
| # com.example.foo.Bar$InnerClass -> com/example/foo | |||
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #86387 +/- ##
===========================================
+ Coverage 42.19% 87.80% +45.60%
===========================================
Files 9719 9751 +32
Lines 550870 552568 +1698
Branches 21542 21542
===========================================
+ Hits 232463 485180 +252717
+ Misses 318058 67039 -251019
Partials 349 349 |
| metrics.incr( | ||
| key=f"{METRIC_PREFIX}.code_mapping.created", | ||
| tags={"platform": platform, "dry_run": dry_run}, | ||
| sample_rate=1.0, | ||
| ) |
There was a problem hiding this comment.
Shouldn't this be under the if? I get that you're not trying to gate it by dry_run, but shouldn't it obey the if repository is not None check on creation?
There was a problem hiding this comment.
UPDATE: Okay, I know you changed the logic around, but I'm still not sure I understand it. Shouldn't the code_mapping.created only fire when either:
- we're not in dry-run, we call
get_or_create, andcreatedcomes back true, or - we are in dry-run, so we don't call
get_or_create, but if we had called it, it would have come backcreated = True?
The first one works with the code above, but for the second one, don't we have to make sure the record doesn't yet exist before saying that created would have been true?
armenzg
force-pushed
the
dry_run/metric/derived_code_mapping/armenzg
branch
from
8f2922d to
9d61289
Compare
| }, | ||
| metrics.incr( | ||
| key=f"{METRIC_PREFIX}.code_mapping.created", | ||
| tags={"platform": platform, "dry_run": dry_run}, |
There was a problem hiding this comment.
We're assuming that in dry run mode, it would have created the record.
| project=project, stack_root=code_mapping.stacktrace_root | ||
| ) | ||
| if existing_code_mappings.exists(): | ||
| logger.warning("Investigate.", extra=extra) |
There was a problem hiding this comment.
This should never happen, thus, logging to Sentry.
This will track when a repository is created and when the metrics would have created a repository or a code mapping (useful for Java's dry run).