feat: add org generic counters [TET-695] #3708
Conversation
andriisoldatenko
force-pushed
the
andrii/add-org-generic-counter
branch
from
7f5302a to
d6f927e
Compare
| @@ -0,0 +1,57 @@ | |||
| version: v1 | |||
| kind: entity | |||
| name: org_metrics_counters | |||
There was a problem hiding this comment.
I would make this generic_org_metrics_counters, otherwise the name will collide with the existing entity.
| required_time_column: timestamp | ||
|
|
||
| storages: | ||
| - storage: org_metrics_counters |
There was a problem hiding this comment.
I mentioned this in Slack, but this storage is referencing the release health metrics counters. You will need a new storage in generic_metrics/storages/ that will be very similar to this one, but with a new name. Then you can reference that storage here.
| name: generic_org_metrics_counters | ||
| storage: | ||
| key: generic_org_metrics_counters | ||
| set_key: generic_org_metrics_counters |
There was a problem hiding this comment.
This should be generic_metrics_counters. The set_key is different from the key, and specifies which cluster the storage is referencing. You don't need a new cluster.
andriisoldatenko
changed the title
There was a problem hiding this comment.
This all looks good to me, but I think you should add a test here so you can do an E2E check of the new entity: https://github.com/getsentry/snuba/blob/master/tests/test_generic_metrics_api.py
do you want me to add |
andriisoldatenko
force-pushed
the
andrii/add-org-generic-counter
branch
from
f8bd089 to
ba623a4
Compare
| { name: project_id, type: UInt, args: { size: 64 } }, | ||
| { name: metric_id, type: UInt, args: { size: 64 } }, | ||
| { name: granularity, type: UInt, args: { size: 8 } }, | ||
| { name: timestamp, type: DateTime }, |
There was a problem hiding this comment.
is this missing the actual count column?
or do you just not need it?
There was a problem hiding this comment.
I think we need it here, @evanh @untitaker wdyt?
I'm not very familiar with how this will work under the hood.
There was a problem hiding this comment.
only i can say it works with and without this block. i just did quick test.
There was a problem hiding this comment.
I see, yeah I prefer to implement as minimally as we need. So later we can extend.
#42939) This PR implements prioritize by project bias. In detail: We run celery task every 24 at 8:00AM (UTC randomly selected) for every ORG (we call it *prioritise by project snuba query* ) and all projects inside this org, and for a given combination of org and projects run an adjustment model to recalculate sample rates if necessary. Then we cache sample rate using redis cluster -> `SENTRY_DYNAMIC_SAMPLING_RULES_REDIS_CLUSTER` using this pattern for key: `f"ds::o:{org_id}:p:{project_id}:prioritise_projects"`. When relay fetches `projectconfig` endpoint we run `generate_rules` functions to generate all dynamic sampling biases, so and we check if we have adjusted sample rate for this project in the cache, so we apply it as **uniform bias**, otherwise we use default one. Regarding *prioritize by project snuba query* is cross org snuba query that utilizes a new generic counter metric, which was introduced in [relay]( getsentry/relay#1734) `c:transactions/count_per_root_project@none`. TODO: - [x] Provision infrastructure to run clickhouse clusters for the counters tables. This is primarily dependent on ops - [x] Start running the snuba consumers to read and write to the counters table. SnS can work on this - [x] Add unit-tests; - [x] Update snuba query using new metric - [x] Hide behind feature flag related PRs: - Implement new metric in relay: getsentry/relay#1734 - Add org generic counters [TET-695] getsentry/snuba#3708 - Introduce new storages for counters in snuba getsentry/snuba#3679 - Add feature flag: https://github.com/getsentry/getsentry/pull/9323 - Add cross organization methods for the string indexer #45076 #45076 [TET-695]: https://getsentry.atlassian.net/browse/TET-695?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com> Co-authored-by: Nar Saynorath <[email protected]>
This PR adds the ability to query cross org query for the generic metric counter.