Skip to content

Commit

Permalink
davids fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
volokluev committed Jan 8, 2025
1 parent bc3f9d4 commit 07f5413
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 34 deletions.
24 changes: 6 additions & 18 deletions snuba/web/rpc/v1/resolvers/R_eap_spans/common/aggregation.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class ExtrapolationContext(ABC):
sample_count: int

@property
def extrapolated_data_present(self) -> bool:
def is_data_present(self) -> bool:
return self.sample_count > 0

@property
Expand All @@ -67,8 +67,8 @@ def from_row(
value = row_data[column_label]

confidence_interval = None
average_sample_rate = None
sample_count = None
average_sample_rate = 0
sample_count = 0

percentile = 0.0
granularity = 0.0
Expand Down Expand Up @@ -108,18 +108,6 @@ def from_row(
elif custom_column_information.custom_column_id == "count":
sample_count = col_value

if (
confidence_interval is None
or average_sample_rate is None
or sample_count is None
):
return GenericExtrapolationContext(
value=value,
confidence_interval=None,
average_sample_rate=0,
sample_count=0,
)

if is_percentile:
return PercentileExtrapolationContext(
value=value,
Expand Down Expand Up @@ -150,7 +138,7 @@ def is_extrapolated(self) -> bool:

@cached_property
def reliability(self) -> Reliability.ValueType:
if not self.is_extrapolated or not self.extrapolated_data_present:
if not self.is_extrapolated or not self.is_data_present:
return Reliability.RELIABILITY_UNSPECIFIED

relative_confidence = (
Expand Down Expand Up @@ -183,7 +171,7 @@ def is_extrapolated(self) -> bool:

@cached_property
def reliability(self) -> Reliability.ValueType:
if not self.is_extrapolated or not self.extrapolated_data_present:
if not self.is_extrapolated or not self.is_data_present:
return Reliability.RELIABILITY_UNSPECIFIED

lower_bound, upper_bound = _calculate_approximate_ci_percentile_levels(
Expand Down Expand Up @@ -492,7 +480,7 @@ def get_confidence_interval_column(
# │ ⎲ 1
# ╲ │ ⎳ ── ₙ
# ╲ │ ⁱ⁼¹ wᵢ ⎲ 2 2 2
# Z * ╲ │-log(──────) * ⎳ xᵢwᵢ - xᵢwᵢ
# Z * ╲ │-log(──────) * ⎳ xᵢwᵢ - xᵢwᵢ
# ╲│ n ⁱ⁼¹
Function.FUNCTION_SUM: f.multiply(
z_value,
Expand Down
26 changes: 10 additions & 16 deletions snuba/web/rpc/v1/resolvers/R_eap_spans/resolver_time_series.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,7 @@ def _convert_result_timeseries(
extrapolation_context = ExtrapolationContext.from_row(
timeseries.label, row_data
)
if (
# This isn't quite right but all non extrapolated aggregates
# are assumed to be present.
not extrapolation_context.is_extrapolated
# This checks if the extrapolated aggregate is present by
# inspecting the sample count
or extrapolation_context.extrapolated_data_present
):
if extrapolation_context.is_data_present:
timeseries.data_points.append(
DataPoint(
data=row_data[timeseries.label],
Expand Down Expand Up @@ -188,32 +181,33 @@ def _build_query(request: TimeSeriesRequest) -> Query:
for aggregation in request.aggregations
]

extrapolation_columns = []
additional_context_columns = []
for aggregation in request.aggregations:
if (
aggregation.extrapolation_mode
== ExtrapolationMode.EXTRAPOLATION_MODE_SAMPLE_WEIGHTED
):
confidence_interval_column = get_confidence_interval_column(aggregation)
if confidence_interval_column is not None:
extrapolation_columns.append(
additional_context_columns.append(
SelectedExpression(
name=confidence_interval_column.alias,
expression=confidence_interval_column,
)
)

average_sample_rate_column = get_average_sample_rate_column(aggregation)
count_column = get_count_column(aggregation)
extrapolation_columns.append(
additional_context_columns.append(
SelectedExpression(
name=average_sample_rate_column.alias,
expression=average_sample_rate_column,
)
)
extrapolation_columns.append(
SelectedExpression(name=count_column.alias, expression=count_column)
)

count_column = get_count_column(aggregation)
additional_context_columns.append(
SelectedExpression(name=count_column.alias, expression=count_column)
)

groupby_columns = [
SelectedExpression(
Expand Down Expand Up @@ -254,7 +248,7 @@ def _build_query(request: TimeSeriesRequest) -> Query:
),
*aggregation_columns,
*groupby_columns,
*extrapolation_columns,
*additional_context_columns,
],
granularity=request.granularity_secs,
condition=base_conditions_and(
Expand Down

0 comments on commit 07f5413

Please sign in to comment.