Skip to content
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

feat(replays): add replay_id uuid processors to config #5791

Merged
merged 10 commits into from
Apr 18, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ schema:
args: { schema_modifiers: [nullable], size: 64 },
},
{ name: deleted, type: UInt, args: { size: 8 } },
{ name: trace_id, type: UUID, args: { schema_modifiers: [nullable] } }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we adding trace_id? Isn't this pr a followup to the migration where you added replay_id?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you also have to add a column to the entities/discover.yaml file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the catch, should have been replay_id

]
local_table_name: discover_local
dist_table_name: discover_dist
Expand Down Expand Up @@ -115,9 +116,7 @@ query_processors:
column_name: tags
- processor: UUIDColumnProcessor
args:
columns: !!set
trace_id: null
event_id: null
columns: [event_id, trace_id, replay_id]
- processor: HexIntColumnProcessor
args:
columns: !!set
Expand Down
16 changes: 1 addition & 15 deletions snuba/datasets/configuration/events/storages/errors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -268,17 +268,6 @@ allocation_policies:
- referrer
default_config_overrides:
is_enforced: 0
- name: CrossOrgQueryAllocationPolicy
args:
required_tenant_types:
- referrer
default_config_overrides:
is_enforced: 0
is_active: 0
cross_org_referrer_limits:
getsentry.tasks.backfill_grouping_records:
max_threads: 2
concurrent_limit: 4

query_processors:
- processor: UniqInSelectAndHavingProcessor
Expand All @@ -303,10 +292,7 @@ query_processors:
- processor: UserColumnProcessor
- processor: UUIDColumnProcessor
args:
columns: !!set
event_id: null
primary_hash: null
trace_id: null
columns: [event_id, primary_hash, trace_id, replay_id]
- processor: HexIntColumnProcessor
args:
columns: !!set
Expand Down
16 changes: 1 addition & 15 deletions snuba/datasets/configuration/events/storages/errors_ro.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -266,17 +266,6 @@ allocation_policies:
- referrer
default_config_overrides:
is_enforced: 0
- name: CrossOrgQueryAllocationPolicy
args:
required_tenant_types:
- referrer
default_config_overrides:
is_enforced: 0
is_active: 0
cross_org_referrer_limits:
getsentry.tasks.backfill_grouping_records:
max_threads: 2
concurrent_limit: 4


query_processors:
Expand All @@ -302,10 +291,7 @@ query_processors:
- processor: UserColumnProcessor
- processor: UUIDColumnProcessor
args:
columns: !!set
primary_hash: null
event_id: null
trace_id: null
columns: [event_id, primary_hash, trace_id, replay_id]
- processor: HexIntColumnProcessor
args:
columns: !!set
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ schema:
},
{ name: app_start_type, type: String },
{ name: profile_id, type: UUID, args: { schema_modifiers: [nullable] } },
{ name: replay_id, type: UUID, args: { schema_modifiers: [nullable] } },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed for transactions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you also need to add a replay_id column like this to discover.yaml?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we want to query on this column which exists in the transactions dataset

]
local_table_name: transactions_local
dist_table_name: transactions_dist
Expand Down Expand Up @@ -202,7 +203,7 @@ query_processors:
trace.span_id: span_id
- processor: UUIDColumnProcessor
args:
columns: [event_id, trace_id, profile_id]
columns: [event_id, trace_id, profile_id, replay_id]
- processor: HexIntColumnProcessor
args:
columns: [span_id]
Expand Down
3 changes: 1 addition & 2 deletions snuba/migrations/group_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ def get_migrations(self) -> Sequence[str]:
"0005_discover_fix_transaction_name",
"0006_discover_add_trace_id",
"0007_discover_add_span_id",
"0008_discover_add_replay_id",
]


Expand Down Expand Up @@ -330,8 +331,6 @@ def get_migrations(self) -> Sequence[str]:
"0037_add_record_meta_column_sets",
"0038_add_record_meta_column_distributions",
"0039_add_record_meta_column_gauges",
"0040_remove_counters_meta_tables",
"0041_adjust_partitioning_meta_tables",
]


Expand Down
56 changes: 56 additions & 0 deletions snuba/snuba_migrations/discover/0008_discover_add_replay_id.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
from typing import Sequence

from snuba.clickhouse.columns import UUID, Column
from snuba.clusters.storage_sets import StorageSetKey
from snuba.migrations import migration, operations
from snuba.migrations.columns import MigrationModifiers as Modifiers
from snuba.migrations.operations import OperationTarget


class Migration(migration.ClickhouseNodeMigration):
"""
Add replay_id to merge table
"""

blocking = False

def forwards_ops(self) -> Sequence[operations.SqlOperation]:
return [
operations.AddColumn(
storage_set=StorageSetKey.DISCOVER,
table_name=table_name,
column=Column("replay_id", UUID(Modifiers(nullable=True))),
after="span_id",
target=target,
)
for table_name, target in [
("discover_local", OperationTarget.LOCAL),
("discover_dist", OperationTarget.DISTRIBUTED),
]
]

def backwards_ops(self) -> Sequence[operations.SqlOperation]:
return [
operations.DropColumn(
storage_set=StorageSetKey.DISCOVER,
table_name=table_name,
column_name="replay_id",
target=target,
)
for table_name, target in [
("discover_dist", OperationTarget.DISTRIBUTED),
("discover_local", OperationTarget.LOCAL),
]
]

def forwards_local(self) -> Sequence[operations.SqlOperation]:
return self.forwards_ops()

def backwards_local(self) -> Sequence[operations.SqlOperation]:
return self.backwards_ops()

def forwards_dist(self) -> Sequence[operations.SqlOperation]:
return self.forwards_ops()

def backwards_dist(self) -> Sequence[operations.SqlOperation]:
return self.backwards_ops()
Loading