Skip to content

Conversation

@xjshi
Copy link
Contributor

@xjshi xjshi commented Jan 6, 2026

In SentryTraceContext, the 'sampled' key was incorrectly using _sampleRate.

📜 Description

In SentryTraceContext.m, the 'sampled' key was incorrectly using _sampleRate. This resulted in malformed trace contexts being sent to Sentry, potentially breaking sampling decisions in distributed tracing.

In SentryTraceContext, the 'sampled' key was incorrectly using _sampleRate.
@itaybre
Copy link
Contributor

itaybre commented Jan 7, 2026

@xjshi thanks for the fix!

Would you mind adding a test for this? Or would like me to push the required tests for this? (to this doesn't happen again)

@itaybre itaybre added the ready-to-merge Use this label to trigger all PR workflows label Jan 7, 2026
@codecov
Copy link

codecov bot commented Jan 7, 2026

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
4205 2 4203 13
View the top 1 failed test(s) by shortest run time
SentryTests.SentryFileManagerTests::testCreateDirectoryIfNotExists_successful_shouldNotLogError
Stack Traces | 0s run time
.../SentryTests/Helper/SentryFileManagerTests.swift:1056 - XCTAssertEqual failed: ("1") is not equal to ("0")
View the full list of 1 ❄️ flaky test(s)
SentryCrashReportStore_Tests::testDeleteAllReports

Flake rate in main: 99.12% (Passed 1 times, Failed 112 times)

Stack Traces | 0s run time
.../SentryTests/SentryCrash/SentryCrashReportStore_Tests.m:126 - ((sentrycrashcrs_getReportCount()) equal to (reportCount)) failed: ("3") is not equal to ("4")

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@xjshi
Copy link
Contributor Author

xjshi commented Jan 8, 2026

@itaybre Test added. It verifies all fields in the dictionary returned by the serialize method to ensure correct mapping.

@philprime
Copy link
Member

@sentry review

@itaybre
Copy link
Contributor

itaybre commented Jan 9, 2026

Updating the branch, so checks can pass 🟢

Copy link
Contributor

@itaybre itaybre left a comment

Choose a reason for hiding this comment

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

LGTM

@itaybre
Copy link
Contributor

itaybre commented Jan 9, 2026

Merging this PR, I will open a different one with the changelog changes

@itaybre itaybre merged commit 013fd4d into getsentry:main Jan 9, 2026
240 of 247 checks passed
itaybre added a commit that referenced this pull request Jan 9, 2026
itaybre added a commit that referenced this pull request Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Use this label to trigger all PR workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants