Repository navigation
feat(core): Report cellular network technology - #6827
Conversation
The Android SDK reports the generation of the cellular network technology in device.connection_effective_type from sentry-java 8.60.0. The field reaches events through serializeScope, so this SDK only has to bundle that version. iOS already reports the field, because sentry-cocoa 9.30.0 puts it into the extra context that fetchNativeDeviceContexts merges into the device context. The tests guard the path from the native layer to the event. The version bump follows when sentry-java 8.60.0 is released. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
|
|
@antonis this one is practically ready and will work automatically once Android releases a new version (iOS released it already). I wonder what you think of this PR — there is really no work needs to be done on our side (except for a test), and I feel like we could either merge this now or wait until the next Android release. Wdyt? |
antonis
left a comment
There was a problem hiding this comment.
LGTM 🎉
Happy to approve and leave the merge to you.
I would advocate on keeping this unmerged to test on a device after the Android bump and avoid unnecessary reverts if our 8.30.0 ships before Android 8.60.0 is available.
|
let's wait before Android gets released later this week |
…ve layer sentry-android 8.60.0 is not released yet. The module now fills device.connection_effective_type when sentry-android does not set it. After the bump, this fallback has no effect and can be removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…act Native layer" This reverts commit c94e729.
…d iOS Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| contexts: expect.objectContaining({ | ||
| device: expect.objectContaining({ | ||
| arch: expect.any(String), | ||
| connection_type: expect.any(String), |
There was a problem hiding this comment.
Bug: The iOS e2e test incorrectly asserts that the optional connection_type field is always present, which can lead to flaky test failures.
Severity: LOW
Suggested Fix
The assertion for connection_type should be updated to handle cases where the field is absent. Instead of expect.any(String), consider a more flexible matcher that accepts either a string or an undefined value to align with the native SDK's behavior and prevent flaky test failures.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: samples/react-native/e2e/tests/captureMessage/captureMessage.test.ios.ts#L61
Potential issue: The iOS end-to-end test at `captureMessage.test.ios.ts:61` adds a
strict assertion for `connection_type: expect.any(String)`. However, the underlying
`sentry-cocoa` SDK documentation specifies that the `connection_type` field is optional
and will be omitted if the connection is unknown. This discrepancy will cause the test
to fail when the SDK legitimately omits this field, a plausible scenario in CI
environments due to timing or network conditions. This makes the test flaky and
unreliable.
Did we get this right? 👍 / 👎 to inform future reviews.
📢 Type of change
📜 Description
Events now report the generation of the cellular network technology in
device.connection_effective_typeon Android and iOS, for example4gor5g.The native SDKs produce the field, and this SDK passes it on without code changes. On iOS, sentry-cocoa 9.30.0 adds it to the extra context that
fetchNativeDeviceContextsmerges into the device context. On Android, sentry-java 8.60.0 (getsentry/sentry-java#6146) adds it to the device context thatserializeScopereturns.This pull request must merge after the bump to sentry-android 8.60.0.
💡 Motivation and Context
Closes #4256
💚 How did you test it?
A unit test in
devicecontext.test.tsmakes sure that both fields reach the event device context from the native layer.📝 Checklist
sendDefaultPIIis enabled.🤖 Generated with Claude Code