Skip to content

Commit 42a0a89

Browse files
authored
feat(profiling): Flag to test always using direct continuous profile … (#93705)
…chunks Testing the strategy of always including direct continuous profile chunks. This was a concern previously on web servers where idle time was the majority of the flamegraph. But with lifecycle=trace, this is less of an issue. And by always including it, we can improve the situation where SDK devs are switching between lifecycle=trace and lifecycle=manual.
1 parent 90e4076 commit 42a0a89

File tree

3 files changed

+74
-41
lines changed

3 files changed

+74
-41
lines changed

src/sentry/api/endpoints/organization_profiling_profiles.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ def get(self, request: Request, organization: Organization) -> HttpResponse:
8080

8181
with handle_query_errors():
8282
executor = FlamegraphExecutor(
83+
request=request,
8384
snuba_params=snuba_params,
8485
data_source=serialized["dataSource"],
8586
query=serialized.get("query", ""),

src/sentry/features/temporary.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,8 @@ def register_temporary_features(manager: FeatureManager):
290290
manager.add("organizations:profiling-browser", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=False)
291291
# Enables separate differential flamegraph page
292292
manager.add("organizations:profiling-differential-flamegraph-page", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
293+
# Enables ability usage of direct profile chunks all the time
294+
manager.add("organizations:profiling-flamegraph-use-direct-chunks", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
293295
# Enable global suspect functions in profiling
294296
manager.add("organizations:profiling-global-suspect-functions", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
295297
# Enable profiling summary redesign view

src/sentry/profiles/flamegraph.py

Lines changed: 71 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from datetime import datetime
55
from typing import Any, Literal, NotRequired, TypedDict
66

7+
from rest_framework.request import Request as HttpRequest
78
from snuba_sdk import (
89
And,
910
Column,
@@ -19,7 +20,7 @@
1920
Storage,
2021
)
2122

22-
from sentry import options
23+
from sentry import features, options
2324
from sentry.search.eap.types import EAPResponse, SearchResolverConfig
2425
from sentry.search.events.builder.discover import DiscoverQueryBuilder
2526
from sentry.search.events.builder.profile_functions import ProfileFunctionsQueryBuilder
@@ -85,11 +86,13 @@ class FlamegraphExecutor:
8586
def __init__(
8687
self,
8788
*,
89+
request: HttpRequest,
8890
snuba_params: SnubaParams,
8991
data_source: Literal["functions", "transactions", "profiles", "spans"],
9092
query: str,
9193
fingerprint: int | None = None,
9294
):
95+
self.request = request
9396
self.snuba_params = snuba_params
9497
self.data_source = data_source
9598
self.query = query
@@ -165,12 +168,14 @@ def get_profile_candidates_from_functions(self) -> ProfileCandidates:
165168
max_profiles - len(transaction_profile_candidates), 0
166169
)
167170

171+
continuous_profile_candidates, _ = self.get_chunks_for_profilers(
172+
profiler_metas,
173+
max_continuous_profile_candidates,
174+
)
175+
168176
return {
169177
"transaction": transaction_profile_candidates,
170-
"continuous": self.get_chunks_for_profilers(
171-
profiler_metas,
172-
max_continuous_profile_candidates,
173-
),
178+
"continuous": continuous_profile_candidates,
174179
}
175180

176181
def get_profile_candidates_from_transactions(self) -> ProfileCandidates:
@@ -196,23 +201,25 @@ def get_profile_candidates_from_transactions(self) -> ProfileCandidates:
196201
max_profiles - len(transaction_profile_candidates), 0
197202
)
198203

204+
continuous_profile_candidates, _ = self.get_chunks_for_profilers(
205+
[
206+
ProfilerMeta(
207+
project_id=row["project.id"],
208+
profiler_id=row["profiler.id"],
209+
thread_id=row["thread.id"],
210+
start=row["precise.start_ts"],
211+
end=row["precise.finish_ts"],
212+
transaction_id=row["id"],
213+
)
214+
for row in results["data"]
215+
if row["profiler.id"] is not None and row["thread.id"]
216+
],
217+
max_continuous_profile_candidates,
218+
)
219+
199220
return {
200221
"transaction": transaction_profile_candidates,
201-
"continuous": self.get_chunks_for_profilers(
202-
[
203-
ProfilerMeta(
204-
project_id=row["project.id"],
205-
profiler_id=row["profiler.id"],
206-
thread_id=row["thread.id"],
207-
start=row["precise.start_ts"],
208-
end=row["precise.finish_ts"],
209-
transaction_id=row["id"],
210-
)
211-
for row in results["data"]
212-
if row["profiler.id"] is not None and row["thread.id"]
213-
],
214-
max_continuous_profile_candidates,
215-
),
222+
"continuous": continuous_profile_candidates,
216223
}
217224

218225
def get_transactions_based_candidate_query(
@@ -264,9 +271,11 @@ def get_transactions_based_candidate_query(
264271

265272
def get_chunks_for_profilers(
266273
self, profiler_metas: list[ProfilerMeta], limit: int
267-
) -> list[ContinuousProfileCandidate]:
274+
) -> tuple[list[ContinuousProfileCandidate], float]:
275+
total_duration = 0.0
276+
268277
if len(profiler_metas) == 0:
269-
return []
278+
return [], total_duration
270279

271280
chunk_size = options.get("profiling.continuous-profiling.chunks-query.size")
272281
queries = [
@@ -304,6 +313,8 @@ def get_chunks_for_profilers(
304313
"end": str(int(profiler_meta.end * 1e9)),
305314
}
306315

316+
total_duration += profiler_meta.end - profiler_meta.start
317+
307318
if profiler_meta.transaction_id is not None:
308319
candidate["transaction_id"] = profiler_meta.transaction_id
309320

@@ -317,7 +328,7 @@ def get_chunks_for_profilers(
317328
continue
318329
break
319330

320-
return continuous_profile_candidates
331+
return continuous_profile_candidates, total_duration
321332

322333
def _create_chunks_query(self, profiler_metas: list[ProfilerMeta]) -> Query:
323334
assert profiler_metas, "profiler_metas cannot be empty"
@@ -480,22 +491,38 @@ def get_profile_candidates_from_profiles(self) -> ProfileCandidates:
480491
]
481492

482493
continuous_profile_candidates: list[ContinuousProfileCandidate] = []
494+
continuous_duration = 0.0
483495

484496
# If there are continuous profiles attached to transactions, we prefer those as
485497
# the active thread id gives us more user friendly flamegraphs than without.
486498
if profiler_metas:
487-
continuous_profile_candidates = self.get_chunks_for_profilers(
499+
continuous_profile_candidates, continuous_duration = self.get_chunks_for_profilers(
488500
profiler_metas, max_continuous_profile_candidates
489501
)
490502

503+
seen_chunks = {
504+
(candidate["profiler_id"], candidate["chunk_id"])
505+
for candidate in continuous_profile_candidates
506+
}
507+
508+
always_use_direct_chunks = features.has(
509+
"organizations:profiling-flamegraph-always-use-direct-chunks",
510+
self.snuba_params.organization,
511+
actor=self.request.user,
512+
)
513+
491514
# If we still don't have any continuous profile candidates, we'll fall back to
492515
# directly using the continuous profiling data
493-
if not continuous_profile_candidates:
494-
total_duration = 0.0
495-
516+
if not continuous_profile_candidates or always_use_direct_chunks:
517+
total_duration = continuous_duration if always_use_direct_chunks else 0.0
496518
max_duration = options.get("profiling.continuous-profiling.flamegraph.max-seconds")
497519

498520
for row in continuous_profile_results["data"]:
521+
522+
# Make sure to dedupe profile chunks so we don't reuse chunks
523+
if (row["profiler_id"], row["chunk_id"]) in seen_chunks:
524+
continue
525+
499526
start = datetime.fromisoformat(row["start_timestamp"]).timestamp()
500527
end = datetime.fromisoformat(row["end_timestamp"]).timestamp()
501528

@@ -535,22 +562,25 @@ def get_profile_candidates_from_spans(self) -> ProfileCandidates:
535562
max_continuous_profile_candidates = max(
536563
max_profiles - len(transaction_profile_candidates), 0
537564
)
565+
566+
continuous_profile_candidates, _ = self.get_chunks_for_profilers(
567+
[
568+
ProfilerMeta(
569+
project_id=row["project.id"],
570+
profiler_id=row["profiler.id"],
571+
thread_id=row["thread.id"],
572+
start=row["precise.start_ts"],
573+
end=row["precise.finish_ts"],
574+
)
575+
for row in results["data"]
576+
if row["profiler.id"] is not None and row["thread.id"]
577+
],
578+
max_continuous_profile_candidates,
579+
)
580+
538581
return {
539582
"transaction": transaction_profile_candidates,
540-
"continuous": self.get_chunks_for_profilers(
541-
[
542-
ProfilerMeta(
543-
project_id=row["project.id"],
544-
profiler_id=row["profiler.id"],
545-
thread_id=row["thread.id"],
546-
start=row["precise.start_ts"],
547-
end=row["precise.finish_ts"],
548-
)
549-
for row in results["data"]
550-
if row["profiler.id"] is not None and row["thread.id"]
551-
],
552-
max_continuous_profile_candidates,
553-
),
583+
"continuous": continuous_profile_candidates,
554584
}
555585

556586
def get_spans_based_candidates(self, query: str | None, limit: int) -> EAPResponse:

0 commit comments

Comments
 (0)