Skip to content

Breadcrumbs for database operations #1656

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
denrase merged 26 commits into from
Oct 31, 2023
Merged

Breadcrumbs for database operations #1656

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
347b8f4
Prepare breadcrumb tests
denrase Sep 19, 2023
12f0b6d
Merge branch 'main' into feat/database-breadcrumbs
denrase Sep 25, 2023
6de44bb
create breadcrumbs for batch operations
denrase Sep 25, 2023
bd9a135
add breadcrumbs to sentry database & executor
denrase Sep 25, 2023
2e6d025
create breadcrumb when opening db
denrase Sep 25, 2023
b7a2fbc
create breadcrumb in SentrySqfliteDatabaseFactory
denrase Sep 25, 2023
86d796e
choose correct breadcrumb in batch tests
denrase Sep 26, 2023
cfc7715
set additional error info in batch
denrase Sep 26, 2023
98ee0af
set query and error breadcrumb type
denrase Sep 26, 2023
a38cfd5
update in sqflite and database factory
denrase Sep 26, 2023
932ca1c
Merge branch 'main' into feat/database-breadcrumbs
denrase Sep 26, 2023
aa66f4a
run format
denrase Sep 26, 2023
0382383
add changelog entries
denrase Sep 26, 2023
2778594
bump ios im runtime
denrase Sep 26, 2023
9978a74
manually set deployemtn target for pods
denrase Oct 3, 2023
26a77fc
Merge branch 'main' into feat/database-breadcrumbs
denrase Oct 17, 2023
788364d
Change status value
denrase Oct 17, 2023
e58a4d8
Merge branch 'main' into feat/database-breadcrumbs
denrase Oct 24, 2023
af3d295
change breadcrumb type and level
denrase Oct 24, 2023
1a5d4e3
remove duplicated lines
denrase Oct 24, 2023
aaf22b4
set type query to success breadcrumbs
denrase Oct 24, 2023
24f79d4
Merge branch 'main' into feat/database-breadcrumbs
denrase Oct 30, 2023
73d740a
fix changelog
denrase Oct 30, 2023
2c353b2
remove remaining error level
denrase Oct 31, 2023
6830e50
Merge branch 'main' into feat/database-breadcrumbs
denrase Oct 31, 2023
a20d194
fix changelog
denrase Oct 31, 2023
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
set additional error info in batch
  • Loading branch information
@denrase
denrase committed Sep 26, 2023
commit cfc7715a97f1d43fd16f99a79f5df6c898f70d7a
41 changes: 25 additions & 16 deletions sqflite/lib/src/sentry_batch.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ class SentryBatch implements Batch {
span?.origin = SentryTraceOrigins.autoDbSqfliteBatch;
setDatabaseAttributeData(span, _dbName);

final Map<String, dynamic> breadcrumbData = {};
var breadcrumb = Breadcrumb(
message: _buffer.toString().trim(),
data: {},
);
setDatabaseAttributeOnBreadcrumb(breadcrumb, _dbName);

try {
final result = await _batch.apply(
Expand All @@ -65,22 +69,23 @@ class SentryBatch implements Batch {
);

span?.status = SpanStatus.ok();
breadcrumbData['status'] = 'ok';

breadcrumb.data?['status'] = 'ok';

return result;
} catch (exception) {
span?.throwable = exception;
span?.status = SpanStatus.internalError();
breadcrumbData['status'] = 'internalError';

breadcrumb.data?['status'] = 'internalError';
breadcrumb = breadcrumb.copyWith(
type: 'error',
Copy link
Contributor

@stefanosiano stefanosiano Oct 10, 2023

Choose a reason for hiding this comment

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

type shouldn't be error here

Copy link
Collaborator Author

@denrase denrase Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I checked in the documentation and error seemed appropriate, or am I missing something here?

https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/#breadcrumb-types

Do you have a suggestion which type to use instead?

Copy link
Contributor

@stefanosiano stefanosiano Oct 18, 2023

Choose a reason for hiding this comment

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

my understanding is the error type is for Exceptions.
Looking at what is done for Java OkHttp, we send a breadcrumb with type http, regardless the status code.
Let me ask the team and get back to you

Copy link
Contributor

@stefanosiano stefanosiano Oct 18, 2023

Choose a reason for hiding this comment

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

Confirmed: the query type is used to report a query is being executed. it has nothing to do with its result.
The error type is used for exceptions only
Also, we could change the level to warning, as the exception in the query does not necessarily means a crash of the app, or does it?

Copy link
Collaborator Author

@denrase denrase Oct 24, 2023

Choose a reason for hiding this comment

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

Thank you for clearing this up! Warning seems better yes, as users might handle these.

Copy link
Collaborator Author

@denrase denrase Oct 24, 2023

Choose a reason for hiding this comment

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

Ok updated them to query. Thinking if this is also correct in apply and commit, which are write operations. 🤔

Copy link
Contributor

@buenaflor buenaflor Oct 30, 2023

Choose a reason for hiding this comment

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

@stefanosiano any thoughts on that? not sure about that here

Copy link
Contributor

@stefanosiano stefanosiano Oct 31, 2023

Choose a reason for hiding this comment

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

yeah, I think it's fine, as they are still operations on the database

level: SentryLevel.error,
);

rethrow;
} finally {
await span?.finish();
final breadcrumb = Breadcrumb(
message: _buffer.toString().trim(),
data: breadcrumbData,
);
setDatabaseAttributeOnBreadcrumb(breadcrumb, _dbName);
// ignore: invalid_use_of_internal_member
await _hub.scope.addBreadcrumb(breadcrumb);
}
Expand All @@ -105,7 +110,11 @@ class SentryBatch implements Batch {
span?.origin = SentryTraceOrigins.autoDbSqfliteBatch;
setDatabaseAttributeData(span, _dbName);

final Map<String, dynamic> breadcrumbData = {};
var breadcrumb = Breadcrumb(
message: _buffer.toString().trim(),
data: {},
);
setDatabaseAttributeOnBreadcrumb(breadcrumb, _dbName);

try {
final result = await _batch.commit(
Expand All @@ -115,22 +124,22 @@ class SentryBatch implements Batch {
);

span?.status = SpanStatus.ok();
breadcrumbData['status'] = 'ok';
breadcrumb.data?['status'] = 'ok';

return result;
} catch (exception) {
span?.throwable = exception;
span?.status = SpanStatus.internalError();
breadcrumbData['status'] = 'internalError';

breadcrumb.data?['status'] = 'internalError';
breadcrumb = breadcrumb.copyWith(
type: 'error',
level: SentryLevel.error,
);

rethrow;
} finally {
await span?.finish();
final breadcrumb = Breadcrumb(
message: _buffer.toString().trim(),
data: breadcrumbData,
);
setDatabaseAttributeOnBreadcrumb(breadcrumb, _dbName);
// ignore: invalid_use_of_internal_member
await _hub.scope.addBreadcrumb(breadcrumb);
}
Expand Down
4 changes: 4 additions & 0 deletions sqflite/test/sentry_batch_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,8 @@ SELECT * FROM Product''';

final breadcrumb = fixture.hub.scope.breadcrumbs.last;
expect(breadcrumb.data?['status'], 'internalError');
expect(breadcrumb.type, 'error');
expect(breadcrumb.level, SentryLevel.error);
});

test('apply sets span to internal error if its thrown', () async {
Expand Down Expand Up @@ -644,6 +646,8 @@ SELECT * FROM Product''';

final breadcrumb = fixture.hub.scope.breadcrumbs.last;
expect(breadcrumb.data?['status'], 'internalError');
expect(breadcrumb.type, 'error');
expect(breadcrumb.level, SentryLevel.error);
});
});
}
Expand Down