Skip to content

Commit

Permalink
fix(validation): EntityContainsColumnValidator wasn't actually enabled (
Browse files Browse the repository at this point in the history
#4399)

* it actually is applied

* time column on all entity

* reset sentry dsn

* dupilcate column

* events

* undo unecessary ones

* defaultnone

* remove print

* add more mappers

* duplicate column again

* update tests

* add use_case_id and granularity to generic_metrics entity and fix test

* fix test query tests with real column names

* fix test_format_expressions

* fix more tests

* fix tests and add granularity and use_case_id to metrics entities

* fix more tests

* fix tests

* add missing roup_raw to spans storage and entity

* fix test for clickhouse v23

* override entity column validator in tests

* add typing

---------

Co-authored-by: Enoch Tang <enoch.tang@sentry.io>
  • Loading branch information
rahul-kumar-saini and enochtangg committed Nov 21, 2023
1 parent 200f361 commit 3c3e7ca
Show file tree
Hide file tree
Showing 49 changed files with 746 additions and 532 deletions.
13 changes: 5 additions & 8 deletions snuba/datasets/configuration/discover/entities/discover.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ kind: entity
name: discover
schema:
[
{ name: event_id, type: FixedString, args: { length: 32 }},
{ name: event_id, type: FixedString, args: { length: 32 } },
{ name: project_id, type: UInt, args: { size: 64 } },
{ name: type, type: String, args: { schema_modifiers: [nullable] } },
{ name: timestamp, type: DateTime },
{ name: time, type: DateTime },
{ name: platform, type: String, args: { schema_modifiers: [nullable] } },
{ name: environment, type: String, args: { schema_modifiers: [nullable] } },
{ name: release, type: String, args: { schema_modifiers: [nullable] } },
Expand Down Expand Up @@ -254,11 +255,7 @@ schema:
type: String,
args: { schema_modifiers: [nullable] },
},
{
name: profile_id,
type: UUID,
args: { schema_modifiers: [nullable] },
},
{ name: profile_id, type: UUID, args: { schema_modifiers: [nullable] } },
{
name: replay_id,
type: UUID,
Expand All @@ -267,12 +264,12 @@ schema:
{
name: trace_sampled,
type: UInt,
args: { schema_modifiers: [ nullable ], size: 8 },
args: { schema_modifiers: [nullable], size: 8 },
},
{
name: num_processing_errors,
type: UInt,
args: { schema_modifiers: [ nullable ], size: 64 },
args: { schema_modifiers: [nullable], size: 64 },
},
]
required_time_column: timestamp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ schema:
[
{ name: project_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: time, type: DateTime },
{ name: rtime, type: DateTime },
{ name: event_id, type: UUID },
{ name: platform, type: String },
{ name: environment, type: String, args: { schema_modifiers: [nullable] } },
Expand Down Expand Up @@ -195,12 +197,12 @@ schema:
{
name: trace_sampled,
type: UInt,
args: { schema_modifiers: [ nullable ], size: 8 },
args: { schema_modifiers: [nullable], size: 8 },
},
{
name: num_processing_errors,
type: UInt,
args: { schema_modifiers: [ nullable ], size: 64 },
args: { schema_modifiers: [nullable], size: 64 },
},
{ name: replay_id, type: UUID, args: { schema_modifiers: [nullable] } },
]
Expand Down Expand Up @@ -538,7 +540,6 @@ subscription_validators:
- orderby
required_time_column: timestamp


join_relationships:
grouped:
rhs_entity: groupedmessage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,15 @@ schema:
{ name: title, type: String, args: { schema_modifiers: [readonly] } },
{ name: transaction_source, type: String },
{ name: timestamp, type: DateTime, args: { schema_modifiers: [readonly] } },
{ name: time, type: DateTime, args: { schema_modifiers: [readonly] } },
{
name: group_ids,
type: Array,
args: { inner_type: { type: UInt, args: { size: 64 } } },
},
{ name: app_start_type, type: String },
{ name: profile_id, type: UUID, args: {schema_modifiers: [nullable]}},
{ name: replay_id, type: UUID, args: {schema_modifiers: [nullable]}}
{ name: profile_id, type: UUID, args: { schema_modifiers: [nullable] } },
{ name: replay_id, type: UUID, args: { schema_modifiers: [nullable] } }
]
required_time_column: finish_ts
storages:
Expand Down Expand Up @@ -354,7 +355,7 @@ subscription_validators:
{
max_allowed_aggregations: 1,
disallowed_aggregations: [groupby, having, orderby],
required_time_column: finish_ts
required_time_column: finish_ts,
},
},
]
8 changes: 5 additions & 3 deletions snuba/datasets/configuration/entity_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,13 @@ def _build_join_relationships(config: dict[str, Any]) -> dict[str, JoinRelations
return relationships


def _build_validation_mode(mode: str | None) -> ColumnValidationMode:
def _build_validation_mode(mode: str | None) -> ColumnValidationMode | None:
if not mode:
return ColumnValidationMode.DO_NOTHING
return None

if mode == "warn":
if mode == "do_nothing":
return ColumnValidationMode.DO_NOTHING
elif mode == "warn":
return ColumnValidationMode.WARN
elif mode == "error":
return ColumnValidationMode.ERROR
Expand Down
7 changes: 4 additions & 3 deletions snuba/datasets/configuration/events/entities/events.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ schema:
[
{ name: project_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: time, type: DateTime },
{ name: rtime, type: DateTime },
{ name: event_id, type: UUID },
{ name: platform, type: String },
{ name: environment, type: String, args: { schema_modifiers: [nullable] } },
Expand Down Expand Up @@ -195,12 +197,12 @@ schema:
{
name: trace_sampled,
type: UInt,
args: { schema_modifiers: [ nullable ], size: 8 },
args: { schema_modifiers: [nullable], size: 8 },
},
{
name: num_processing_errors,
type: UInt,
args: { schema_modifiers: [ nullable ], size: 64 },
args: { schema_modifiers: [nullable], size: 64 },
},
{ name: replay_id, type: UUID, args: { schema_modifiers: [nullable] } },
]
Expand Down Expand Up @@ -473,7 +475,6 @@ subscription_validators:
- orderby
required_time_column: timestamp


join_relationships:
grouped:
rhs_entity: groupedmessage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 8 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 8 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 8 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 8 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand All @@ -23,7 +25,7 @@ schema:
{
name: value,
type: AggregateFunction,
args: { func: sum, arg_types: [ { type: Float, args: { size: 64 } } ] },
args: { func: sum, arg_types: [{ type: Float, args: { size: 64 } }] },
},
]
required_time_column: timestamp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 8 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 8 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 8 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 32 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 32 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 32 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 8 } },
{ name: use_case_id, type: String },
]
required_time_column: timestamp

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 32 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
2 changes: 2 additions & 0 deletions snuba/datasets/configuration/metrics/entities/org_sets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ schema:
{ name: metric_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: bucketed_time, type: DateTime },
{ name: granularity, type: UInt, args: { size: 32 } },
{ name: use_case_id, type: String },
{
name: tags,
type: Nested,
Expand Down
40 changes: 20 additions & 20 deletions snuba/datasets/configuration/outcomes/entities/outcomes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,38 +7,38 @@ schema:
{ name: project_id, type: UInt, args: { size: 64 } },
{ name: key_id, type: UInt, args: { size: 64 } },
{ name: timestamp, type: DateTime },
{ name: time, type: DateTime },
{ name: outcome, type: UInt, args: { size: 8 } },
{ name: reason, type: String },
{ name: quantity, type: UInt, args: { size: 64 } },
{ name: category, type: UInt, args: { size: 8 } },
{ name: times_seen, type: UInt, args: { size: 64 } },
{ name: time, type: DateTime },
]

required_time_column: timestamp
storages:
- storage: outcomes_hourly
is_writable: false
- storage: outcomes_raw
is_writable: true
- storage: outcomes_hourly
is_writable: false
- storage: outcomes_raw
is_writable: true
storage_selector:
selector: SimpleQueryStorageSelector
args:
storage: outcomes_hourly
query_processors:
- processor: BasicFunctionsProcessor
- processor: TimeSeriesProcessor
args:
time_group_columns:
time: timestamp
time_parse_columns:
- timestamp
- processor: ReferrerRateLimiterProcessor
- processor: OrganizationRateLimiterProcessor
args:
org_column: org_id
- processor: BasicFunctionsProcessor
- processor: TimeSeriesProcessor
args:
time_group_columns:
time: timestamp
time_parse_columns:
- timestamp
- processor: ReferrerRateLimiterProcessor
- processor: OrganizationRateLimiterProcessor
args:
org_column: org_id
validators:
- validator: EntityRequiredColumnValidator
args:
required_filter_columns:
- org_id
- validator: EntityRequiredColumnValidator
args:
required_filter_columns:
- org_id
Loading

0 comments on commit 3c3e7ca

Please sign in to comment.