-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(reports): Cache per-project weekly report metrics #116739
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
Changes from 20 commits
8984411
e120c09
a29ab0d
8f90afe
103b522
419adb7
1bab33b
b6f5ee3
c076d81
0e4314c
8edfe8b
20fd594
fa2b08c
a49272d
cf72ce3
2d0b585
59edfc4
94689e7
bde2262
732aa53
8c2ece0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from datetime import timedelta | ||
| from typing import Any | ||
|
|
||
| from django.conf import settings | ||
| from sentry_redis_tools.clients import RedisCluster, StrictRedis | ||
|
|
||
| from sentry.utils import json, metrics, redis | ||
|
|
||
| CACHE_TTL = timedelta(days=10) | ||
| KEY_PREFIX = "wr:proj_metrics" | ||
|
|
||
|
|
||
| def _make_cache_key(org_id: int, project_id: int) -> str: | ||
| return f"{KEY_PREFIX}:{org_id}:{project_id}" | ||
|
|
||
|
|
||
| def _get_redis_client() -> RedisCluster[str] | StrictRedis[str]: | ||
| return redis.redis_clusters.get(settings.SENTRY_WEEKLY_REPORTS_REDIS_CLUSTER) | ||
|
|
||
|
|
||
| def cache_project_metrics( | ||
| org_id: int, | ||
| project_metrics: dict[int, dict[str, int]], | ||
| ) -> None: | ||
| client = _get_redis_client() | ||
| pipeline = client.pipeline() | ||
| ttl_seconds = int(CACHE_TTL.total_seconds()) | ||
|
|
||
| for project_id, values in project_metrics.items(): | ||
| key = _make_cache_key(org_id, project_id) | ||
| pipeline.set(key, json.dumps(values), ex=ttl_seconds) | ||
|
|
||
| pipeline.execute() | ||
|
|
||
|
|
||
| def read_project_metrics( | ||
| org_id: int, | ||
| project_ids: list[int], | ||
| ) -> dict[int, dict[str, Any]]: | ||
| if not project_ids: | ||
| return {} | ||
|
|
||
| client = _get_redis_client() | ||
| pipeline = client.pipeline() | ||
|
|
||
| for project_id in project_ids: | ||
| pipeline.get(_make_cache_key(org_id, project_id)) | ||
|
|
||
| results = pipeline.execute() | ||
|
|
||
| result_map: dict[int, dict[str, Any]] = {} | ||
| for i, project_id in enumerate(project_ids): | ||
| raw = results[i] | ||
| if raw is None: | ||
| metrics.incr("weekly_report.cache.miss") | ||
| else: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we record cache hits as well? another way you could do this would be to have a single metric called
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be part of your next PR btw, doesn't have to block this one |
||
| result_map[project_id] = json.loads(raw) | ||
|
|
||
| return result_map | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| from sentry.tasks.summaries.weekly_report_cache import ( | ||
| _make_cache_key, | ||
| cache_project_metrics, | ||
| read_project_metrics, | ||
| ) | ||
| from sentry.testutils.cases import TestCase | ||
|
|
||
|
|
||
| class WeeklyReportCacheTest(TestCase): | ||
| def test_make_cache_key(self) -> None: | ||
| key = _make_cache_key(org_id=1, project_id=2) | ||
| assert key == "wr:proj_metrics:1:2" | ||
|
|
||
| def test_write_and_read(self) -> None: | ||
| org_id = self.organization.id | ||
| project = self.create_project(organization=self.organization) | ||
|
|
||
| cache_project_metrics(org_id, {project.id: {"e": 500, "t": 3000}}) | ||
|
|
||
| result = read_project_metrics(org_id=org_id, project_ids=[project.id]) | ||
|
|
||
| assert result[project.id] == {"e": 500, "t": 3000} | ||
|
|
||
| def test_read_empty_cache(self) -> None: | ||
| result = read_project_metrics(org_id=self.organization.id, project_ids=[999]) | ||
|
|
||
| assert result == {} | ||
|
|
||
| def test_read_empty_project_ids(self) -> None: | ||
| result = read_project_metrics(org_id=self.organization.id, project_ids=[]) | ||
|
|
||
| assert result == {} | ||
|
|
||
| def test_write_empty_metrics_is_noop(self) -> None: | ||
| cache_project_metrics(self.organization.id, {}) | ||
|
|
||
| def test_multiple_projects(self) -> None: | ||
| org_id = self.organization.id | ||
| p1 = self.create_project(organization=self.organization) | ||
| p2 = self.create_project(organization=self.organization) | ||
|
|
||
| cache_project_metrics( | ||
| org_id, | ||
| { | ||
| p1.id: {"e": 100, "t": 200}, | ||
| p2.id: {"e": 300, "t": 400}, | ||
| }, | ||
| ) | ||
|
|
||
| result = read_project_metrics(org_id=org_id, project_ids=[p1.id, p2.id]) | ||
|
|
||
| assert result[p1.id] == {"e": 100, "t": 200} | ||
| assert result[p2.id] == {"e": 300, "t": 400} |
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 just set this as an int, e.g.
10 * 24 * 60 * 60