From 6aa04fac02447956cf06314e98c4c1f4ac4c7c23 Mon Sep 17 00:00:00 2001 From: buenaflor Date: Tue, 5 Sep 2023 17:24:33 +0200 Subject: [PATCH 01/12] add db attibutes to spans --- sqflite/lib/src/sentry_batch.dart | 10 ++++ sqflite/lib/src/sentry_database.dart | 18 +++++++ sqflite/lib/src/sentry_database_executor.dart | 45 ++++++++++++++++ sqflite/pubspec.yaml | 1 + sqflite/test/sentry_batch_test.dart | 47 +++++++++++++++++ sqflite/test/sentry_database_test.dart | 51 +++++++++++++++++++ 6 files changed, 172 insertions(+) diff --git a/sqflite/lib/src/sentry_batch.dart b/sqflite/lib/src/sentry_batch.dart index 95949125bd..8bc8f4448c 100644 --- a/sqflite/lib/src/sentry_batch.dart +++ b/sqflite/lib/src/sentry_batch.dart @@ -5,6 +5,7 @@ import 'package:sqflite/sqflite.dart'; // ignore: implementation_imports import 'package:sqflite_common/src/sql_builder.dart'; +import '../sentry_sqflite.dart'; import 'sentry_database.dart'; /// A [Batch] wrapper that adds Sentry support. @@ -47,8 +48,13 @@ class SentryBatch implements Batch { SentryDatabase.dbOp, description: _buffer.toString().trim(), ); + // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteBatch; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _batch.apply( @@ -86,6 +92,10 @@ class SentryBatch implements Batch { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteBatch; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _batch.commit( diff --git a/sqflite/lib/src/sentry_database.dart b/sqflite/lib/src/sentry_database.dart index f47551cff8..1d0d2bb1fe 100644 --- a/sqflite/lib/src/sentry_database.dart +++ b/sqflite/lib/src/sentry_database.dart @@ -1,7 +1,9 @@ import 'package:meta/meta.dart'; import 'package:sentry/sentry.dart'; import 'package:sqflite/sqflite.dart'; +import 'package:path/path.dart' as p; +import '../sentry_sqflite.dart'; import 'sentry_database_executor.dart'; import 'sentry_sqflite_transaction.dart'; import 'version.dart'; @@ -31,6 +33,15 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { static const dbSqlQueryOp = 'db.sql.query'; static const _dbSqlOp = 'db.sql.transaction'; + @internal + // ignore: public_member_api_docs + static const dbSystem = 'db.system'; + @internal + // ignore: public_member_api_docs + static const dbName = 'db.name'; + @internal + // ignore: public_member_api_docs + static String? currentDbName; /// ```dart /// import 'package:sqflite/sqflite.dart'; @@ -48,6 +59,7 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { final options = _hub.options; options.sdk.addIntegration('SentrySqfliteTracing'); options.sdk.addPackage(packageName, sdkVersion); + currentDbName = p.basenameWithoutExtension(_database.path); } // TODO: check if perf is enabled @@ -63,6 +75,8 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabase; + currentDbName = null; + try { await _database.close(); @@ -113,6 +127,10 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabase; + span?.setData(dbSystem, 'sqlite'); + if (currentDbName != null) { + span?.setData(dbName, currentDbName); + } Future newAction(Transaction txn) async { final executor = diff --git a/sqflite/lib/src/sentry_database_executor.dart b/sqflite/lib/src/sentry_database_executor.dart index b43d13c8d6..b451a66e41 100644 --- a/sqflite/lib/src/sentry_database_executor.dart +++ b/sqflite/lib/src/sentry_database_executor.dart @@ -41,6 +41,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = @@ -68,8 +72,13 @@ class SentryDatabaseExecutor implements DatabaseExecutor { SentryDatabase.dbSqlExecuteOp, description: sql, ); + span?.setData(SentryDatabase.dbSystem, 'sqlite'); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { await _executor.execute(sql, arguments); @@ -107,6 +116,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _executor.insert( @@ -163,6 +176,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _executor.query( @@ -226,6 +243,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _executor.queryCursor( @@ -266,6 +287,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _executor.rawDelete(sql, arguments); @@ -294,6 +319,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _executor.rawInsert(sql, arguments); @@ -325,6 +354,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _executor.rawQuery(sql, arguments); @@ -357,6 +390,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _executor.rawQueryCursor( @@ -389,6 +426,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _executor.rawUpdate(sql, arguments); @@ -430,6 +471,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; + span?.setData(SentryDatabase.dbSystem, 'sqlite'); + if (SentryDatabase.currentDbName != null) { + span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + } try { final result = await _executor.update( diff --git a/sqflite/pubspec.yaml b/sqflite/pubspec.yaml index db60eec2a5..55fa554d05 100644 --- a/sqflite/pubspec.yaml +++ b/sqflite/pubspec.yaml @@ -14,6 +14,7 @@ dependencies: sqflite: ^2.0.0 sqflite_common: ^2.0.0 meta: ^1.3.0 + path: ^1.8.3 dev_dependencies: lints: ^2.0.0 diff --git a/sqflite/test/sentry_batch_test.dart b/sqflite/test/sentry_batch_test.dart index e01aa4d5c7..991f17a1ae 100644 --- a/sqflite/test/sentry_batch_test.dart +++ b/sqflite/test/sentry_batch_test.dart @@ -285,6 +285,53 @@ SELECT * FROM Product'''; await db.close(); }); + test('apply creates db span with dbSystem and dbName attributes', () async { + final db = await fixture.getDatabase(); + final batch = db.batch(); + + batch.insert('Product', {'title': 'Product 1'}); + + await batch.apply(); + + final span = fixture.tracer.children.last; + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], SentryDatabase.currentDbName); + + await db.close(); + }); + + test('commit creates db span with dbSystem and dbName attributes', + () async { + final db = await fixture.getDatabase(); + final batch = db.batch(); + + batch.insert('Product', {'title': 'Product 1'}); + + await batch.commit(); + + final span = fixture.tracer.children.last; + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], SentryDatabase.currentDbName); + + await db.close(); + }); + + test('dbName attribute is null if currentDbName is null', () async { + final db = await fixture.getDatabase(); + final batch = db.batch(); + SentryDatabase.currentDbName = null; + + batch.insert('Product', {'title': 'Product 1'}); + + await batch.commit(); + + final span = fixture.tracer.children.last; + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], isNull); + + await db.close(); + }); + tearDown(() { databaseFactory = sqfliteDatabaseFactoryDefault; }); diff --git a/sqflite/test/sentry_database_test.dart b/sqflite/test/sentry_database_test.dart index 3a76d8066e..90f051242c 100644 --- a/sqflite/test/sentry_database_test.dart +++ b/sqflite/test/sentry_database_test.dart @@ -130,6 +130,35 @@ void main() { await db.close(); }); + test('opening db sets currentDbName with :memory:', () async { + final db = await fixture.getSut(); + + expect(SentryDatabase.currentDbName, ':memory:'); + + await db.close(); + }); + + test('opening db sets currentDbName with db file without extension', + () async { + final db = await fixture.getSut( + database: await openDatabase('path/database/mydatabase.db'), + execute: false); + + expect(SentryDatabase.currentDbName, 'mydatabase'); + + await db.close(); + }); + + test('closing db sets currentDbName to null', () async { + final db = await fixture.getSut(); + + expect(SentryDatabase.currentDbName, ':memory:'); + + await db.close(); + + expect(SentryDatabase.currentDbName, null); + }); + tearDown(() { databaseFactory = sqfliteDatabaseFactoryDefault; }); @@ -212,6 +241,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'DELETE FROM Product'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -231,6 +262,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'DELETE FROM Product'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -253,6 +286,8 @@ void main() { 'INSERT INTO Product (title) VALUES (?)', ); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -272,6 +307,8 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -291,6 +328,8 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -310,6 +349,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'DELETE FROM Product'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -333,6 +374,8 @@ void main() { 'INSERT INTO Product (title) VALUES (?)', ); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -352,6 +395,8 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -371,6 +416,8 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -390,6 +437,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'UPDATE Product SET title = ?'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, @@ -409,6 +458,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'UPDATE Product SET title = ?'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystem], 'sqlite'); + expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); expect( span.origin, From bcc9dd978de79084990b1233d35b26daedc67c42 Mon Sep 17 00:00:00 2001 From: buenaflor Date: Tue, 5 Sep 2023 17:34:15 +0200 Subject: [PATCH 02/12] change variable names --- sqflite/lib/src/sentry_batch.dart | 8 ++-- sqflite/lib/src/sentry_database.dart | 8 ++-- sqflite/lib/src/sentry_database_executor.dart | 46 +++++++++---------- sqflite/test/sentry_batch_test.dart | 12 ++--- sqflite/test/sentry_database_test.dart | 44 +++++++++--------- 5 files changed, 59 insertions(+), 59 deletions(-) diff --git a/sqflite/lib/src/sentry_batch.dart b/sqflite/lib/src/sentry_batch.dart index 8bc8f4448c..abaa62c08e 100644 --- a/sqflite/lib/src/sentry_batch.dart +++ b/sqflite/lib/src/sentry_batch.dart @@ -51,9 +51,9 @@ class SentryBatch implements Batch { // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteBatch; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -92,9 +92,9 @@ class SentryBatch implements Batch { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteBatch; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { diff --git a/sqflite/lib/src/sentry_database.dart b/sqflite/lib/src/sentry_database.dart index 1d0d2bb1fe..159d35783c 100644 --- a/sqflite/lib/src/sentry_database.dart +++ b/sqflite/lib/src/sentry_database.dart @@ -35,10 +35,10 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { static const _dbSqlOp = 'db.sql.transaction'; @internal // ignore: public_member_api_docs - static const dbSystem = 'db.system'; + static const dbSystemKey = 'db.system'; @internal // ignore: public_member_api_docs - static const dbName = 'db.name'; + static const dbNameKey = 'db.name'; @internal // ignore: public_member_api_docs static String? currentDbName; @@ -127,9 +127,9 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabase; - span?.setData(dbSystem, 'sqlite'); + span?.setData(dbSystemKey, 'sqlite'); if (currentDbName != null) { - span?.setData(dbName, currentDbName); + span?.setData(dbNameKey, currentDbName); } Future newAction(Transaction txn) async { diff --git a/sqflite/lib/src/sentry_database_executor.dart b/sqflite/lib/src/sentry_database_executor.dart index b451a66e41..3ef1ac7d96 100644 --- a/sqflite/lib/src/sentry_database_executor.dart +++ b/sqflite/lib/src/sentry_database_executor.dart @@ -41,9 +41,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -72,12 +72,12 @@ class SentryDatabaseExecutor implements DatabaseExecutor { SentryDatabase.dbSqlExecuteOp, description: sql, ); - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -116,9 +116,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -176,9 +176,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -243,9 +243,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -287,9 +287,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -319,9 +319,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -354,9 +354,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -390,9 +390,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -426,9 +426,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { @@ -471,9 +471,9 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystem, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbName, SentryDatabase.currentDbName); + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); } try { diff --git a/sqflite/test/sentry_batch_test.dart b/sqflite/test/sentry_batch_test.dart index 991f17a1ae..852d3ef0ae 100644 --- a/sqflite/test/sentry_batch_test.dart +++ b/sqflite/test/sentry_batch_test.dart @@ -294,8 +294,8 @@ SELECT * FROM Product'''; await batch.apply(); final span = fixture.tracer.children.last; - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], SentryDatabase.currentDbName); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], SentryDatabase.currentDbName); await db.close(); }); @@ -310,8 +310,8 @@ SELECT * FROM Product'''; await batch.commit(); final span = fixture.tracer.children.last; - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], SentryDatabase.currentDbName); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], SentryDatabase.currentDbName); await db.close(); }); @@ -326,8 +326,8 @@ SELECT * FROM Product'''; await batch.commit(); final span = fixture.tracer.children.last; - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], isNull); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], isNull); await db.close(); }); diff --git a/sqflite/test/sentry_database_test.dart b/sqflite/test/sentry_database_test.dart index 90f051242c..5c7879a7e5 100644 --- a/sqflite/test/sentry_database_test.dart +++ b/sqflite/test/sentry_database_test.dart @@ -241,8 +241,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'DELETE FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -262,8 +262,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'DELETE FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -286,8 +286,8 @@ void main() { 'INSERT INTO Product (title) VALUES (?)', ); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -307,8 +307,8 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -328,8 +328,8 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -349,8 +349,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'DELETE FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -374,8 +374,8 @@ void main() { 'INSERT INTO Product (title) VALUES (?)', ); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -395,8 +395,8 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -416,8 +416,8 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -437,8 +437,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'UPDATE Product SET title = ?'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, @@ -458,8 +458,8 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'UPDATE Product SET title = ?'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystem], 'sqlite'); - expect(span.data[SentryDatabase.dbName], inMemoryDatabasePath); + expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, From f9a3a886659218d12f29c6114439c5cfb3421355 Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 10:36:02 +0200 Subject: [PATCH 03/12] update changelog --- CHANGELOG.md | 4 ++++ sqflite/lib/src/utils/sentry_database_span_attributes.dart | 0 2 files changed, 4 insertions(+) create mode 100644 sqflite/lib/src/utils/sentry_database_span_attributes.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index dd68f3d14a..6e27beb1bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Add db.system and db.name attributes to db spans data ([#1629](https://github.com/getsentry/sentry-dart/pull/1629)) + ### Fixes - Normalize data properties of `SentryUser` and `Breadcrumb` before sending over method channel ([#1591](https://github.com/getsentry/sentry-dart/pull/1591)) diff --git a/sqflite/lib/src/utils/sentry_database_span_attributes.dart b/sqflite/lib/src/utils/sentry_database_span_attributes.dart new file mode 100644 index 0000000000..e69de29bb2 From 296c765cbb11f97bf75b9891e55e6f5312ee6b1e Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 10:36:49 +0200 Subject: [PATCH 04/12] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e27beb1bd..f1cebdf426 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -### Features +### Enhancements - Add db.system and db.name attributes to db spans data ([#1629](https://github.com/getsentry/sentry-dart/pull/1629)) From ef1e08cf5f6a5ff5e18690a735114be8c29319cb Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 11:00:58 +0200 Subject: [PATCH 05/12] update tests --- sqflite/lib/src/sentry_batch.dart | 11 +--- sqflite/lib/src/sentry_database.dart | 29 +++++++--- sqflite/lib/src/sentry_database_executor.dart | 58 +++++-------------- .../sentry_database_span_attributes.dart | 12 ++++ sqflite/pubspec.yaml | 1 - sqflite/test/sentry_batch_test.dart | 12 ++-- sqflite/test/sentry_database_test.dart | 36 +++++++----- ...ry_sqflite_database_factory_dart_test.dart | 5 +- 8 files changed, 80 insertions(+), 84 deletions(-) diff --git a/sqflite/lib/src/sentry_batch.dart b/sqflite/lib/src/sentry_batch.dart index abaa62c08e..0e4d0c85f5 100644 --- a/sqflite/lib/src/sentry_batch.dart +++ b/sqflite/lib/src/sentry_batch.dart @@ -7,6 +7,7 @@ import 'package:sqflite_common/src/sql_builder.dart'; import '../sentry_sqflite.dart'; import 'sentry_database.dart'; +import 'utils/sentry_database_span_attributes.dart'; /// A [Batch] wrapper that adds Sentry support. /// @@ -51,10 +52,7 @@ class SentryBatch implements Batch { // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteBatch; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _batch.apply( @@ -92,10 +90,7 @@ class SentryBatch implements Batch { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteBatch; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _batch.commit( diff --git a/sqflite/lib/src/sentry_database.dart b/sqflite/lib/src/sentry_database.dart index 159d35783c..e4e05e3321 100644 --- a/sqflite/lib/src/sentry_database.dart +++ b/sqflite/lib/src/sentry_database.dart @@ -1,12 +1,12 @@ import 'package:meta/meta.dart'; import 'package:sentry/sentry.dart'; import 'package:sqflite/sqflite.dart'; -import 'package:path/path.dart' as p; import '../sentry_sqflite.dart'; import 'sentry_database_executor.dart'; import 'sentry_sqflite_transaction.dart'; import 'version.dart'; +import 'utils/sentry_database_span_attributes.dart'; /// A [Database] wrapper that adds Sentry support. /// @@ -38,10 +38,14 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { static const dbSystemKey = 'db.system'; @internal // ignore: public_member_api_docs + static const dbSystem = 'sqlite'; + @internal + // ignore: public_member_api_docs static const dbNameKey = 'db.name'; @internal // ignore: public_member_api_docs - static String? currentDbName; + static String? dbName; + /// ```dart /// import 'package:sqflite/sqflite.dart'; @@ -59,7 +63,19 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { final options = _hub.options; options.sdk.addIntegration('SentrySqfliteTracing'); options.sdk.addPackage(packageName, sdkVersion); - currentDbName = p.basenameWithoutExtension(_database.path); + dbName = _basenameWithoutExtension(_database.path); + } + + /// Gets the part of path after the last separator, and without any trailing file extension. + String _basenameWithoutExtension(String filePath) { + int lastIndex = filePath.lastIndexOf('/'); + int dotIndex = filePath.lastIndexOf('.'); + + if (dotIndex == -1 || (lastIndex != -1 && dotIndex < lastIndex)) { + return filePath; + } + + return filePath.substring(lastIndex + 1, dotIndex); } // TODO: check if perf is enabled @@ -75,7 +91,7 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabase; - currentDbName = null; + dbName = null; try { await _database.close(); @@ -127,10 +143,7 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabase; - span?.setData(dbSystemKey, 'sqlite'); - if (currentDbName != null) { - span?.setData(dbNameKey, currentDbName); - } + setDatabaseAttributeData(span); Future newAction(Transaction txn) async { final executor = diff --git a/sqflite/lib/src/sentry_database_executor.dart b/sqflite/lib/src/sentry_database_executor.dart index 3ef1ac7d96..ab5e8b23dc 100644 --- a/sqflite/lib/src/sentry_database_executor.dart +++ b/sqflite/lib/src/sentry_database_executor.dart @@ -7,6 +7,7 @@ import 'package:sqflite_common/src/sql_builder.dart'; import 'sentry_batch.dart'; import 'sentry_database.dart'; +import 'utils/sentry_database_span_attributes.dart'; @internal // ignore: public_member_api_docs @@ -41,10 +42,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = @@ -72,13 +70,10 @@ class SentryDatabaseExecutor implements DatabaseExecutor { SentryDatabase.dbSqlExecuteOp, description: sql, ); - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); + span?.setData(SentryDatabase.dbSystemKey, SentryDatabase.dbSystem); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { await _executor.execute(sql, arguments); @@ -116,10 +111,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _executor.insert( @@ -176,10 +168,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _executor.query( @@ -243,10 +232,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _executor.queryCursor( @@ -287,10 +273,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _executor.rawDelete(sql, arguments); @@ -319,10 +302,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _executor.rawInsert(sql, arguments); @@ -354,10 +334,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _executor.rawQuery(sql, arguments); @@ -390,10 +367,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _executor.rawQueryCursor( @@ -426,10 +400,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _executor.rawUpdate(sql, arguments); @@ -471,10 +442,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - span?.setData(SentryDatabase.dbSystemKey, 'sqlite'); - if (SentryDatabase.currentDbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.currentDbName); - } + setDatabaseAttributeData(span); try { final result = await _executor.update( diff --git a/sqflite/lib/src/utils/sentry_database_span_attributes.dart b/sqflite/lib/src/utils/sentry_database_span_attributes.dart index e69de29bb2..d2542a80fd 100644 --- a/sqflite/lib/src/utils/sentry_database_span_attributes.dart +++ b/sqflite/lib/src/utils/sentry_database_span_attributes.dart @@ -0,0 +1,12 @@ +import 'package:sentry/sentry.dart'; + +import '../../sentry_sqflite.dart'; + +/// Sets the database attributes on the [span]. +/// It contains the database system and the database name. +void setDatabaseAttributeData(ISentrySpan? span) { + span?.setData(SentryDatabase.dbSystemKey, SentryDatabase.dbSystem); + if (SentryDatabase.dbName != null) { + span?.setData(SentryDatabase.dbNameKey, SentryDatabase.dbName); + } +} \ No newline at end of file diff --git a/sqflite/pubspec.yaml b/sqflite/pubspec.yaml index 55fa554d05..db60eec2a5 100644 --- a/sqflite/pubspec.yaml +++ b/sqflite/pubspec.yaml @@ -14,7 +14,6 @@ dependencies: sqflite: ^2.0.0 sqflite_common: ^2.0.0 meta: ^1.3.0 - path: ^1.8.3 dev_dependencies: lints: ^2.0.0 diff --git a/sqflite/test/sentry_batch_test.dart b/sqflite/test/sentry_batch_test.dart index 852d3ef0ae..093bba0995 100644 --- a/sqflite/test/sentry_batch_test.dart +++ b/sqflite/test/sentry_batch_test.dart @@ -294,8 +294,8 @@ SELECT * FROM Product'''; await batch.apply(); final span = fixture.tracer.children.last; - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); - expect(span.data[SentryDatabase.dbNameKey], SentryDatabase.currentDbName); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); + expect(span.data[SentryDatabase.dbNameKey], SentryDatabase.dbName); await db.close(); }); @@ -310,8 +310,8 @@ SELECT * FROM Product'''; await batch.commit(); final span = fixture.tracer.children.last; - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); - expect(span.data[SentryDatabase.dbNameKey], SentryDatabase.currentDbName); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); + expect(span.data[SentryDatabase.dbNameKey], SentryDatabase.dbName); await db.close(); }); @@ -319,14 +319,14 @@ SELECT * FROM Product'''; test('dbName attribute is null if currentDbName is null', () async { final db = await fixture.getDatabase(); final batch = db.batch(); - SentryDatabase.currentDbName = null; + SentryDatabase.dbName = null; batch.insert('Product', {'title': 'Product 1'}); await batch.commit(); final span = fixture.tracer.children.last; - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], isNull); await db.close(); diff --git a/sqflite/test/sentry_database_test.dart b/sqflite/test/sentry_database_test.dart index 5c7879a7e5..77a9d562a3 100644 --- a/sqflite/test/sentry_database_test.dart +++ b/sqflite/test/sentry_database_test.dart @@ -63,6 +63,8 @@ void main() { await db.close(); + expect(SentryDatabase.dbName, null); + final span = fixture.tracer.children.last; expect(span.context.operation, 'db'); expect(span.context.description, 'Close DB: $inMemoryDatabasePath'); @@ -84,6 +86,8 @@ void main() { expect(span.context.operation, 'db.sql.transaction'); expect(span.context.description, 'Transaction DB: $inMemoryDatabasePath'); expect(span.status, SpanStatus.ok()); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); + expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( span.origin, // ignore: invalid_use_of_internal_member @@ -109,6 +113,8 @@ void main() { ); expect(insertSpan.context.parentSpanId, trSpan.context.spanId); expect(insertSpan.status, SpanStatus.ok()); + expect(insertSpan.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); + expect(insertSpan.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( insertSpan.origin, @@ -133,7 +139,7 @@ void main() { test('opening db sets currentDbName with :memory:', () async { final db = await fixture.getSut(); - expect(SentryDatabase.currentDbName, ':memory:'); + expect(SentryDatabase.dbName, ':memory:'); await db.close(); }); @@ -144,7 +150,7 @@ void main() { database: await openDatabase('path/database/mydatabase.db'), execute: false); - expect(SentryDatabase.currentDbName, 'mydatabase'); + expect(SentryDatabase.dbName, 'mydatabase'); await db.close(); }); @@ -152,11 +158,11 @@ void main() { test('closing db sets currentDbName to null', () async { final db = await fixture.getSut(); - expect(SentryDatabase.currentDbName, ':memory:'); + expect(SentryDatabase.dbName, inMemoryDatabasePath); await db.close(); - expect(SentryDatabase.currentDbName, null); + expect(SentryDatabase.dbName, null); }); tearDown(() { @@ -241,7 +247,7 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'DELETE FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -262,7 +268,7 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'DELETE FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -286,7 +292,7 @@ void main() { 'INSERT INTO Product (title) VALUES (?)', ); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -307,7 +313,7 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -328,7 +334,7 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -349,7 +355,7 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'DELETE FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -374,7 +380,7 @@ void main() { 'INSERT INTO Product (title) VALUES (?)', ); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -395,7 +401,7 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -416,7 +422,7 @@ void main() { expect(span.context.operation, 'db.sql.query'); expect(span.context.description, 'SELECT * FROM Product'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -437,7 +443,7 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'UPDATE Product SET title = ?'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -458,7 +464,7 @@ void main() { expect(span.context.operation, 'db.sql.execute'); expect(span.context.description, 'UPDATE Product SET title = ?'); expect(span.status, SpanStatus.ok()); - expect(span.data[SentryDatabase.dbSystemKey], 'sqlite'); + expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(span.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( diff --git a/sqflite/test/sentry_sqflite_database_factory_dart_test.dart b/sqflite/test/sentry_sqflite_database_factory_dart_test.dart index 61f8aaa9ca..46e84b3347 100644 --- a/sqflite/test/sentry_sqflite_database_factory_dart_test.dart +++ b/sqflite/test/sentry_sqflite_database_factory_dart_test.dart @@ -37,6 +37,7 @@ void main() { test('returns wrapped data base if performance enabled', () async { final db = await openDatabase(inMemoryDatabasePath); + expect(SentryDatabase.dbName, inMemoryDatabasePath); expect(db is SentryDatabase, true); await db.close(); @@ -47,6 +48,7 @@ void main() { final db = await openDatabase(inMemoryDatabasePath); + expect(SentryDatabase.dbName, inMemoryDatabasePath); expect(db is! SentryDatabase, true); await db.close(); @@ -56,10 +58,11 @@ void main() { () async { final db = await openDatabase(inMemoryDatabasePath); + expect(SentryDatabase.dbName, inMemoryDatabasePath); + final span = fixture.tracer.children.last; expect(span.context.operation, 'db'); expect(span.context.description, 'Open DB: $inMemoryDatabasePath'); - expect(span.context.description, 'Open DB: $inMemoryDatabasePath'); expect( span.origin, From 5f33fca8ab268ec8dcd19287d8d13c062f3514e6 Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 11:08:25 +0200 Subject: [PATCH 06/12] update imports --- sqflite/lib/src/sentry_batch.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/sqflite/lib/src/sentry_batch.dart b/sqflite/lib/src/sentry_batch.dart index 0e4d0c85f5..6605793aee 100644 --- a/sqflite/lib/src/sentry_batch.dart +++ b/sqflite/lib/src/sentry_batch.dart @@ -5,7 +5,6 @@ import 'package:sqflite/sqflite.dart'; // ignore: implementation_imports import 'package:sqflite_common/src/sql_builder.dart'; -import '../sentry_sqflite.dart'; import 'sentry_database.dart'; import 'utils/sentry_database_span_attributes.dart'; From 514de71326cd2cc1f6cb703eeda56a16452881b5 Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 11:17:51 +0200 Subject: [PATCH 07/12] update tests --- sqflite/test/sentry_sqflite_database_factory_dart_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqflite/test/sentry_sqflite_database_factory_dart_test.dart b/sqflite/test/sentry_sqflite_database_factory_dart_test.dart index 46e84b3347..746f9add56 100644 --- a/sqflite/test/sentry_sqflite_database_factory_dart_test.dart +++ b/sqflite/test/sentry_sqflite_database_factory_dart_test.dart @@ -48,7 +48,7 @@ void main() { final db = await openDatabase(inMemoryDatabasePath); - expect(SentryDatabase.dbName, inMemoryDatabasePath); + expect(SentryDatabase.dbName, null); expect(db is! SentryDatabase, true); await db.close(); From c55ce57f143c7605dd52cb90de3131ab3e31dcf1 Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 11:40:49 +0200 Subject: [PATCH 08/12] remove unused import --- sqflite/lib/src/sentry_database.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/sqflite/lib/src/sentry_database.dart b/sqflite/lib/src/sentry_database.dart index e4e05e3321..c184ba899e 100644 --- a/sqflite/lib/src/sentry_database.dart +++ b/sqflite/lib/src/sentry_database.dart @@ -2,7 +2,6 @@ import 'package:meta/meta.dart'; import 'package:sentry/sentry.dart'; import 'package:sqflite/sqflite.dart'; -import '../sentry_sqflite.dart'; import 'sentry_database_executor.dart'; import 'sentry_sqflite_transaction.dart'; import 'version.dart'; From 39b4c2584a8b466252c3c0e329c8939e3050dec9 Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 12:56:04 +0200 Subject: [PATCH 09/12] couple dbName to db instance --- sqflite/lib/src/sentry_batch.dart | 9 ++++-- sqflite/lib/src/sentry_database.dart | 18 +++++------- sqflite/lib/src/sentry_database_executor.dart | 29 ++++++++++--------- .../lib/src/sentry_sqflite_transaction.dart | 7 +++-- .../sentry_database_span_attributes.dart | 8 ++--- sqflite/test/sentry_batch_test.dart | 20 ++----------- sqflite/test/sentry_database_test.dart | 10 +++---- ...ry_sqflite_database_factory_dart_test.dart | 5 ++-- sqflite/test/sentry_sqflite_test.dart | 1 + 9 files changed, 49 insertions(+), 58 deletions(-) diff --git a/sqflite/lib/src/sentry_batch.dart b/sqflite/lib/src/sentry_batch.dart index 6605793aee..150b1c51da 100644 --- a/sqflite/lib/src/sentry_batch.dart +++ b/sqflite/lib/src/sentry_batch.dart @@ -22,6 +22,7 @@ import 'utils/sentry_database_span_attributes.dart'; class SentryBatch implements Batch { final Batch _batch; final Hub _hub; + final String? _dbName; // we don't clear the buffer because SqfliteBatch don't either final _buffer = StringBuffer(); @@ -37,7 +38,9 @@ class SentryBatch implements Batch { SentryBatch( this._batch, { @internal Hub? hub, - }) : _hub = hub ?? HubAdapter(); + @internal String? dbName, + }) : _hub = hub ?? HubAdapter(), + _dbName = dbName; @override Future> apply({bool? noResult, bool? continueOnError}) { @@ -51,7 +54,7 @@ class SentryBatch implements Batch { // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteBatch; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _batch.apply( @@ -89,7 +92,7 @@ class SentryBatch implements Batch { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteBatch; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _batch.commit( diff --git a/sqflite/lib/src/sentry_database.dart b/sqflite/lib/src/sentry_database.dart index c184ba899e..e0a58ebee8 100644 --- a/sqflite/lib/src/sentry_database.dart +++ b/sqflite/lib/src/sentry_database.dart @@ -43,8 +43,7 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { static const dbNameKey = 'db.name'; @internal // ignore: public_member_api_docs - static String? dbName; - + String? dbName; /// ```dart /// import 'package:sqflite/sqflite.dart'; @@ -57,16 +56,16 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { this._database, { @internal Hub? hub, }) : _hub = hub ?? HubAdapter(), - super(_database, hub: hub) { + dbName = _basenameWithoutExtension(_database.path), + super(_database, hub: hub, dbName: _basenameWithoutExtension(_database.path)) { // ignore: invalid_use_of_internal_member final options = _hub.options; options.sdk.addIntegration('SentrySqfliteTracing'); options.sdk.addPackage(packageName, sdkVersion); - dbName = _basenameWithoutExtension(_database.path); } /// Gets the part of path after the last separator, and without any trailing file extension. - String _basenameWithoutExtension(String filePath) { + static String _basenameWithoutExtension(String filePath) { int lastIndex = filePath.lastIndexOf('/'); int dotIndex = filePath.lastIndexOf('.'); @@ -90,11 +89,10 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabase; - dbName = null; - try { await _database.close(); + dbName = null; span?.status = SpanStatus.ok(); } catch (exception) { span?.throwable = exception; @@ -142,13 +140,13 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabase; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, dbName); Future newAction(Transaction txn) async { final executor = - SentryDatabaseExecutor(txn, parentSpan: span, hub: _hub); + SentryDatabaseExecutor(txn, parentSpan: span, hub: _hub, dbName: dbName); final sentrySqfliteTransaction = - SentrySqfliteTransaction(executor, hub: _hub); + SentrySqfliteTransaction(executor, hub: _hub, dbName: dbName); return await action(sentrySqfliteTransaction); } diff --git a/sqflite/lib/src/sentry_database_executor.dart b/sqflite/lib/src/sentry_database_executor.dart index ab5e8b23dc..c232ca9e24 100644 --- a/sqflite/lib/src/sentry_database_executor.dart +++ b/sqflite/lib/src/sentry_database_executor.dart @@ -14,18 +14,21 @@ import 'utils/sentry_database_span_attributes.dart'; class SentryDatabaseExecutor implements DatabaseExecutor { final DatabaseExecutor _executor; final ISentrySpan? _parentSpan; + final String? _dbName; // ignore: public_member_api_docs SentryDatabaseExecutor( this._executor, { ISentrySpan? parentSpan, @internal Hub? hub, + @internal String? dbName, }) : _parentSpan = parentSpan, - _hub = hub ?? HubAdapter(); + _hub = hub ?? HubAdapter(), + _dbName = dbName; final Hub _hub; @override - Batch batch() => SentryBatch(_executor.batch(), hub: _hub); + Batch batch() => SentryBatch(_executor.batch(), hub: _hub, dbName: _dbName); @override Database get database => _executor.database; @@ -42,7 +45,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = @@ -73,7 +76,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { span?.setData(SentryDatabase.dbSystemKey, SentryDatabase.dbSystem); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { await _executor.execute(sql, arguments); @@ -111,7 +114,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _executor.insert( @@ -168,7 +171,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _executor.query( @@ -232,7 +235,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _executor.queryCursor( @@ -273,7 +276,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _executor.rawDelete(sql, arguments); @@ -302,7 +305,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _executor.rawInsert(sql, arguments); @@ -334,7 +337,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _executor.rawQuery(sql, arguments); @@ -367,7 +370,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _executor.rawQueryCursor( @@ -400,7 +403,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _executor.rawUpdate(sql, arguments); @@ -442,7 +445,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { ); // ignore: invalid_use_of_internal_member span?.origin = SentryTraceOrigins.autoDbSqfliteDatabaseExecutor; - setDatabaseAttributeData(span); + setDatabaseAttributeData(span, _dbName); try { final result = await _executor.update( diff --git a/sqflite/lib/src/sentry_sqflite_transaction.dart b/sqflite/lib/src/sentry_sqflite_transaction.dart index c207023e9c..78c41b487d 100644 --- a/sqflite/lib/src/sentry_sqflite_transaction.dart +++ b/sqflite/lib/src/sentry_sqflite_transaction.dart @@ -21,16 +21,19 @@ import 'sentry_batch.dart'; class SentrySqfliteTransaction extends Transaction implements DatabaseExecutor { final DatabaseExecutor _executor; final Hub _hub; + final String? _dbName; // ignore: public_member_api_docs @internal SentrySqfliteTransaction( this._executor, { @internal Hub? hub, - }) : _hub = hub ?? HubAdapter(); + @internal String? dbName, + }) : _hub = hub ?? HubAdapter(), + _dbName = dbName; @override - Batch batch() => SentryBatch(_executor.batch(), hub: _hub); + Batch batch() => SentryBatch(_executor.batch(), hub: _hub, dbName: _dbName); @override Database get database => _executor.database; diff --git a/sqflite/lib/src/utils/sentry_database_span_attributes.dart b/sqflite/lib/src/utils/sentry_database_span_attributes.dart index d2542a80fd..119f2dd1b8 100644 --- a/sqflite/lib/src/utils/sentry_database_span_attributes.dart +++ b/sqflite/lib/src/utils/sentry_database_span_attributes.dart @@ -4,9 +4,9 @@ import '../../sentry_sqflite.dart'; /// Sets the database attributes on the [span]. /// It contains the database system and the database name. -void setDatabaseAttributeData(ISentrySpan? span) { +void setDatabaseAttributeData(ISentrySpan? span, String? dbName) { span?.setData(SentryDatabase.dbSystemKey, SentryDatabase.dbSystem); - if (SentryDatabase.dbName != null) { - span?.setData(SentryDatabase.dbNameKey, SentryDatabase.dbName); + if (dbName != null) { + span?.setData(SentryDatabase.dbNameKey, dbName); } -} \ No newline at end of file +} diff --git a/sqflite/test/sentry_batch_test.dart b/sqflite/test/sentry_batch_test.dart index 093bba0995..35d4b51a66 100644 --- a/sqflite/test/sentry_batch_test.dart +++ b/sqflite/test/sentry_batch_test.dart @@ -295,7 +295,7 @@ SELECT * FROM Product'''; final span = fixture.tracer.children.last; expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); - expect(span.data[SentryDatabase.dbNameKey], SentryDatabase.dbName); + expect(span.data[SentryDatabase.dbNameKey], (db as SentryDatabase).dbName); await db.close(); }); @@ -311,23 +311,7 @@ SELECT * FROM Product'''; final span = fixture.tracer.children.last; expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); - expect(span.data[SentryDatabase.dbNameKey], SentryDatabase.dbName); - - await db.close(); - }); - - test('dbName attribute is null if currentDbName is null', () async { - final db = await fixture.getDatabase(); - final batch = db.batch(); - SentryDatabase.dbName = null; - - batch.insert('Product', {'title': 'Product 1'}); - - await batch.commit(); - - final span = fixture.tracer.children.last; - expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); - expect(span.data[SentryDatabase.dbNameKey], isNull); + expect(span.data[SentryDatabase.dbNameKey], (db as SentryDatabase).dbName); await db.close(); }); diff --git a/sqflite/test/sentry_database_test.dart b/sqflite/test/sentry_database_test.dart index 77a9d562a3..c7199d4d3f 100644 --- a/sqflite/test/sentry_database_test.dart +++ b/sqflite/test/sentry_database_test.dart @@ -63,7 +63,7 @@ void main() { await db.close(); - expect(SentryDatabase.dbName, null); + expect(db.dbName, null); final span = fixture.tracer.children.last; expect(span.context.operation, 'db'); @@ -139,7 +139,7 @@ void main() { test('opening db sets currentDbName with :memory:', () async { final db = await fixture.getSut(); - expect(SentryDatabase.dbName, ':memory:'); + expect(db.dbName, ':memory:'); await db.close(); }); @@ -150,7 +150,7 @@ void main() { database: await openDatabase('path/database/mydatabase.db'), execute: false); - expect(SentryDatabase.dbName, 'mydatabase'); + expect(db.dbName, 'mydatabase'); await db.close(); }); @@ -158,11 +158,11 @@ void main() { test('closing db sets currentDbName to null', () async { final db = await fixture.getSut(); - expect(SentryDatabase.dbName, inMemoryDatabasePath); + expect(db.dbName, inMemoryDatabasePath); await db.close(); - expect(SentryDatabase.dbName, null); + expect(db.dbName, null); }); tearDown(() { diff --git a/sqflite/test/sentry_sqflite_database_factory_dart_test.dart b/sqflite/test/sentry_sqflite_database_factory_dart_test.dart index 746f9add56..d2ed2e9fb5 100644 --- a/sqflite/test/sentry_sqflite_database_factory_dart_test.dart +++ b/sqflite/test/sentry_sqflite_database_factory_dart_test.dart @@ -37,8 +37,8 @@ void main() { test('returns wrapped data base if performance enabled', () async { final db = await openDatabase(inMemoryDatabasePath); - expect(SentryDatabase.dbName, inMemoryDatabasePath); expect(db is SentryDatabase, true); + expect((db as SentryDatabase).dbName, inMemoryDatabasePath); await db.close(); }); @@ -48,7 +48,6 @@ void main() { final db = await openDatabase(inMemoryDatabasePath); - expect(SentryDatabase.dbName, null); expect(db is! SentryDatabase, true); await db.close(); @@ -58,7 +57,7 @@ void main() { () async { final db = await openDatabase(inMemoryDatabasePath); - expect(SentryDatabase.dbName, inMemoryDatabasePath); + expect((db as SentryDatabase).dbName, inMemoryDatabasePath); final span = fixture.tracer.children.last; expect(span.context.operation, 'db'); diff --git a/sqflite/test/sentry_sqflite_test.dart b/sqflite/test/sentry_sqflite_test.dart index 54ff24232d..ff75d67e6d 100644 --- a/sqflite/test/sentry_sqflite_test.dart +++ b/sqflite/test/sentry_sqflite_test.dart @@ -97,6 +97,7 @@ void main() { expect(span.status, SpanStatus.ok()); // ignore: invalid_use_of_internal_member expect(span.origin, SentryTraceOrigins.autoDbSqfliteOpenDatabase); + expect((db as SentryDatabase).dbName, inMemoryDatabasePath); await db.close(); }); From dec3513b2728fd419cf609c584053e9abb2fb76c Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 12:58:28 +0200 Subject: [PATCH 10/12] format --- sqflite/lib/src/sentry_database.dart | 11 ++++++----- sqflite/test/mocks/mocks.dart | 3 ++- sqflite/test/sentry_batch_test.dart | 6 ++++-- sqflite/test/sentry_database_test.dart | 8 +++++--- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/sqflite/lib/src/sentry_database.dart b/sqflite/lib/src/sentry_database.dart index e0a58ebee8..68b9509155 100644 --- a/sqflite/lib/src/sentry_database.dart +++ b/sqflite/lib/src/sentry_database.dart @@ -57,7 +57,8 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { @internal Hub? hub, }) : _hub = hub ?? HubAdapter(), dbName = _basenameWithoutExtension(_database.path), - super(_database, hub: hub, dbName: _basenameWithoutExtension(_database.path)) { + super(_database, + hub: hub, dbName: _basenameWithoutExtension(_database.path)) { // ignore: invalid_use_of_internal_member final options = _hub.options; options.sdk.addIntegration('SentrySqfliteTracing'); @@ -66,8 +67,8 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { /// Gets the part of path after the last separator, and without any trailing file extension. static String _basenameWithoutExtension(String filePath) { - int lastIndex = filePath.lastIndexOf('/'); - int dotIndex = filePath.lastIndexOf('.'); + final int lastIndex = filePath.lastIndexOf('/'); + final int dotIndex = filePath.lastIndexOf('.'); if (dotIndex == -1 || (lastIndex != -1 && dotIndex < lastIndex)) { return filePath; @@ -143,8 +144,8 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { setDatabaseAttributeData(span, dbName); Future newAction(Transaction txn) async { - final executor = - SentryDatabaseExecutor(txn, parentSpan: span, hub: _hub, dbName: dbName); + final executor = SentryDatabaseExecutor(txn, + parentSpan: span, hub: _hub, dbName: dbName); final sentrySqfliteTransaction = SentrySqfliteTransaction(executor, hub: _hub, dbName: dbName); diff --git a/sqflite/test/mocks/mocks.dart b/sqflite/test/mocks/mocks.dart index 0a3115df74..3f0af8274c 100644 --- a/sqflite/test/mocks/mocks.dart +++ b/sqflite/test/mocks/mocks.dart @@ -33,7 +33,8 @@ ISentrySpan startTransactionShim( DatabaseExecutor, ], customMocks: [ - MockSpec(fallbackGenerators: {#startTransaction: startTransactionShim}) + MockSpec( + fallbackGenerators: {#startTransaction: startTransactionShim}), ], ) void main() {} diff --git a/sqflite/test/sentry_batch_test.dart b/sqflite/test/sentry_batch_test.dart index 35d4b51a66..ce6f59e0d5 100644 --- a/sqflite/test/sentry_batch_test.dart +++ b/sqflite/test/sentry_batch_test.dart @@ -295,7 +295,8 @@ SELECT * FROM Product'''; final span = fixture.tracer.children.last; expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); - expect(span.data[SentryDatabase.dbNameKey], (db as SentryDatabase).dbName); + expect( + span.data[SentryDatabase.dbNameKey], (db as SentryDatabase).dbName); await db.close(); }); @@ -311,7 +312,8 @@ SELECT * FROM Product'''; final span = fixture.tracer.children.last; expect(span.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); - expect(span.data[SentryDatabase.dbNameKey], (db as SentryDatabase).dbName); + expect( + span.data[SentryDatabase.dbNameKey], (db as SentryDatabase).dbName); await db.close(); }); diff --git a/sqflite/test/sentry_database_test.dart b/sqflite/test/sentry_database_test.dart index c7199d4d3f..d24dfc31ea 100644 --- a/sqflite/test/sentry_database_test.dart +++ b/sqflite/test/sentry_database_test.dart @@ -113,7 +113,8 @@ void main() { ); expect(insertSpan.context.parentSpanId, trSpan.context.spanId); expect(insertSpan.status, SpanStatus.ok()); - expect(insertSpan.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); + expect( + insertSpan.data[SentryDatabase.dbSystemKey], SentryDatabase.dbSystem); expect(insertSpan.data[SentryDatabase.dbNameKey], inMemoryDatabasePath); expect( @@ -147,8 +148,9 @@ void main() { test('opening db sets currentDbName with db file without extension', () async { final db = await fixture.getSut( - database: await openDatabase('path/database/mydatabase.db'), - execute: false); + database: await openDatabase('path/database/mydatabase.db'), + execute: false, + ); expect(db.dbName, 'mydatabase'); From 86138198d0419729bdf0d15308d52e82a06b831a Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 13:11:24 +0200 Subject: [PATCH 11/12] use path pub --- sqflite/lib/src/sentry_database.dart | 17 +++-------------- sqflite/pubspec.yaml | 1 + 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/sqflite/lib/src/sentry_database.dart b/sqflite/lib/src/sentry_database.dart index 68b9509155..d7f0d7138c 100644 --- a/sqflite/lib/src/sentry_database.dart +++ b/sqflite/lib/src/sentry_database.dart @@ -6,6 +6,7 @@ import 'sentry_database_executor.dart'; import 'sentry_sqflite_transaction.dart'; import 'version.dart'; import 'utils/sentry_database_span_attributes.dart'; +import 'package:path/path.dart' as p; /// A [Database] wrapper that adds Sentry support. /// @@ -56,27 +57,15 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { this._database, { @internal Hub? hub, }) : _hub = hub ?? HubAdapter(), - dbName = _basenameWithoutExtension(_database.path), + dbName = p.basenameWithoutExtension(_database.path), super(_database, - hub: hub, dbName: _basenameWithoutExtension(_database.path)) { + hub: hub, dbName: p.basenameWithoutExtension(_database.path)) { // ignore: invalid_use_of_internal_member final options = _hub.options; options.sdk.addIntegration('SentrySqfliteTracing'); options.sdk.addPackage(packageName, sdkVersion); } - /// Gets the part of path after the last separator, and without any trailing file extension. - static String _basenameWithoutExtension(String filePath) { - final int lastIndex = filePath.lastIndexOf('/'); - final int dotIndex = filePath.lastIndexOf('.'); - - if (dotIndex == -1 || (lastIndex != -1 && dotIndex < lastIndex)) { - return filePath; - } - - return filePath.substring(lastIndex + 1, dotIndex); - } - // TODO: check if perf is enabled @override diff --git a/sqflite/pubspec.yaml b/sqflite/pubspec.yaml index db60eec2a5..55fa554d05 100644 --- a/sqflite/pubspec.yaml +++ b/sqflite/pubspec.yaml @@ -14,6 +14,7 @@ dependencies: sqflite: ^2.0.0 sqflite_common: ^2.0.0 meta: ^1.3.0 + path: ^1.8.3 dev_dependencies: lints: ^2.0.0 From d14d661bdae5590660a14a662a91b27c27204e69 Mon Sep 17 00:00:00 2001 From: buenaflor Date: Thu, 7 Sep 2023 13:32:43 +0200 Subject: [PATCH 12/12] don't set dbName to null when db close() is called --- sqflite/lib/src/sentry_database.dart | 3 +-- sqflite/test/sentry_database_test.dart | 4 ---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/sqflite/lib/src/sentry_database.dart b/sqflite/lib/src/sentry_database.dart index d7f0d7138c..71147c3464 100644 --- a/sqflite/lib/src/sentry_database.dart +++ b/sqflite/lib/src/sentry_database.dart @@ -44,7 +44,7 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { static const dbNameKey = 'db.name'; @internal // ignore: public_member_api_docs - String? dbName; + String dbName; /// ```dart /// import 'package:sqflite/sqflite.dart'; @@ -82,7 +82,6 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { try { await _database.close(); - dbName = null; span?.status = SpanStatus.ok(); } catch (exception) { span?.throwable = exception; diff --git a/sqflite/test/sentry_database_test.dart b/sqflite/test/sentry_database_test.dart index d24dfc31ea..e05bd2367c 100644 --- a/sqflite/test/sentry_database_test.dart +++ b/sqflite/test/sentry_database_test.dart @@ -63,8 +63,6 @@ void main() { await db.close(); - expect(db.dbName, null); - final span = fixture.tracer.children.last; expect(span.context.operation, 'db'); expect(span.context.description, 'Close DB: $inMemoryDatabasePath'); @@ -163,8 +161,6 @@ void main() { expect(db.dbName, inMemoryDatabasePath); await db.close(); - - expect(db.dbName, null); }); tearDown(() {