Skip to content

Tracing for File IO integration #1160

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 24 commits into from
Dec 6, 2022
Merged

Tracing for File IO integration #1160

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
85df9db
Tracing for File IO integration
marandaneto Dec 2, 2022
0e48d00
add file workflow
marandaneto Dec 2, 2022
0f74b5b
fix pr id
marandaneto Dec 2, 2022
2a999cd
copy tests
marandaneto Dec 2, 2022
2f372ac
add tests for create
marandaneto Dec 2, 2022
d6a5de6
fix
marandaneto Dec 2, 2022
ac4d4dc
fixes
marandaneto Dec 2, 2022
05ea2c9
add missing tests
marandaneto Dec 5, 2022
153813b
remove
marandaneto Dec 5, 2022
1312453
add extension
marandaneto Dec 5, 2022
244a6f4
fixes
marandaneto Dec 5, 2022
00cd84a
add example
marandaneto Dec 5, 2022
5ce347d
add readme
marandaneto Dec 5, 2022
74a42f1
fixes comments
marandaneto Dec 5, 2022
52bf20d
enable strict analyzer
marandaneto Dec 5, 2022
e39ecc7
add integration
marandaneto Dec 5, 2022
1841669
add @experimental annotation
marandaneto Dec 5, 2022
a794e14
overwrite pana threshold
marandaneto Dec 5, 2022
6fe9687
Update .craft.yml
marandaneto Dec 5, 2022
7b46c5c
remove not needed items from gh action
marandaneto Dec 6, 2022
35215e2
Merge branch 'feat/sentry_file' of github.com:getsentry/sentry-dart i…
marandaneto Dec 6, 2022
88b6194
add dart file to min test
marandaneto Dec 6, 2022
964eb27
add example
marandaneto Dec 6, 2022
de0ca3c
remove example
marandaneto Dec 6, 2022
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
fixes
  • Loading branch information
@marandaneto
marandaneto committed Dec 2, 2022
commit ac4d4dc926e3678fab8170da90cb85798c5eed05
3 changes: 2 additions & 1 deletion .github/workflows/file.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
sdk: [stable, beta]
# removing beta because of Dart 2.19.0
sdk: [stable]
exclude:
- os: macos-latest
sdk: beta
Expand Down
94 changes: 64 additions & 30 deletions file/lib/src/sentry_file.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

// ignore_for_file: invalid_use_of_internal_member

import 'dart:async';
import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';
import 'package:meta/meta.dart';
import 'package:sentry/sentry.dart';

typedef Callback<T> = T Function();
typedef Callback<T> = FutureOr<T> Function();

class SentryFile implements File {
SentryFile(
Expand All @@ -21,7 +22,7 @@ class SentryFile implements File {

@override
Future<File> copy(String newPath) {
return _wrap(_file.copy(newPath), 'file.copy');
return _wrap(() async => _file.copy(newPath), 'file.copy');
}

@override
Expand All @@ -32,7 +33,7 @@ class SentryFile implements File {
@override
Future<File> create({bool recursive = false}) {
return _wrap(
_file.create(recursive: recursive),
() async => _file.create(recursive: recursive),
'file.write',
);
}
Expand All @@ -47,7 +48,7 @@ class SentryFile implements File {

@override
Future<FileSystemEntity> delete({bool recursive = false}) {
return _wrap(_file.delete(recursive: recursive), 'file.delete');
return _wrap(() async => _file.delete(recursive: recursive), 'file.delete');
}

@override
Expand All @@ -57,7 +58,7 @@ class SentryFile implements File {

@override
Future<RandomAccessFile> open({FileMode mode = FileMode.read}) {
return _wrap(_file.open(mode: mode), 'file.open');
return _wrap(() async => _file.open(mode: mode), 'file.open');
}

// coverage:ignore-start
Expand All @@ -81,7 +82,7 @@ class SentryFile implements File {

@override
Future<Uint8List> readAsBytes() {
return _wrap(_file.readAsBytes(), 'file.read');
return _wrap(() async => _file.readAsBytes(), 'file.read');
}

@override
Expand All @@ -91,7 +92,8 @@ class SentryFile implements File {

@override
Future<List<String>> readAsLines({Encoding encoding = utf8}) {
return _wrap(_file.readAsLines(encoding: encoding), 'file.read');
return _wrap(
() async => _file.readAsLines(encoding: encoding), 'file.read');
}

@override
Expand All @@ -104,7 +106,8 @@ class SentryFile implements File {

@override
Future<String> readAsString({Encoding encoding = utf8}) {
return _wrap(_file.readAsString(encoding: encoding), 'file.read');
return _wrap(
() async => _file.readAsString(encoding: encoding), 'file.read');
}

@override
Expand All @@ -117,7 +120,7 @@ class SentryFile implements File {

@override
Future<File> rename(String newPath) {
return _wrap(_file.rename(newPath), 'file.rename');
return _wrap(() async => _file.rename(newPath), 'file.rename');
}

@override
Expand All @@ -132,7 +135,7 @@ class SentryFile implements File {
bool flush = false,
}) {
return _wrap(
_file.writeAsBytes(bytes, mode: mode, flush: flush),
() async => _file.writeAsBytes(bytes, mode: mode, flush: flush),
'file.write',
);
}
Expand All @@ -157,7 +160,7 @@ class SentryFile implements File {
bool flush = false,
}) {
return _wrap(
_file.writeAsString(
() async => _file.writeAsString(
contents,
mode: mode,
encoding: encoding,
Expand Down Expand Up @@ -185,25 +188,11 @@ class SentryFile implements File {
);
}

void _setSize(ISentrySpan? span, dynamic data) {
// method that returns null dont have a size
if (data == null) {
return;
}
if (data is List<int>) {
span?.setData('file.size', data.length);
} else if (data is File) {
span?.setData('file.size', data.lengthSync());
// TODO: if its a copy, we need the new path here too or not?
// TODO: append size in bytes or human readable size
}
}

String _getDesc() {
return uri.pathSegments.isNotEmpty ? uri.pathSegments.last : path;
}

Future<T> _wrap<T>(Future<T> future, String operation) async {
Future<T> _wrap<T>(Callback<T> callback, String operation) async {
final desc = _getDesc();

final currentSpan = _hub.getSpan();
Expand All @@ -215,8 +204,30 @@ class SentryFile implements File {
}
T data;
try {
data = await future;
_setSize(span, data);
// workaround for having the length when the file does not exist
// exists or its being deleted.
int? length;
var hasLength = false;
try {
length = await _file.length();
hasLength = true;
} catch (_) {
// ignore in case something goes wrong
}

data = await callback();

if (!hasLength) {
try {
length = await _file.length();
} catch (_) {
// ignore in case something goes wrong
}
}

if (length != null) {
span?.setData('file.size', length);
}

span?.status = SpanStatus.ok();
} catch (exception) {
Expand All @@ -239,10 +250,33 @@ class SentryFile implements File {
if (_hub.options.sendDefaultPii) {
span?.setData('file.path', absolute.path);
}

T data;
try {
data = callback();
_setSize(span, data);
// workaround for having the length when the file does not exist
// exists or its being deleted.
int? length;
var hasLength = false;
try {
length = _file.lengthSync();
hasLength = true;
} catch (_) {
// ignore in case something goes wrong
}

data = callback() as T;

if (!hasLength) {
Copy link
Contributor

@krystofwoldrich krystofwoldrich Dec 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

L: What if we are overriding the content of the file do we want to know the length before or after?
If I'm looking correctly at the moment we would get the length before.

Copy link
Contributor Author

@marandaneto marandaneto Dec 6, 2022

Choose a reason for hiding this comment

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

The data can only be read before, because after, the file can be deleted, renamed, moved, so it's a trade-off we made.
Maybe I should point that out in the docs.

try {
length = _file.lengthSync();
} catch (_) {
// ignore in case something goes wrong
}
}

if (length != null) {
span?.setData('file.size', length);
}

span?.status = SpanStatus.ok();
} catch (exception) {
Expand Down
Loading