Skip to content

Conversation

@marandaneto
Copy link
Contributor

@marandaneto marandaneto commented Sep 29, 2021

📜 Description

Feat: Sentry Performance for HTTP client

💡 Motivation and Context

Closes #595

💚 How did you test it?

running and unit tests

📝 Checklist

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

🔮 Next steps

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2021

Codecov Report

Merging #603 (9ebaad7) into main (d07e63a) will increase coverage by 1.71%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #603      +/-   ##
==========================================
+ Coverage   90.38%   92.09%   +1.71%     
==========================================
  Files          89        7      -82     
  Lines        2953      253    -2700     
==========================================
- Hits         2669      233    -2436     
+ Misses        284       20     -264     
Impacted Files Coverage Δ
dart/lib/src/integration.dart
.../lib/src/enricher/io_enricher_event_processor.dart
dart/lib/src/hub_adapter.dart
...art/lib/src/enricher/enricher_event_processor.dart
dart/lib/src/protocol/sentry_event.dart
dart/lib/src/isolate_error_integration.dart
dart/lib/src/protocol/span_id.dart
...event_processor/deduplication_event_processor.dart
dart/lib/src/protocol/sentry_transaction.dart
dart/lib/src/platform/_io_platform.dart
... and 63 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d07e63a...9ebaad7. Read the comment docs.

@marandaneto marandaneto marked this pull request as ready for review September 30, 2021 09:14
@marandaneto marandaneto requested review from a team and ueman September 30, 2021 09:15
Copy link
Collaborator

@ueman ueman left a comment

Choose a reason for hiding this comment

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

LGTM
Though there are some failing tests it seems

@ueman
Copy link
Collaborator

ueman commented Sep 30, 2021

Isn't there a SentryOption which decides wether tracing is enabled or not?

@marandaneto
Copy link
Contributor Author

Isn't there a SentryOption which decides wether tracing is enabled or not?

yes, but SentryHttpClient nor Hub have access to the SentryOptions, Java exposed Hub#getOptions as workaround, we can do that maybe later, I'd not block this PR right now tho

@marandaneto marandaneto merged commit 4eb2218 into main Sep 30, 2021
@marandaneto marandaneto deleted the feat/sentry-header branch September 30, 2021 13:25
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.

Add traceHeaders support for performance API

4 participants