Skip to content

(feat-V7) Log Tracing #4827

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.

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

Open
wants to merge 22 commits into
base: v7
Choose a base branch
from
Open

(feat-V7) Log Tracing #4827

wants to merge 22 commits into from

Conversation

lucas-zimerman
Copy link
Collaborator

@lucas-zimerman lucas-zimerman commented May 13, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

Blocked by:

📜 Description

Add the required changes on the Client in order to capture logs. All platforms will be supported with the initial PR (web, iOS and Android)

💡 Motivation and Context

Fixes #4856

💚 How did you test it?

Locally, with native SDKs disabled for testing the JavaScript integration, and with native SDKs enabled to test the Native integration

Pii

Pii seems to be filtered serverside so we don't need to do anything on the SDK.
image

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

  • Implement tests
  • Decide what todo with internal logs sent to logger. (Doesn't seems to be sent)
  • Redirect BeforeLog from native to JS Layer
  • Add documentation.

@lucas-zimerman lucas-zimerman changed the base branch from main to v7 May 13, 2025 14:22
Copy link
Contributor

github-actions bot commented May 13, 2025

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 473.64 ms 463.42 ms -10.22 ms
Size 17.75 MiB 19.58 MiB 1.83 MiB

Baseline results on branch: v7

Startup times

Revision Plain With Sentry Diff
9fbcfaf 420.06 ms 435.31 ms 15.25 ms
9da5c4e 478.08 ms 467.46 ms -10.63 ms
fb6493a 446.78 ms 441.65 ms -5.13 ms
8a54dff 482.57 ms 467.16 ms -15.41 ms

App size

Revision Plain With Sentry Diff
9fbcfaf 17.75 MiB 19.54 MiB 1.79 MiB
9da5c4e 17.75 MiB 20.16 MiB 2.41 MiB
fb6493a 17.75 MiB 19.58 MiB 1.83 MiB
8a54dff 17.75 MiB 19.54 MiB 1.80 MiB

Previous results on branch: lz/v7-test-logs

Startup times

Revision Plain With Sentry Diff
b592bbd 489.78 ms 487.25 ms -2.53 ms
ee79082 439.09 ms 481.84 ms 42.75 ms

App size

Revision Plain With Sentry Diff
b592bbd 17.75 MiB 19.56 MiB 1.82 MiB
ee79082 17.75 MiB 19.56 MiB 1.82 MiB

Copy link
Contributor

github-actions bot commented May 13, 2025

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1224.33 ms 1228.20 ms 3.88 ms
Size 2.63 MiB 3.78 MiB 1.15 MiB

Baseline results on branch: v7

Startup times

Revision Plain With Sentry Diff
9da5c4e+dirty 1231.84 ms 1235.49 ms 3.65 ms
8a54dff+dirty 1226.17 ms 1227.79 ms 1.63 ms
9fbcfaf+dirty 1216.66 ms 1212.06 ms -4.60 ms
fb6493a+dirty 1222.42 ms 1235.50 ms 13.08 ms

App size

Revision Plain With Sentry Diff
9da5c4e+dirty 2.63 MiB 3.76 MiB 1.13 MiB
8a54dff+dirty 2.63 MiB 3.79 MiB 1.16 MiB
9fbcfaf+dirty 2.63 MiB 3.77 MiB 1.14 MiB
fb6493a+dirty 2.63 MiB 3.78 MiB 1.15 MiB

Previous results on branch: lz/v7-test-logs

Startup times

Revision Plain With Sentry Diff
ee79082+dirty 1217.93 ms 1221.08 ms 3.15 ms
b592bbd+dirty 1219.21 ms 1232.27 ms 13.06 ms

App size

Revision Plain With Sentry Diff
ee79082+dirty 2.63 MiB 3.76 MiB 1.13 MiB
b592bbd+dirty 2.63 MiB 3.76 MiB 1.13 MiB

Copy link
Contributor

github-actions bot commented May 13, 2025

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1231.61 ms 1234.17 ms 2.56 ms
Size 3.19 MiB 4.35 MiB 1.16 MiB

Baseline results on branch: v7

Startup times

Revision Plain With Sentry Diff
9da5c4e+dirty 1215.41 ms 1226.38 ms 10.97 ms
8a54dff+dirty 1244.29 ms 1254.71 ms 10.43 ms
9fbcfaf+dirty 1215.22 ms 1222.26 ms 7.04 ms
fb6493a+dirty 1229.61 ms 1221.21 ms -8.40 ms

App size

Revision Plain With Sentry Diff
9da5c4e+dirty 3.19 MiB 4.33 MiB 1.14 MiB
8a54dff+dirty 3.19 MiB 4.35 MiB 1.17 MiB
9fbcfaf+dirty 3.19 MiB 4.34 MiB 1.15 MiB
fb6493a+dirty 3.19 MiB 4.35 MiB 1.16 MiB

Previous results on branch: lz/v7-test-logs

Startup times

Revision Plain With Sentry Diff
ee79082+dirty 1223.58 ms 1225.13 ms 1.54 ms
b592bbd+dirty 1218.20 ms 1229.35 ms 11.14 ms

App size

Revision Plain With Sentry Diff
ee79082+dirty 3.19 MiB 4.33 MiB 1.14 MiB
b592bbd+dirty 3.19 MiB 4.33 MiB 1.14 MiB

@lucas-zimerman
Copy link
Collaborator Author

image
logs are now correctly captured on Android,iOS and JS.

@lucas-zimerman
Copy link
Collaborator Author

build errors will be fixed on #4860

@@ -182,7 +182,7 @@ export const NATIVE: SentryNativeWrapper = {
typeof itemHeader.content_type === 'string' ? itemHeader.content_type : 'application/octet-stream';
bytesPayload = itemPayload;
} else {
bytesContentType = 'application/json';
bytesContentType = 'application/vnd.sentry.items.log+json';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should we use the new content type for all requests or only the ones that contain logs? @krystofwoldrich @kahest
This can create a break changes on self -hosted, and I am not sure which version has support for logging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

on Sentry JavaScript for example, they use application/vnd.sentry.items.log+json by default on all requests

Copy link
Contributor

github-actions bot commented May 27, 2025

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 415.21 ms 441.44 ms 26.22 ms
Size 7.15 MiB 8.34 MiB 1.18 MiB

Baseline results on branch: v7

Startup times

Revision Plain With Sentry Diff
9fbcfaf+dirty 417.92 ms 431.81 ms 13.90 ms
fb6493a+dirty 352.12 ms 372.92 ms 20.80 ms
8a54dff+dirty 393.65 ms 392.32 ms -1.33 ms
9da5c4e+dirty 399.70 ms 407.34 ms 7.64 ms

App size

Revision Plain With Sentry Diff
9fbcfaf+dirty 7.15 MiB 8.29 MiB 1.14 MiB
fb6493a+dirty 7.15 MiB 8.33 MiB 1.18 MiB
8a54dff+dirty 7.15 MiB 8.30 MiB 1.15 MiB
9da5c4e+dirty 7.15 MiB 8.42 MiB 1.27 MiB

Previous results on branch: lz/v7-test-logs

Startup times

Revision Plain With Sentry Diff
ee79082+dirty 431.77 ms 430.77 ms -1.00 ms
b592bbd+dirty 457.76 ms 478.40 ms 20.64 ms

App size

Revision Plain With Sentry Diff
ee79082+dirty 7.15 MiB 8.32 MiB 1.17 MiB
b592bbd+dirty 7.15 MiB 8.32 MiB 1.17 MiB

CHANGELOG.md Outdated
@@ -61,6 +84,10 @@ Version 7 of the SDK is compatible with Sentry self-hosted versions 24.4.2 or hi
- `@sentry/utils` package, the exports were moved to `@sentry/core`
- Standalone `Client` interface & deprecate `BaseClient`

### Self Hosted

- It is recommended to use Sentry Self Hosted version `24.8.0` this and future versions.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I validated that capture of events/exceptions are working on 24.8.0, but we should at least recommend the first version suggested here getsentry/self-hosted#3560

CHANGELOG.md Outdated
@@ -61,6 +84,10 @@ Version 7 of the SDK is compatible with Sentry self-hosted versions 24.4.2 or hi
- `@sentry/utils` package, the exports were moved to `@sentry/core`
- Standalone `Client` interface & deprecate `BaseClient`

### Self Hosted

- It is recommended to use Sentry Self Hosted version `25.2.0` or new for React Native V7 or newer
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this change affects all events/transactions due to the content type of the request being changed.

25.2.0 is a safe value for it getsentry/self-hosted#3560, even if we don't officially support logs, it'll not drop other types of content

@lucas-zimerman lucas-zimerman changed the title (WIP) Log Tracing (feat-V7) Log Tracing May 27, 2025
@lucas-zimerman lucas-zimerman marked this pull request as ready for review May 28, 2025 10:28
@lucas-zimerman
Copy link
Collaborator Author

lucas-zimerman commented May 28, 2025

@krystofwoldrich @antonis it's ready for review :D

Comment on lines +19 to +36
To enable it add the following code to your Sentry Options:

```typescript
_experiments: {
enableLogs: true,
},
```

You can also filter the logs being collected by adding beforeSendLogs into `_experiments`

```typescript
_experiments: {
enableLogs: true,
beforeSendLog: (log) => {
return log;
},
},
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I think we should align one tab to the right

Copy link
Collaborator

@antonis antonis left a comment

Choose a reason for hiding this comment

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

LGTM and worked as expected in my tests 🎸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants