Skip to content
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

Emit transaction.data in contexts.trace.data #2284

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Sep 12, 2024

📜 Description

Adds the transaction data to contexts.trace as well.

💡 Motivation and Context

As preparation for extracting metrics from transaction data.

Closes #2257

💚 How did you test it?

Unit test, spotlight

📝 Checklist

  • I reviewed submitted code
  • 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
  • All tests passing
  • No breaking changes

🔮 Next steps

Copy link
Contributor

github-actions bot commented Sep 12, 2024

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against c6acda5

Copy link

codecov bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.34%. Comparing base (3cc09ec) to head (c6acda5).

Files with missing lines Patch % Lines
dart/lib/src/protocol/sentry_trace_context.dart 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2284      +/-   ##
==========================================
- Coverage   88.35%   88.34%   -0.01%     
==========================================
  Files         247      247              
  Lines        8578     8583       +5     
==========================================
+ Hits         7579     7583       +4     
- Misses        999     1000       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 492.48 ms 539.48 ms 47.00 ms
Size 6.49 MiB 7.55 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
64af39c 386.80 ms 471.11 ms 84.31 ms
0ceb89c 304.57 ms 357.18 ms 52.61 ms
be08ed1 361.37 ms 414.84 ms 53.47 ms
3f23617 385.77 ms 476.10 ms 90.33 ms
42f6e7e 308.71 ms 360.06 ms 51.35 ms
c70e01a 331.04 ms 401.46 ms 70.42 ms
9d7e862 426.35 ms 510.88 ms 84.53 ms
7f75f32 347.36 ms 419.58 ms 72.22 ms
86d4841 286.35 ms 372.43 ms 86.08 ms
3637a22 322.59 ms 390.00 ms 67.41 ms

App size

Revision Plain With Sentry Diff
64af39c 6.27 MiB 7.20 MiB 958.83 KiB
0ceb89c 5.94 MiB 6.95 MiB 1.01 MiB
be08ed1 6.33 MiB 7.26 MiB 946.42 KiB
3f23617 5.94 MiB 6.96 MiB 1.02 MiB
42f6e7e 6.27 MiB 7.20 MiB 956.06 KiB
c70e01a 5.94 MiB 6.97 MiB 1.03 MiB
9d7e862 6.33 MiB 7.26 MiB 943.41 KiB
7f75f32 6.26 MiB 7.20 MiB 959.18 KiB
86d4841 6.15 MiB 7.13 MiB 1000.49 KiB
3637a22 6.06 MiB 7.09 MiB 1.03 MiB

Copy link
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1255.10 ms 1263.20 ms 8.10 ms
Size 8.38 MiB 9.73 MiB 1.36 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8776cdf 1237.64 ms 1246.83 ms 9.20 ms
f79eecf 1210.25 ms 1221.65 ms 11.40 ms
abfcdb5 1230.87 ms 1244.94 ms 14.06 ms
9d7e862 1236.88 ms 1258.62 ms 21.74 ms
7e7f0b1 1230.52 ms 1251.49 ms 20.97 ms
3d305b9 1230.29 ms 1247.39 ms 17.10 ms
1596141 1230.77 ms 1241.90 ms 11.13 ms
d4d0807 1246.94 ms 1260.69 ms 13.75 ms
051e97a 1245.94 ms 1249.51 ms 3.57 ms
d089990 1206.19 ms 1233.08 ms 26.89 ms

App size

Revision Plain With Sentry Diff
8776cdf 8.33 MiB 9.40 MiB 1.07 MiB
f79eecf 8.29 MiB 9.36 MiB 1.07 MiB
abfcdb5 8.33 MiB 9.64 MiB 1.31 MiB
9d7e862 8.32 MiB 9.38 MiB 1.05 MiB
7e7f0b1 8.33 MiB 9.61 MiB 1.27 MiB
3d305b9 8.33 MiB 9.63 MiB 1.29 MiB
1596141 8.29 MiB 9.37 MiB 1.08 MiB
d4d0807 8.33 MiB 9.64 MiB 1.31 MiB
051e97a 8.28 MiB 9.34 MiB 1.06 MiB
d089990 8.33 MiB 9.40 MiB 1.07 MiB

Copy link
Collaborator

@martinhaintz martinhaintz left a comment

Choose a reason for hiding this comment

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

looks good.

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

Successfully merging this pull request may close these issues.

Emit transaction.data inside contexts.trace.data
2 participants