Skip to content

feat(profiling): add platform header to the chunk item-type in the envelope #4178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 21, 2025
+18 −9
Merged

feat(profiling): add platform header to the chunk item-type in the envelope #4178

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81aadbb
add platform header to the chunk item-type in the envelope
viglia Mar 20, 2025
c62897b
fix linter
viglia Mar 20, 2025
0d1a30d
fix blake linter warning
viglia Mar 20, 2025
1002ac6
add default value as a fallback when getting the platform for the chunk
viglia Mar 21, 2025
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
6 changes: 5 additions & 1 deletion sentry_sdk/envelope.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@ def add_profile_chunk(
):
# type: (...) -> None
self.add_item(
Item(payload=PayloadRef(json=profile_chunk), type="profile_chunk")
Item(
payload=PayloadRef(json=profile_chunk),
type="profile_chunk",
headers={"platform": profile_chunk.get("platform")},
Copy link
Contributor

@phacops phacops Mar 20, 2025

Choose a reason for hiding this comment

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

I wouldn't mind to avoid any potential None value here. Can we hardcode the same value we set in profile_chunk? Or only add the headers if we have a key platform?

Copy link
Contributor Author

@viglia viglia Mar 20, 2025

Choose a reason for hiding this comment

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

@phacops I could throw-in an extra check if you wish but just fyi: if profile_chunk.get("platform") returns None it means the platform is not set at the payload either and therefore the whole profile would be rejected as it does not comply to the Relay defined schema (platform is a mandatory field).

Copy link
Contributor

@phacops phacops Mar 20, 2025

Choose a reason for hiding this comment

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

True. I'm just allergic to null values.

)
)

def add_checkin(
Expand Down
21 changes: 13 additions & 8 deletions tests/profiler/test_continuous_profiler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ def assert_single_transaction_with_profile_chunks(
if max_chunks is not None:
assert len(items["profile_chunk"]) <= max_chunks

for chunk_item in items["profile_chunk"]:
chunk = chunk_item.payload.json
headers = chunk_item.headers
assert chunk["platform"] == headers["platform"]

transaction = items["transaction"][0].payload.json

trace_context = transaction["contexts"]["trace"]
Expand Down Expand Up @@ -215,12 +220,12 @@ def assert_single_transaction_without_profile_chunks(envelopes):
pytest.param(
start_profile_session,
stop_profile_session,
id="start_profile_session/stop_profile_session",
id="start_profile_session/stop_profile_session (deprecated)",
),
pytest.param(
start_profiler,
stop_profiler,
id="start_profiler/stop_profiler (deprecated)",
id="start_profiler/stop_profiler",
),
],
)
Expand Down Expand Up @@ -292,12 +297,12 @@ def test_continuous_profiler_auto_start_and_manual_stop(
pytest.param(
start_profile_session,
stop_profile_session,
id="start_profile_session/stop_profile_session",
id="start_profile_session/stop_profile_session (deprecated)",
),
pytest.param(
start_profiler,
stop_profiler,
id="start_profiler/stop_profiler (deprecated)",
id="start_profiler/stop_profiler",
),
],
)
Expand Down Expand Up @@ -374,12 +379,12 @@ def test_continuous_profiler_manual_start_and_stop_sampled(
pytest.param(
start_profile_session,
stop_profile_session,
id="start_profile_session/stop_profile_session",
id="start_profile_session/stop_profile_session (deprecated)",
),
pytest.param(
start_profiler,
stop_profiler,
id="start_profiler/stop_profiler (deprecated)",
id="start_profiler/stop_profiler",
),
],
)
Expand Down Expand Up @@ -544,12 +549,12 @@ def test_continuous_profiler_auto_start_and_stop_unsampled(
pytest.param(
start_profile_session,
stop_profile_session,
id="start_profile_session/stop_profile_session",
id="start_profile_session/stop_profile_session (deprecated)",
),
pytest.param(
start_profiler,
stop_profiler,
id="start_profiler/stop_profiler (deprecated)",
id="start_profiler/stop_profiler",
),
],
)
Expand Down
Loading