Skip to content

Commit b97c77d

Browse files
vigliaandrewshie-sentry
authored andcommitted
fix(profiling): fix chunked query strategy for flamegraph (#97461)
This fixes a bug for flamegraph chunked query strategy where, for chunks associated to a transaction (stored into the `ProfilerMeta`), we'd only use the latest "window" `[t-n, t-i]` instead of using the whole time-range scanned `[t-n, t]`.
1 parent aef18f2 commit b97c77d

File tree

1 file changed

+39
-24
lines changed

1 file changed

+39
-24
lines changed

src/sentry/profiles/flamegraph.py

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -247,21 +247,21 @@ def get_profile_candidates_from_transactions_v2(self) -> ProfileCandidates:
247247
profiler_metas: list[ProfilerMeta] = []
248248

249249
assert self.snuba_params.start is not None and self.snuba_params.end is not None
250-
original_start, original_end = self.snuba_params.start, self.snuba_params.end
250+
snuba_params = self.snuba_params.copy()
251251

252252
for chunk_start, chunk_end in split_datetime_range_exponential(
253-
original_start,
254-
original_end,
253+
self.snuba_params.start,
254+
self.snuba_params.end,
255255
initial_chunk_delta,
256256
max_chunk_delta,
257257
multiplier,
258258
reverse=True,
259259
):
260-
self.snuba_params.start = chunk_start
261-
self.snuba_params.end = chunk_end
260+
snuba_params.start = chunk_start
261+
snuba_params.end = chunk_end
262262

263263
builder = self.get_transactions_based_candidate_query(
264-
query=self.query, limit=max_profiles
264+
query=self.query, limit=max_profiles, snuba_params=snuba_params
265265
)
266266

267267
results = builder.run_query(
@@ -298,9 +298,9 @@ def get_profile_candidates_from_transactions_v2(self) -> ProfileCandidates:
298298
continuous_profile_candidates: list[ContinuousProfileCandidate] = []
299299

300300
if max_continuous_profile_candidates > 0:
301+
snuba_params.end = self.snuba_params.end
301302
continuous_profile_candidates, _ = self.get_chunks_for_profilers(
302-
profiler_metas,
303-
max_continuous_profile_candidates,
303+
profiler_metas, max_continuous_profile_candidates, snuba_params
304304
)
305305

306306
return {
@@ -309,12 +309,15 @@ def get_profile_candidates_from_transactions_v2(self) -> ProfileCandidates:
309309
}
310310

311311
def get_transactions_based_candidate_query(
312-
self, query: str | None, limit: int
312+
self,
313+
query: str | None,
314+
limit: int,
315+
snuba_params: SnubaParams | None = None,
313316
) -> DiscoverQueryBuilder:
314317
builder = DiscoverQueryBuilder(
315318
dataset=Dataset.Discover,
316319
params={},
317-
snuba_params=self.snuba_params,
320+
snuba_params=snuba_params or self.snuba_params,
318321
selected_columns=[
319322
"id",
320323
"project.id",
@@ -356,7 +359,10 @@ def get_transactions_based_candidate_query(
356359
return builder
357360

358361
def get_chunks_for_profilers(
359-
self, profiler_metas: list[ProfilerMeta], limit: int
362+
self,
363+
profiler_metas: list[ProfilerMeta],
364+
limit: int,
365+
snuba_params: SnubaParams | None = None,
360366
) -> tuple[list[ContinuousProfileCandidate], float]:
361367
total_duration = 0.0
362368

@@ -365,7 +371,8 @@ def get_chunks_for_profilers(
365371

366372
chunk_size = options.get("profiling.continuous-profiling.chunks-query.size")
367373
queries = [
368-
self._create_chunks_query(chunk) for chunk in chunked(profiler_metas, chunk_size)
374+
self._create_chunks_query(chunk, snuba_params)
375+
for chunk in chunked(profiler_metas, chunk_size)
369376
]
370377

371378
results = self._query_chunks_for_profilers(queries)
@@ -416,8 +423,11 @@ def get_chunks_for_profilers(
416423

417424
return continuous_profile_candidates, total_duration
418425

419-
def _create_chunks_query(self, profiler_metas: list[ProfilerMeta]) -> Query:
426+
def _create_chunks_query(
427+
self, profiler_metas: list[ProfilerMeta], snuba_params: SnubaParams | None = None
428+
) -> Query:
420429
assert profiler_metas, "profiler_metas cannot be empty"
430+
snuba_params = snuba_params or self.snuba_params
421431

422432
profiler_conditions = [profiler_meta.as_condition() for profiler_meta in profiler_metas]
423433

@@ -434,10 +444,10 @@ def _create_chunks_query(self, profiler_metas: list[ProfilerMeta]) -> Query:
434444
start_condition = Condition(
435445
Column("start_timestamp"),
436446
Op.LT,
437-
resolve_datetime64(self.snuba_params.end),
447+
resolve_datetime64(snuba_params.end),
438448
)
439449
end_condition = Condition(
440-
Column("end_timestamp"), Op.GTE, resolve_datetime64(self.snuba_params.start)
450+
Column("end_timestamp"), Op.GTE, resolve_datetime64(snuba_params.start)
441451
)
442452

443453
return Query(
@@ -652,21 +662,21 @@ def get_profile_candidates_from_profiles_v2(self) -> ProfileCandidates:
652662
profiler_metas: list[ProfilerMeta] = []
653663

654664
assert self.snuba_params.start is not None and self.snuba_params.end is not None
655-
original_start, original_end = self.snuba_params.start, self.snuba_params.end
665+
snuba_params = self.snuba_params.copy()
656666

657667
for chunk_start, chunk_end in split_datetime_range_exponential(
658-
original_start,
659-
original_end,
668+
self.snuba_params.start,
669+
self.snuba_params.end,
660670
initial_chunk_delta,
661671
max_chunk_delta,
662672
multiplier,
663673
reverse=True,
664674
):
665-
self.snuba_params.start = chunk_start
666-
self.snuba_params.end = chunk_end
675+
snuba_params.start = chunk_start
676+
snuba_params.end = chunk_end
667677

668678
builder = self.get_transactions_based_candidate_query(
669-
query=self.query, limit=max_profiles
679+
query=self.query, limit=max_profiles, snuba_params=snuba_params
670680
)
671681
results = builder.run_query(referrer)
672682
results = builder.process_results(results)
@@ -704,8 +714,9 @@ def get_profile_candidates_from_profiles_v2(self) -> ProfileCandidates:
704714
# If there are continuous profiles attached to transactions, we prefer those as
705715
# the active thread id gives us more user friendly flamegraphs than without.
706716
if profiler_metas and max_continuous_profile_candidates > 0:
717+
snuba_params.end = self.snuba_params.end
707718
continuous_profile_candidates, continuous_duration = self.get_chunks_for_profilers(
708-
profiler_metas, max_continuous_profile_candidates
719+
profiler_metas, max_continuous_profile_candidates, snuba_params
709720
)
710721

711722
seen_chunks = {
@@ -720,10 +731,14 @@ def get_profile_candidates_from_profiles_v2(self) -> ProfileCandidates:
720731
conditions = []
721732
conditions.append(Condition(Column("project_id"), Op.IN, self.snuba_params.project_ids))
722733
conditions.append(
723-
Condition(Column("start_timestamp"), Op.LT, resolve_datetime64(original_end))
734+
Condition(
735+
Column("start_timestamp"), Op.LT, resolve_datetime64(self.snuba_params.end)
736+
)
724737
)
725738
conditions.append(
726-
Condition(Column("end_timestamp"), Op.GTE, resolve_datetime64(original_start))
739+
Condition(
740+
Column("end_timestamp"), Op.GTE, resolve_datetime64(self.snuba_params.start)
741+
)
727742
)
728743
environments = self.snuba_params.environment_names
729744
if environments:

0 commit comments

Comments
 (0)