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

Implement envelope based transport #391

Merged
merged 95 commits into from
Jun 8, 2021

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Mar 29, 2021

📜 Description

  • Implemented envelope based transport of events.

Open Questions

  • Can we remove file based envelope transport? -> No
  • Should hints be implemented as well? -> Later
  • Do we need to update interface of hubs? -> No

💡 Motivation and Context

With envelopes, we can implement features like feedback and attachments.

Closes #344

💚 How did you test it?

📝 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

  • Update accordingly when open questions are answered...

@denrase denrase self-assigned this Mar 29, 2021
@codecov-io
Copy link

codecov-io commented Mar 29, 2021

Codecov Report

Merging #391 (94c2099) into main (4d42b29) will increase coverage by 2.41%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #391      +/-   ##
==========================================
+ Coverage   90.74%   93.16%   +2.41%     
==========================================
  Files          52        5      -47     
  Lines        1654      161    -1493     
==========================================
- Hits         1501      150    -1351     
+ Misses        153       11     -142     
Impacted Files Coverage Δ
flutter/lib/src/sentry_flutter.dart 93.54% <ø> (ø)
flutter/lib/src/file_system_transport.dart 100.00% <100.00%> (ø)
dart/lib/src/throwable_mechanism.dart
dart/lib/src/protocol/debug_meta.dart
dart/lib/src/protocol/sdk_version.dart
dart/lib/src/protocol/sentry_browser.dart
dart/lib/src/protocol/sentry_message.dart
dart/lib/src/hub.dart
dart/lib/src/protocol/sentry_package.dart
dart/lib/src/protocol/sentry_device.dart
... and 35 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 4d42b29...94c2099. Read the comment docs.

@denrase
Copy link
Collaborator Author

denrase commented May 25, 2021

@marandaneto Fixed

CHANGELOG.md Outdated Show resolved Hide resolved
@marandaneto
Copy link
Contributor

@denrase I'd just figure out if we can do something about https://github.com/getsentry/sentry-dart/pull/391/files#r639644591 otherwise ready to go, thanks for all the work, this will unblock us in so many ways :)

@denrase
Copy link
Collaborator Author

denrase commented May 28, 2021

@marandaneto We now use PrivateSentrySDKOnly exposed method instead of accessing private API, thx @philipphofmann for the PR! 🙇

CHANGELOG.md Outdated Show resolved Hide resolved
@marandaneto
Copy link
Contributor

@denrase could you fix the conflicts? we are ready to merge if we solve the open comments + conflicts, thanks a lot

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

@philipphofmann
Copy link
Member

@marandaneto We now use PrivateSentrySDKOnly exposed method instead of accessing private API, thx @philipphofmann for the PR! 🙇

No problem @denrase. The thing with the private headers can break easily and we needed to improve this.

@ueman ueman added the 6.0.0 label Jun 6, 2021
@marandaneto
Copy link
Contributor

@denrase 5.1 has been released so we can merge to main, would you mind fixing the conflicts last time? sorry about that :(
feel free to press green too ^^

@denrase
Copy link
Collaborator Author

denrase commented Jun 8, 2021

@marandaneto Will do, thanks!

denrase and others added 4 commits June 8, 2021 09:18
# Conflicts:
#	flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt
#	flutter/ios/Classes/SentryFlutterPluginApple.swift
…pe-based-transport

# Conflicts:
#	dart/test/protocol/contexts_test.dart
# Conflicts:
#	dart/test/protocol/contexts_test.dart
…hub.com/getsentry/sentry-dart into feat/implement-envelope-based-transport

# Conflicts:
#	dart/test/protocol/contexts_test.dart
@denrase
Copy link
Collaborator Author

denrase commented Jun 8, 2021

@ueman Ah, was not done yet and also just merging in main

@denrase
Copy link
Collaborator Author

denrase commented Jun 8, 2021

Think we are ready! Thanks everyone for the support and feedback! 🙇

@denrase denrase merged commit 8024a6c into main Jun 8, 2021
@denrase denrase deleted the feat/implement-envelope-based-transport branch June 8, 2021 08:27
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.

Implement Envelope based transport
8 participants