feat(uptime): storage configuration and and consumer #6697

JoshFerge · 2024-12-20T18:30:58Z

No description provided.

github-actions · 2024-12-20T18:31:40Z

This PR has a migration; here is the generated SQL for ./snuba/migrations/groups.py ()

-- start migrations

-- forward migration events_analytics_platform : 0020_uptime_monitors_init
Local op: CREATE TABLE IF NOT EXISTS uptime_monitor_checks_local (organization_id UInt64, project_id UInt64, environment LowCardinality(Nullable(String)), uptime_subscription_id UUID, uptime_check_id UUID, scheduled_check_time DateTime64(3), timestamp DateTime64(3), duration_ms UInt64, region_slug LowCardinality(String), check_status LowCardinality(String), check_status_reason LowCardinality(String), http_status_code Nullable(UInt16), trace_id UUID, retention_days UInt16) ENGINE ReplicatedReplacingMergeTree('/clickhouse/tables/events_analytics_platform/{shard}/default/uptime_monitor_checks_local', '{replica}') ORDER BY (organization_id, project_id, toDateTime(timestamp), trace_id, uptime_check_id) PARTITION BY (retention_days, toMonday(timestamp)) TTL toDateTime(timestamp) + toIntervalDay(retention_days) SETTINGS index_granularity=8192;
Distributed op: CREATE TABLE IF NOT EXISTS uptime_monitor_checks_dist (organization_id UInt64, project_id UInt64, environment LowCardinality(Nullable(String)), uptime_subscription_id UUID, uptime_check_id UUID, scheduled_check_time DateTime64(3), timestamp DateTime64(3), duration_ms UInt64, region_slug LowCardinality(String), check_status LowCardinality(String), check_status_reason LowCardinality(String), http_status_code Nullable(UInt16), trace_id UUID, retention_days UInt16) ENGINE Distributed(`cluster_one_sh`, default, uptime_monitor_checks_local, cityHash64(reinterpretAsUInt128(trace_id)));
-- end forward migration events_analytics_platform : 0020_uptime_monitors_init




-- backward migration events_analytics_platform : 0020_uptime_monitors_init
Distributed op: DROP TABLE IF EXISTS uptime_monitor_checks_dist;
Local op: DROP TABLE IF EXISTS uptime_monitor_checks_local;
-- end backward migration events_analytics_platform : 0020_uptime_monitors_init

codecov · 2024-12-20T19:01:40Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
2636	1	2635	6

View the top 1 failed tests by shortest run time

tests.utils.streams.test_topics::test_valid_topics

Stack Traces | 0.128s run time

Traceback (most recent call last):
  File ".../local/lib/python3.11....../site-packages/sentry_kafka_schemas/sentry_kafka_schemas.py", line 80, in get_topic
    with open(topic_path) as f:
         ^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '.../local/lib/python3.11.../sentry_kafka_schemas/topics/snuba-dead-letter-uptime-results.yaml'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File ".../utils/streams/test_topics.py", line 10, in test_valid_topics
    sentry_kafka_schemas.get_topic(
  File ".../local/lib/python3.11....../site-packages/sentry_kafka_schemas/sentry_kafka_schemas.py", line 83, in get_topic
    raise SchemaNotFound
sentry_kafka_schemas.sentry_kafka_schemas.SchemaNotFound

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

JoshFerge · 2024-12-20T22:53:40Z

depends on getsentry/sentry-kafka-schemas#359 being merged and us bumping the kafka schema

JoshFerge · 2025-01-06T21:26:17Z

needs #6717 but otherwise should be good to go

needed for #6697

rust_snuba/src/processors/uptime_monitor_checks.rs

phacops · 2025-01-07T01:43:14Z

rust_snuba/src/processors/uptime_monitor_checks.rs

+#[serde(rename_all = "snake_case")]
+pub struct CheckStatusReason {
+    /// The type of the status reason
+    pub r#type: String,


I'm sure it works but damn that's ugly. What happened to naming it ty and putting an #[serde(rename = "type")]...

Also, it looks like it's a very static reason, we could benefit from having it be a 'a &str instead of String.

it seems like i'd have to introduce a bunch of lifetime stuff in order for this to work, is this really necessary?

Not that big of a deal, saves on memory. We do it a lot in Relay too.

https://github.com/getsentry/relay/blob/master/relay-server/src/services/store.rs#L1255

went ahead and converted all Strings to &strs for all the strings! let me know if what i did makes sense, not super familiar with lifetimes in rust.

rust_snuba/src/processors/uptime_monitor_checks.rs

volokluev · 2025-01-08T00:00:25Z

snuba/datasets/configuration/events_analytics_platform/storages/uptime_monitor_checks.yaml

+    [
+      { name: organization_id, type: UInt, args: { size: 64 } },
+      { name: project_id, type: UInt, args: { size: 64 } },
+      { name: environment, type: String, args: { schema_modifiers: [nullable, low_cardinality] } },


this should not be low cardinality, it's set by a customer, you can't guarantee it

environment is set to low cardinality on some other datasets, was just copying based on that. i can remove it if you'd like though

snuba/snuba/datasets/configuration/issues/entities/search_issues.yaml

Line 25 in 4c7d8d9

{ name: environment, type: String, args: { schema_modifiers: [ low_cardinality, nullable ] } },

snuba/snuba/snuba_migrations/events/0011_rebuild_errors.py

Line 24 in 4c7d8d9

Column("environment", String(Modifiers(nullable=True, low_cardinality=True))),

phacops · 2025-01-08T03:13:05Z

snuba/clusters/storage_sets.py

@@ -20,6 +20,7 @@
    "EVENTS_ANALYTICS_PLATFORM": "events_analytics_platform",
    "GROUP_ATTRIBUTES": "group_attributes",
    "PROFILE_CHUNKS": "profile_chunks",
+    "UPTIME_RESULTS": "uptime_monitor_checks",


You don't need this. The migration belongs to the events_analytics_platform storage set already. That also means removing migration related changes in this PR (in group_loader.py, groups.py and the directory in snuba_migrations).

I believe it hasn't been run because it's prefixed with 0020_ when the previous one is 0018_, and yes, that's my fault since I reverted 0019_, but as soon as you fixed that, the migration will run everywhere the events_analytics_platform is available.

i'll update the migration in a separate PR! thanks for the callout.

i've also removed the migration related changes as specified. thanks.

phacops · 2025-01-08T03:14:05Z

snuba/datasets/configuration/events_analytics_platform/storages/uptime_monitor_checks.yaml

+name: uptime_monitor_checks
+storage:
+  key: uptime_monitor_checks
+  set_key: uptime_monitor_checks


I think it should belong to the events_analytics_platform since migrations are also link to the events_analytics_platform storage set.

I suppose it's not a MUST but since the migrations are already in the events_analytics_platform set, I'd keep things together.

i've gone ahead and removed the errant uptime_monitor_checks config stuff.

…82513) part of getsentry/team-uptime#23 requires getsentry/sentry-kafka-schemas#359 and getsentry/snuba#6697

renames the migration as 19 was reverted in the EAP dataset. #6697 (comment)

…82513) part of getsentry/team-uptime#23 requires getsentry/sentry-kafka-schemas#359 and getsentry/snuba#6697

JoshFerge added 20 commits December 20, 2024 11:00

feat(uptime): add initial table migration

13cc0c5

init

d4ffd39

rename uptime_monitor_checks

f9d51bd

adjust schema

e345a58

fix up

c443ab3

fix rust consumer

900d889

tweaks

cd0de05

fix dlq test

6947439

fix unused

cb7d8dc

move storage to eap and remove entity

5c104fe

fix partition key to include retention days

f7bd79f

update sorting key

2c4ce34

remove migration

02952e2

fix up storage

572daf4

fix rust test

fec7ed0

fix rust test

cd96798

fix consumer

6dc60ed

snapshot

0452bfb

fix snapshot

d5533bc

remove unused

3a01f2b

github-actions bot added the migrations label Dec 20, 2024

use new topic

6493b24

JoshFerge mentioned this pull request Dec 20, 2024

feat(uptime): produce to new snuba topic in check results consumer getsentry/sentry#82513

Merged

JoshFerge mentioned this pull request Jan 6, 2025

feat(deps): bump sentry-kafka-schemas #6717

Merged

JoshFerge marked this pull request as ready for review January 6, 2025 21:26

JoshFerge requested a review from a team as a code owner January 6, 2025 21:26

JoshFerge requested a review from phacops January 6, 2025 21:26

Base automatically changed from jferg/uptime-checks-1 to master January 6, 2025 22:24

JoshFerge added a commit that referenced this pull request Jan 6, 2025

feat(deps): bump sentry-kafka-schemas (#6717)

f708511

needed for #6697

JoshFerge added 3 commits January 6, 2025 18:26

Merge branch 'master' into jferg/uptime-checks-2

522fea1

Update 0020_uptime_monitors_init.py

5224057

fix kafka topic

7d3dd44

phacops reviewed Jan 7, 2025

View reviewed changes

JoshFerge added 4 commits January 7, 2025 10:33

pr feedback

a066a79

fix snapshots

f46dd4d

fix schema

b984001

remove dlq for now

4c7d8d9

JoshFerge requested a review from phacops January 7, 2025 19:48

volokluev reviewed Jan 8, 2025

View reviewed changes

phacops reviewed Jan 8, 2025

View reviewed changes

JoshFerge added 3 commits January 8, 2025 13:18

use str instead of String

b957c7b

fix storage configuration

9529097

remove more uneccessary config

0486ddf

github-actions bot removed the migrations label Jan 8, 2025

JoshFerge added a commit to getsentry/sentry that referenced this pull request Jan 8, 2025

feat(uptime): produce to new snuba topic in check results consumer (#…

796238a

…82513) part of getsentry/team-uptime#23 requires getsentry/sentry-kafka-schemas#359 and getsentry/snuba#6697

JoshFerge mentioned this pull request Jan 8, 2025

fix(uptime): rename snuba migration to correct number #6728

Merged

JoshFerge requested a review from phacops January 8, 2025 19:13

phacops approved these changes Jan 8, 2025

View reviewed changes

JoshFerge merged commit e3cb9aa into master Jan 8, 2025
31 checks passed

JoshFerge deleted the jferg/uptime-checks-2 branch January 8, 2025 19:17

JoshFerge added a commit that referenced this pull request Jan 8, 2025

fix(uptime): rename snuba migration to correct number (#6728)

0932e89

renames the migration as 19 was reverted in the EAP dataset. #6697 (comment)

andrewshie-sentry pushed a commit to getsentry/sentry that referenced this pull request Jan 22, 2025

feat(uptime): produce to new snuba topic in check results consumer (#…

4a7b532

…82513) part of getsentry/team-uptime#23 requires getsentry/sentry-kafka-schemas#359 and getsentry/snuba#6697

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(uptime): storage configuration and and consumer #6697

feat(uptime): storage configuration and and consumer #6697

JoshFerge commented Dec 20, 2024

github-actions bot commented Dec 20, 2024 •

edited

Loading

codecov bot commented Dec 20, 2024 •

edited

Loading

JoshFerge commented Dec 20, 2024

JoshFerge commented Jan 6, 2025

phacops Jan 7, 2025

phacops Jan 7, 2025

JoshFerge Jan 7, 2025

phacops Jan 8, 2025

JoshFerge Jan 8, 2025

volokluev Jan 8, 2025

JoshFerge Jan 8, 2025 •

edited

Loading

phacops Jan 8, 2025

JoshFerge Jan 8, 2025

JoshFerge Jan 8, 2025

phacops Jan 8, 2025

JoshFerge Jan 8, 2025

feat(uptime): storage configuration and and consumer #6697

feat(uptime): storage configuration and and consumer #6697

Conversation

JoshFerge commented Dec 20, 2024

github-actions bot commented Dec 20, 2024 • edited Loading

codecov bot commented Dec 20, 2024 • edited Loading

❌ 1 Tests Failed:

JoshFerge commented Dec 20, 2024

JoshFerge commented Jan 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshFerge Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 20, 2024 •

edited

Loading

codecov bot commented Dec 20, 2024 •

edited

Loading

JoshFerge Jan 8, 2025 •

edited

Loading