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

Improve public facing API for creating Baggage from header #2284

Merged
merged 3 commits into from
Oct 10, 2022

Conversation

adinauer
Copy link
Member

📜 Description

Remove ILogger param from public facing API when creating a Baggage object from header String(s).

💡 Motivation and Context

Retrieving options / logger is only possible using methods marked as @ApiStatus.Internal so instead we skip the param and retrieve it inside using HubAdapter.getInstance().getOptions().getLogger().

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Oct 10, 2022

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

Generated by 🚫 dangerJS against 4f3decd

@github-actions
Copy link
Contributor

github-actions bot commented Oct 10, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 358.94 ms 390.65 ms 31.71 ms
Size 1.73 MiB 2.29 MiB 579.50 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
54cebc8 331.12 ms 385.14 ms 54.02 ms
2f079a1 296.91 ms 337.43 ms 40.51 ms
4dd88fe 302.12 ms 331.17 ms 29.04 ms
1e4690d 354.69 ms 387.88 ms 33.19 ms
4dd88fe 306.88 ms 391.58 ms 84.70 ms
3d89dea 345.59 ms 364.06 ms 18.47 ms
c5ccd8a 329.98 ms 365.52 ms 35.54 ms
7300956 324.20 ms 353.79 ms 29.58 ms
7300956 337.57 ms 384.21 ms 46.64 ms
d4087ee 278.00 ms 313.86 ms 35.86 ms

App size

Revision Plain With Sentry Diff
54cebc8 1.73 MiB 2.29 MiB 579.43 KiB
2f079a1 1.74 MiB 2.33 MiB 605.56 KiB
4dd88fe 1.73 MiB 2.29 MiB 579.50 KiB
1e4690d 1.74 MiB 2.33 MiB 604.92 KiB
4dd88fe 1.73 MiB 2.29 MiB 579.50 KiB
3d89dea 1.74 MiB 2.33 MiB 604.92 KiB
c5ccd8a 1.74 MiB 2.33 MiB 607.44 KiB
7300956 1.73 MiB 2.29 MiB 578.69 KiB
7300956 1.73 MiB 2.29 MiB 578.69 KiB
d4087ee 1.73 MiB 2.29 MiB 579.50 KiB

Previous results on branch: feat/improve-public-facing-baggage-api

Startup times

Revision Plain With Sentry Diff
ad0fcc7 350.14 ms 364.16 ms 14.02 ms

App size

Revision Plain With Sentry Diff
ad0fcc7 1.73 MiB 2.29 MiB 579.50 KiB

@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2022

Codecov Report

Base: 80.10% // Head: 80.08% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (800b00b) compared to base (4dd88fe).
Patch coverage: 20.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2284      +/-   ##
============================================
- Coverage     80.10%   80.08%   -0.03%     
  Complexity     3423     3423              
============================================
  Files           242      242              
  Lines         12692    12696       +4     
  Branches       1698     1698              
============================================
  Hits          10167    10167              
- Misses         1882     1886       +4     
  Partials        643      643              
Impacted Files Coverage Δ
sentry/src/main/java/io/sentry/Baggage.java 80.00% <20.00%> (-2.12%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

CHANGELOG.md Outdated Show resolved Hide resolved
@adinauer adinauer merged commit ece81f7 into main Oct 10, 2022
@adinauer adinauer deleted the feat/improve-public-facing-baggage-api branch October 10, 2022 14:14
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.

3 participants