Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/sentry/api/endpoints/organization_events_heatmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,18 @@ class OrganizationEventsHeatmapEndpoint(OrganizationEventsEndpointBase):
}
)

@staticmethod
def _format_long_float(value: float) -> str:
"""
Python's default float-to-string uses scientific notation for very small
values (e.g. 8.527e-06), which the search query parser does not support.
Fixed-point with 20 decimal places produces a plain decimal string the parser can handle.
"""
if "e" in str(value) or "E" in str(value):
return f"{value:.20f}".rstrip("0").rstrip(".")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Precision loss silently rounds sub-1e-20 values to zero

Low Severity

_format_long_float uses :.20f formatting, which only provides 20 decimal places. For values smaller than ~5×10⁻²¹, the formatted result rounds to all zeros (0.00000000000000000000), and after rstrip("0").rstrip(".") the function silently returns '0' instead of the actual value. This would produce incorrect bucket boundaries in the query without any error signal.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f72abe6. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

welp echarts won't process decimals over 20 places anyways so

else:
return str(value)
Comment thread
nikkikapadia marked this conversation as resolved.

def get(self, request: Request, organization: Organization) -> Response:
"""
Retrieves explore data for a given organization as a heatmap.
Expand Down Expand Up @@ -186,17 +198,19 @@ def get(self, request: Request, organization: Organization) -> Response:
upper_bound = bucket_ranges.min_value + (current_bucket + 1) * bucket_size

if current_bucket == y_buckets - 1:
yAxes[lower_bound] = f"{z_function}_if(`{yAxis}:>={lower_bound}`, {yAxis})"
yAxes[lower_bound] = (
f"{z_function}_if(`{yAxis}:>={self._format_long_float(lower_bound)}`, {yAxis})"
)
else:
yAxes[lower_bound] = (
f"{z_function}_if(`{yAxis}:>={lower_bound} AND {yAxis}:<{upper_bound}`, {yAxis})"
f"{z_function}_if(`{yAxis}:>={self._format_long_float(lower_bound)} AND {yAxis}:<{self._format_long_float(upper_bound)}`, {yAxis})"
)
else:
# if max == min, then just have 1 bucket
bucket_size = 0
y_buckets = 1
yAxes = {
bucket_ranges.min_value: f"{z_function}_if(`{yAxis}:{bucket_ranges.min_value}`, {yAxis})"
bucket_ranges.min_value: f"{z_function}_if(`{yAxis}:{self._format_long_float(bucket_ranges.min_value)}`, {yAxis})"
}

result = dataset.run_timeseries_query(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from datetime import timedelta

import pytest
from django.urls import reverse

from sentry.api.endpoints.organization_events_heatmap import OrganizationEventsHeatmapEndpoint
from sentry.testutils.helpers.datetime import before_now
from tests.snuba.api.endpoints.test_organization_events import (
OrganizationEventsEndpointTestBase,
Expand Down Expand Up @@ -376,3 +378,98 @@ def test_invalid_log_scale(self):
)
assert response.status_code == 400, response.content
assert response.data["detail"] == "logScale cannot be 1"

def test_very_small_float_values(self) -> None:
# Values like 8.527e-06 would previously be formatted in scientific
# notation inside the query string, which the search query parser rejects.
small_values = [0.000008, 0.000009, 0.000010, 0.000011]

trace_metrics = []
for hour, value in enumerate(small_values):
trace_metrics.append(
self.create_trace_metric(
"foo",
value,
"counter",
timestamp=self.start + timedelta(hours=hour),
)
)
self.store_eap_items(trace_metrics)

response = self._do_request(
data={
"start": self.start,
"end": self.start + timedelta(hours=6),
"yAxis": "value",
"interval": "1h",
"yBuckets": 4,
"query": "metric.name:foo metric.type:counter",
"project": self.project.id,
"dataset": self.dataset,
},
)
assert response.status_code == 200, response.content
assert response.data["meta"]["yAxis"]["start"] == pytest.approx(0.000008, rel=1e-3)
assert response.data["meta"]["yAxis"]["end"] == pytest.approx(0.000011, rel=1e-3)

def test_very_small_float_min_equals_max(self) -> None:
# When min == max and the value is very small, the single-bucket query
# must also use plain decimal notation rather than scientific notation.
trace_metrics = [
self.create_trace_metric(
"foo",
0.000008527,
"counter",
timestamp=self.start + timedelta(hours=hour),
)
for hour in range(3)
]
self.store_eap_items(trace_metrics)

response = self._do_request(
data={
"start": self.start,
"end": self.start + timedelta(hours=6),
"yAxis": "value",
"interval": "1h",
"yBuckets": 10,
"query": "metric.name:foo metric.type:counter",
"project": self.project.id,
"dataset": self.dataset,
},
)
assert response.status_code == 200, response.content
assert response.data["meta"]["yAxis"]["bucketCount"] == 1
assert response.data["meta"]["yAxis"]["start"] == pytest.approx(0.000008527, rel=1e-3)


class TestFormatLongFloat:
def test_small_number_no_scientific_notation(self) -> None:
result = OrganizationEventsHeatmapEndpoint._format_long_float(8.527e-06)
assert "e" not in result
assert "E" not in result
assert result == "0.000008527"

def test_normal_number(self) -> None:
result = OrganizationEventsHeatmapEndpoint._format_long_float(123.456)
assert "e" not in result
assert result == "123.456"

def test_huge_number_no_scientific_notation(self) -> None:
result = OrganizationEventsHeatmapEndpoint._format_long_float(
123456789012345678901234567890
)
assert "e" not in result
assert "E" not in result
assert result == "123456789012345678901234567890"

def test_zero(self) -> None:
result = OrganizationEventsHeatmapEndpoint._format_long_float(0.0)
assert result == "0.0"

def test_very_small_number_is_parseable(self) -> None:
# Ensure the formatted string looks like a plain decimal Python float
result = OrganizationEventsHeatmapEndpoint._format_long_float(1.23e-10)
assert "e" not in result
assert "E" not in result
float(result) # must not raise
Loading