Skip to content

Refacto : add a Transport class #123

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.

Sign up for GitHub

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

Merged
marandaneto merged 40 commits into from
Oct 23, 2020
Merged

Refacto : add a Transport class #123

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
a10d740
remove the SentryResponse class
rxlabz Oct 21, 2020
623c904
refactor : add a Transport class
rxlabz Oct 21, 2020
0dcb613
refactor : move the clock initialization to Transport
rxlabz Oct 21, 2020
c97c98b
- refactor : applyScope
rxlabz Oct 21, 2020
477fdac
changelog
rxlabz Oct 21, 2020
b8fb322
fix merge conflicts
rxlabz Oct 22, 2020
fd92e3f
Merge branch 'feature/unified-api' into ref/transport
rxlabz Oct 22, 2020
0944c68
move the headers definition strategy to transport
rxlabz Oct 22, 2020
46829fc
- Instantiate Transport from options
rxlabz Oct 22, 2020
01d4e19
handle event dropping in event processors
rxlabz Oct 22, 2020
fbd001e
fix client._applyScope : merge event and scope tags and extra
rxlabz Oct 22, 2020
4d39531
remove client.toString()
rxlabz Oct 22, 2020
34fb528
fix a test
rxlabz Oct 22, 2020
2437fa4
Merge branch 'feature/unified-api' into ref/transport
rxlabz Oct 22, 2020
57798f5
handle the event serialization in the transport layer
rxlabz Oct 22, 2020
789e154
Merge remote-tracking branch 'origin/feature/unified-api' into ref/tr…
rxlabz Oct 22, 2020
1aa05d6
remove stackFrameFilter references
rxlabz Oct 22, 2020
9895142
Merge remote-tracking branch 'origin/feature/unified-api' into ref/tr…
rxlabz Oct 22, 2020
cb5e309
add a CredentialBuilder
rxlabz Oct 22, 2020
e563d2c
- headers configuration
rxlabz Oct 23, 2020
71b53c8
- fix scope.level precedence
rxlabz Oct 23, 2020
be3f0a2
buildHeaders and fix timestamp test
rxlabz Oct 23, 2020
43b798c
- add a isWeb helper to define the event.platform
rxlabz Oct 23, 2020
19dc91d
feat: apply sample rate
marandaneto Oct 23, 2020
cec1f21
changelog
marandaneto Oct 23, 2020
909d4f9
- add a isWeb helper to define the event.platform and sdkName
rxlabz Oct 23, 2020
8b600dc
- add a isWeb helper to define the event.platform and sdkName
rxlabz Oct 23, 2020
4ff58a6
Merge remote-tracking branch 'origin/feat/sample-rate' into ref/trans…
rxlabz Oct 23, 2020
5493e19
fix
marandaneto Oct 23, 2020
621d6c4
update tests
rxlabz Oct 23, 2020
e11f653
Merge remote-tracking branch 'origin/feat/sample-rate' into ref/trans…
rxlabz Oct 23, 2020
88526e2
remove the platform arg from Client ctor
rxlabz Oct 23, 2020
796b633
set client.options private
rxlabz Oct 23, 2020
fb8b9e0
remove event.platform redefinition
rxlabz Oct 23, 2020
1dbcba4
set options.sdk if null
rxlabz Oct 23, 2020
8b56bce
remove fields from Transport Api
rxlabz Oct 23, 2020
daeb29e
private version consts
rxlabz Oct 23, 2020
098d58c
remove transport from the Client ctor
rxlabz Oct 23, 2020
2833a94
remove options.environmentAttributes
rxlabz Oct 23, 2020
35671d4
event processing : platform
rxlabz Oct 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
- add a isWeb helper to define the event.platform and sdkName 
- remove projectId from sended data
  • Loading branch information
@rxlabz
rxlabz committed Oct 23, 2020
commit 909d4f9aab6775250dfaf803eaf9dba59c31bf65
3 changes: 1 addition & 2 deletions dart/lib/src/browser_client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:http/browser_client.dart';
import 'package:meta/meta.dart';

import 'client.dart';
import 'protocol.dart';
import 'sentry_options.dart';
import 'transport/transport.dart';
import 'version.dart';
Expand Down Expand Up @@ -50,7 +49,7 @@ class SentryBrowserClient extends SentryClient {
options,
transport: Transport(
options: options,
sdk: Sdk(name: browserSdkName, version: sdkVersion),
sdkIdentifier: '${browserSdkName}/${sdkVersion}',
platform: platform,
Copy link
Contributor

@marandaneto marandaneto Oct 22, 2020

Choose a reason for hiding this comment

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

wondering if this is the same as options.sentryClientName, could you check it?

Copy link
Contributor Author

@rxlabz rxlabz Oct 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

platform is 'dart' or 'javascript' , according to the docs options.sentryClientName is 'sentry.{language}.{platform}/{version}'

Copy link
Contributor

@marandaneto marandaneto Oct 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

we could do this then?

factory SentryIOClient(SentryOptions options) {
    options.platform = sdkPlatform;
    SentryIOClient._(options);
}

same for the WebClient, so we move this flag to the options and the transport don't need to take it as a param? we do it automatically.

Copy link
Contributor Author

@rxlabz rxlabz Oct 23, 2020

Choose a reason for hiding this comment

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

✅ platform is now defined at runtime

origin: origin,
),
Expand Down
9 changes: 5 additions & 4 deletions dart/lib/src/io_client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:sentry/sentry.dart';
import 'package:sentry/src/transport/transport.dart';

import 'client.dart';
import 'protocol.dart';

SentryClient createSentryClient(SentryOptions options) =>
SentryIOClient(options);
Expand All @@ -20,11 +19,13 @@ class SentryIOClient extends SentryClient {
factory SentryIOClient(SentryOptions options) =>
SentryIOClient._(options, platform: sdkPlatform);

static const sdk = Sdk(name: sdkName, version: sdkVersion);

SentryIOClient._(SentryOptions options, {@required String platform})
: super.base(
options,
transport: Transport(options: options, sdk: sdk, platform: platform),
transport: Transport(
options: options,
sdkIdentifier: '${sdkName}/${sdkVersion}',
platform: platform,
),
);
}
5 changes: 3 additions & 2 deletions dart/lib/src/protocol/sentry_event.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class SentryEvent {
SentryId eventId,
DateTime timestamp,
String platform,
Sdk sdk,
this.logger,
this.serverName,
this.release,
Expand All @@ -31,10 +32,10 @@ class SentryEvent {
this.userContext,
this.contexts,
this.breadcrumbs,
this.sdk,
}) : eventId = eventId ?? SentryId.newId(),
platform = platform ?? (isWeb ? browserPlatform : sdkPlatform),
Copy link
Contributor

@marandaneto marandaneto Oct 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

if this is going to be set here now, why do we need this on the client/transport? I believe this should be done on the Client, like Java does (via MainEventProcessor)

Copy link
Contributor Author

@rxlabz rxlabz Oct 23, 2020

Choose a reason for hiding this comment

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

✅ I moved it to the client.

timestamp = timestamp ?? getUtcDateTime();
timestamp = timestamp ?? getUtcDateTime(),
sdk = sdk ?? Sdk(name: sdkName, version: sdkVersion);

/// Refers to the default fingerprinting algorithm.
///
Expand Down
8 changes: 4 additions & 4 deletions dart/lib/src/transport/transport.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,23 @@ class Transport {
/// Used by sentry to differentiate browser from io environment
final String platform;

final Sdk sdk;
final String sdkIdentifier;

CredentialBuilder _credentialBuilder;

final Map<String, String> _headers;

Transport({
@required SentryOptions options,
@required this.sdk,
@required this.sdkIdentifier,
@required this.platform,
this.origin,
}) : _options = options,
dsn = Dsn.parse(options.dsn),
_headers = _buildHeaders(sdkIdentifier: sdk.identifier) {
_headers = _buildHeaders(sdkIdentifier: sdkIdentifier) {
_credentialBuilder = CredentialBuilder(
dsn: Dsn.parse(options.dsn),
clientId: sdk.identifier,
clientId: sdkIdentifier,
clock: options.clock,
);
}
Expand Down
6 changes: 5 additions & 1 deletion dart/lib/src/version.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,15 @@
/// This library contains Sentry.io SDK constants used by this package.
library version;

import 'utils.dart';

/// The SDK version reported to Sentry.io in the submitted events.
const String sdkVersion = '4.0.0';

String get sdkName => isWeb ? browserSdkName : ioSdkName;
Copy link
Contributor

@marandaneto marandaneto Oct 23, 2020

Choose a reason for hiding this comment

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

we can make those const private now, if what matters is the sdkName

Copy link
Contributor Author

@rxlabz rxlabz Oct 23, 2020

Choose a reason for hiding this comment

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

✅


/// The default SDK name reported to Sentry.io in the submitted events.
const String sdkName = 'sentry.dart';
const String ioSdkName = 'sentry.dart';

/// The SDK name for web projects reported to Sentry.io in the submitted events.
const String browserSdkName = 'sentry.dart.browser';
Expand Down
2 changes: 1 addition & 1 deletion dart/test/event_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void main() {
'platform': isWeb ? 'javascript' : 'dart',
'event_id': '00000000000000000000000000000000',
'timestamp': '2019-01-01T00:00:00',
'sdk': {'version': sdkVersion, 'name': 'sentry.dart'},
'sdk': {'version': sdkVersion, 'name': sdkName},
'message': {
'formatted': 'test-message 1 2',
'message': 'test-message %d %d',
Expand Down
2 changes: 1 addition & 1 deletion dart/test/test_utils.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ Future testCaptureException(
'event_id': sentryId.toString(),
'timestamp': '2017-01-02T00:00:00',
'platform': 'javascript',
'sdk': {'version': sdkVersion, 'name': 'sentry.dart'},
'sdk': {'version': sdkVersion, 'name': sdkName},
'server_name': 'test.server.com',
'release': '1.2.3',
'environment': 'staging',
Expand Down
3 changes: 2 additions & 1 deletion flutter/example/lib/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ Future<void> main() async {
stackTrace: stackTrace,
// release is required on Web to match the source maps
release: _release,
sdk: const Sdk(name: sdkName, version: sdkVersion),

// sdk: const Sdk(name: sdkName, version: sdkVersion),
);
await _sentry.captureEvent(event);
});
Expand Down