From c47d2215d2fcfd9db0470a83f5f5adeebb8ecd26 Mon Sep 17 00:00:00 2001 From: Austin Treat Emmons Date: Tue, 4 Jun 2024 12:56:32 -0400 Subject: [PATCH] Add process_identifier to SpanRecord (#248) * Creates 'AddProcessIdentifierToSpanRecordMigration', adds to current Updates SpanRecord to have processIdentifier property Adds Migration+Helpers to help run migrations 'upTo' a certain one. * Update migration to inject list of migrations Needed to make testing specific migrations possible --- Sources/EmbraceStorage/EmbraceStorage.swift | 10 +- .../EmbraceStorage/Migration/Migration.swift | 2 +- ...ocessIdentifierToSpanRecordMigration.swift | 73 ++++++ .../Migrations/Migrations+Current.swift | 5 +- .../EmbraceStorage/Records/SpanRecord.swift | 6 +- .../Internal/StorageSpanExporterTests.swift | 8 +- .../EmbraceStorageTests.swift | 21 +- ...IdentifierToSpanRecordMigrationTests.swift | 222 ++++++++++++++++++ .../Migrations/Migrations+CurrentTests.swift | 5 +- .../EmbraceStorageTests/SpanRecordTests.swift | 11 +- .../TestSupport/Migration+Helpers.swift | 23 ++ ...nService.swift => ThrowingMigration.swift} | 14 +- 12 files changed, 366 insertions(+), 34 deletions(-) create mode 100644 Sources/EmbraceStorage/Migration/Migrations/20240523_00_AddProcessIdentifierToSpanRecordMigration.swift create mode 100644 Tests/EmbraceStorageTests/Migration/Migrations/20240523_00_AddProcessIdentifierToSpanRecordMigrationTests.swift create mode 100644 Tests/EmbraceStorageTests/TestSupport/Migration+Helpers.swift rename Tests/EmbraceStorageTests/TestSupport/{ThrowingMigrationService.swift => ThrowingMigration.swift} (77%) diff --git a/Sources/EmbraceStorage/EmbraceStorage.swift b/Sources/EmbraceStorage/EmbraceStorage.swift index 4bea9b2f..9d02100d 100644 --- a/Sources/EmbraceStorage/EmbraceStorage.swift +++ b/Sources/EmbraceStorage/EmbraceStorage.swift @@ -27,14 +27,14 @@ public class EmbraceStorage: Storage { /// - resetIfError: If true and the migrations fail the DB will be reset entirely. public func performMigration( resetIfError: Bool = true, - service: MigrationServiceProtocol = MigrationService() + migrations: [Migration] = .current ) throws { do { - try service.perform(dbQueue, migrations: .current) + try MigrationService().perform(dbQueue, migrations: migrations) } catch let error { if resetIfError { ConsoleLog.error("Error performing migrations, resetting EmbraceStorage: \(error)") - try reset(migrationService: service) + try reset(migrations: migrations) } else { ConsoleLog.error("Error performing migrations. Reset not enabled: \(error)") throw error // re-throw error if auto-recover is not enabled @@ -43,13 +43,13 @@ public class EmbraceStorage: Storage { } /// Deletes the database and recreates it from scratch - func reset(migrationService: MigrationServiceProtocol = MigrationService()) throws { + func reset(migrations: [Migration] = .current) throws { if let fileURL = options.fileURL { try FileManager.default.removeItem(at: fileURL) } dbQueue = try Self.createDBQueue(options: options) - try performMigration(resetIfError: false, service: migrationService) // Do not perpetuate loop + try performMigration(resetIfError: false, migrations: migrations) // Do not perpetuate loop } } diff --git a/Sources/EmbraceStorage/Migration/Migration.swift b/Sources/EmbraceStorage/Migration/Migration.swift index 70961c95..f70bb4f1 100644 --- a/Sources/EmbraceStorage/Migration/Migration.swift +++ b/Sources/EmbraceStorage/Migration/Migration.swift @@ -23,5 +23,5 @@ extension Migration { } extension Migration { - static var foreignKeyChecks: DatabaseMigrator.ForeignKeyChecks { .immediate } + public static var foreignKeyChecks: DatabaseMigrator.ForeignKeyChecks { .immediate } } diff --git a/Sources/EmbraceStorage/Migration/Migrations/20240523_00_AddProcessIdentifierToSpanRecordMigration.swift b/Sources/EmbraceStorage/Migration/Migrations/20240523_00_AddProcessIdentifierToSpanRecordMigration.swift new file mode 100644 index 00000000..e344cdd3 --- /dev/null +++ b/Sources/EmbraceStorage/Migration/Migrations/20240523_00_AddProcessIdentifierToSpanRecordMigration.swift @@ -0,0 +1,73 @@ +// +// Copyright © 2023 Embrace Mobile, Inc. All rights reserved. +// + +import GRDB + +struct AddProcessIdentifierToSpanRecordMigration: Migration { + static var identifier = "AddProcessIdentifierToSpanRecord" // DEV: Must not change + + private static var tempSpansTableName = "spans_temp" + + func perform(_ db: GRDB.Database) throws { + + // create copy of `spans` table in `spans_temp` + // include new column 'process_identifier' + try db.create(table: Self.tempSpansTableName) { t in + + t.column(SpanRecord.Schema.id.name, .text).notNull() + t.column(SpanRecord.Schema.traceId.name, .text).notNull() + t.primaryKey([SpanRecord.Schema.traceId.name, SpanRecord.Schema.id.name]) + + t.column(SpanRecord.Schema.name.name, .text).notNull() + t.column(SpanRecord.Schema.type.name, .text).notNull() + t.column(SpanRecord.Schema.startTime.name, .datetime).notNull() + t.column(SpanRecord.Schema.endTime.name, .datetime) + t.column(SpanRecord.Schema.data.name, .blob).notNull() + + // include new column into `spans_temp` table + t.column(SpanRecord.Schema.processIdentifier.name, .text).notNull() + } + + // copy all existing data into temp table + // include default value for `process_identifier` + try db.execute(literal: """ + INSERT INTO 'spans_temp' ( + 'id', + 'trace_id', + 'name', + 'type', + 'start_time', + 'end_time', + 'data', + 'process_identifier' + ) SELECT + id, + trace_id, + name, + type, + start_time, + end_time, + data, + 'c0ffee' + FROM 'spans' + """) + + // drop original table + try db.drop(table: SpanRecord.databaseTableName) + + // rename temp table to be original table + try db.rename(table: Self.tempSpansTableName, to: SpanRecord.databaseTableName) + + // Create Trigger on new spans table to prevent endTime from being modified on SpanRecord + try db.execute(sql: + """ + CREATE TRIGGER IF NOT EXISTS prevent_closed_span_modification + BEFORE UPDATE ON \(SpanRecord.databaseTableName) + WHEN OLD.end_time IS NOT NULL + BEGIN + SELECT RAISE(ABORT,'Attempted to modify an already closed span.'); + END; + """ ) + } +} diff --git a/Sources/EmbraceStorage/Migration/Migrations/Migrations+Current.swift b/Sources/EmbraceStorage/Migration/Migrations/Migrations+Current.swift index df611ae4..8185296a 100644 --- a/Sources/EmbraceStorage/Migration/Migrations/Migrations+Current.swift +++ b/Sources/EmbraceStorage/Migration/Migrations/Migrations+Current.swift @@ -6,14 +6,15 @@ import Foundation extension Array where Element == Migration { - static var current: [Migration] { + public static var current: [Migration] { return [ // register migrations here // order matters AddSpanRecordMigration(), AddSessionRecordMigration(), AddMetadataRecordMigration(), - AddLogRecordMigration() + AddLogRecordMigration(), + AddProcessIdentifierToSpanRecordMigration() ] } } diff --git a/Sources/EmbraceStorage/Records/SpanRecord.swift b/Sources/EmbraceStorage/Records/SpanRecord.swift index 129353d8..e8cb623d 100644 --- a/Sources/EmbraceStorage/Records/SpanRecord.swift +++ b/Sources/EmbraceStorage/Records/SpanRecord.swift @@ -15,6 +15,7 @@ public struct SpanRecord: Codable { public var data: Data public var startTime: Date public var endTime: Date? + public var processIdentifier: ProcessIdentifier public init( id: String, @@ -23,7 +24,8 @@ public struct SpanRecord: Codable { type: SpanType, data: Data, startTime: Date, - endTime: Date? = nil + endTime: Date? = nil, + processIdentifier: ProcessIdentifier = .current ) { self.id = id self.traceId = traceId @@ -32,6 +34,7 @@ public struct SpanRecord: Codable { self.startTime = startTime self.endTime = endTime self.name = name + self.processIdentifier = processIdentifier } } @@ -44,6 +47,7 @@ extension SpanRecord { static var startTime: Column { Column("start_time") } static var endTime: Column { Column("end_time") } static var name: Column { Column("name") } + static var processIdentifier: Column { Column("process_identifier") } } } diff --git a/Tests/EmbraceCoreTests/Internal/StorageSpanExporterTests.swift b/Tests/EmbraceCoreTests/Internal/StorageSpanExporterTests.swift index ba550ddc..7cf7d7f5 100644 --- a/Tests/EmbraceCoreTests/Internal/StorageSpanExporterTests.swift +++ b/Tests/EmbraceCoreTests/Internal/StorageSpanExporterTests.swift @@ -48,10 +48,10 @@ final class StorageSpanExporterTests: XCTestCase { let exportedSpans: [SpanRecord] = try storage.fetchAll() XCTAssertTrue(exportedSpans.count == 1) - let exportedSpan = exportedSpans.first - XCTAssertEqual(exportedSpan?.traceId, traceId.hexString) - XCTAssertEqual(exportedSpan?.id, spanId.hexString) - XCTAssertEqual(exportedSpan!.endTime!.timeIntervalSince1970, endTime.timeIntervalSince1970, accuracy: 0.01) + let exportedSpan = try XCTUnwrap(exportedSpans.first) + XCTAssertEqual(exportedSpan.traceId, traceId.hexString) + XCTAssertEqual(exportedSpan.id, spanId.hexString) + XCTAssertEqual(exportedSpan.endTime!.timeIntervalSince1970, endTime.timeIntervalSince1970, accuracy: 0.01) } func test_DB_allowsOpenSpan_toUpdateAttributes() throws { diff --git a/Tests/EmbraceStorageTests/EmbraceStorageTests.swift b/Tests/EmbraceStorageTests/EmbraceStorageTests.swift index 2c6130cb..5b6b7369 100644 --- a/Tests/EmbraceStorageTests/EmbraceStorageTests.swift +++ b/Tests/EmbraceStorageTests/EmbraceStorageTests.swift @@ -51,11 +51,8 @@ class EmbraceStorageTests: XCTestCase { func test_performMigration_ifResetsIfErrorTrue_resetsDB() throws { storage = try EmbraceStorage.createInMemoryDb() - let service = ThrowingMigrationService(performToThrow: 1) - try storage.performMigration( - resetIfError: true, - service: service - ) + let migration = ThrowingMigration(performToThrow: 1) + try storage.performMigration(resetIfError: true, migrations: .current + [migration]) try storage.dbQueue.read { db in XCTAssertTrue(try db.tableExists(SessionRecord.databaseTableName)) @@ -64,34 +61,34 @@ class EmbraceStorageTests: XCTestCase { XCTAssertTrue(try db.tableExists(LogRecord.databaseTableName)) } - XCTAssertEqual(service.currentPerformCount, 2) + XCTAssertEqual(migration.currentPerformCount, 2) } func test_performMigration_ifResetsIfErrorTrue_andMigrationFailsTwice_rethrowsError() throws { storage = try EmbraceStorage.createInMemoryDb() - let service = ThrowingMigrationService(performsToThrow: [1, 2]) + let migration = ThrowingMigration(performsToThrow: [1, 2]) XCTAssertThrowsError( try storage.performMigration( resetIfError: true, - service: service + migrations: [migration] ) ) - XCTAssertEqual(service.currentPerformCount, 2) + XCTAssertEqual(migration.currentPerformCount, 2) } func test_performMigration_ifResetsIfErrorFalse_rethrowsError() throws { storage = try EmbraceStorage.createInMemoryDb() - let service = ThrowingMigrationService(performToThrow: 1) + let migration = ThrowingMigration(performToThrow: 1) XCTAssertThrowsError( try storage.performMigration( resetIfError: false, - service: service + migrations: [migration] ) ) - XCTAssertEqual(service.currentPerformCount, 1) + XCTAssertEqual(migration.currentPerformCount, 1) } func test_reset_remakesDB() throws { diff --git a/Tests/EmbraceStorageTests/Migration/Migrations/20240523_00_AddProcessIdentifierToSpanRecordMigrationTests.swift b/Tests/EmbraceStorageTests/Migration/Migrations/20240523_00_AddProcessIdentifierToSpanRecordMigrationTests.swift new file mode 100644 index 00000000..b11859da --- /dev/null +++ b/Tests/EmbraceStorageTests/Migration/Migrations/20240523_00_AddProcessIdentifierToSpanRecordMigrationTests.swift @@ -0,0 +1,222 @@ +// +// Copyright © 2023 Embrace Mobile, Inc. All rights reserved. +// + +import XCTest + +@testable import EmbraceStorage +import EmbraceOTel +import EmbraceCommon +import GRDB + +final class _0240523_00_AddProcessIdentifierToSpanRecordMigration: XCTestCase { + + var storage: EmbraceStorage! + + override func setUpWithError() throws { + storage = try EmbraceStorage.createInMemoryDb(runMigrations: false) + } + + override func tearDownWithError() throws { + try storage.teardown() + } + + func test_identifier() { + let migration = AddProcessIdentifierToSpanRecordMigration() + XCTAssertEqual(migration.identifier, "AddProcessIdentifierToSpanRecord") + } + + func test_perform_withNoExistingRecords() throws { + let migration = AddProcessIdentifierToSpanRecordMigration() + try storage.performMigration(migrations: .current.upTo(identifier: migration.identifier)) + + try storage.dbQueue.write { db in + try migration.perform(db) + } + + try storage.dbQueue.read { db in + XCTAssertTrue(try db.tableExists(SpanRecord.databaseTableName)) + + let columns = try db.columns(in: SpanRecord.databaseTableName) + XCTAssertEqual(columns.count, 8) + + // primary key + XCTAssert(try db.table( + SpanRecord.databaseTableName, + hasUniqueKey: [ + SpanRecord.Schema.traceId.name, + SpanRecord.Schema.id.name + ] + )) + + // id + let idColumn = columns.first { info in + info.name == SpanRecord.Schema.id.name && + info.type == "TEXT" && + info.isNotNull == true + } + XCTAssertNotNil(idColumn) + + // name + let nameColumn = columns.first { info in + info.name == SpanRecord.Schema.name.name && + info.type == "TEXT" && + info.isNotNull == true + } + XCTAssertNotNil(nameColumn) + + // trace_id + let traceIdColumn = columns.first { info in + info.name == SpanRecord.Schema.traceId.name && + info.type == "TEXT" && + info.isNotNull == true + } + XCTAssertNotNil(traceIdColumn) + + // type + let typeColumn = columns.first { info in + info.name == SpanRecord.Schema.type.name && + info.type == "TEXT" && + info.isNotNull == true + } + XCTAssertNotNil(typeColumn) + + // start_time + let startTimeColumn = columns.first { info in + info.name == SpanRecord.Schema.startTime.name && + info.type == "DATETIME" && + info.isNotNull == true + } + XCTAssertNotNil(startTimeColumn) + + // end_time + let endTimeColumn = columns.first { info in + info.name == SpanRecord.Schema.endTime.name && + info.type == "DATETIME" && + info.isNotNull == false + } + XCTAssertNotNil(endTimeColumn) + + // data + let dataColumn = columns.first { info in + info.name == SpanRecord.Schema.data.name && + info.type == "BLOB" && + info.isNotNull == true + } + XCTAssertNotNil(dataColumn) + + // process_identifier + let processIdentifier = columns.first { info in + info.name == SpanRecord.Schema.processIdentifier.name && + info.type == "TEXT" && + info.isNotNull == true + } + XCTAssertNotNil(processIdentifier) + } + } + + func test_perform_migratesExistingEntries() throws { + let migration = AddProcessIdentifierToSpanRecordMigration() + try storage.performMigration(migrations: .current.upTo(identifier: migration.identifier)) + + try storage.dbQueue.write { db in + try db.execute(sql: """ + INSERT INTO 'spans' ( + 'id', + 'trace_id', + 'name', + 'type', + 'start_time', + 'end_time', + 'data' + ) VALUES ( + ?, + ?, + ?, + ?, + ?, + ?, + ? + ); + """, arguments: [ + "3d9381a7f8300102", + "b65cd80e1bea6fd2c27150f8cce3de3e", + "example-name", + SpanType.performance, + Date(), + Date(timeIntervalSinceNow: 2), + Data() + ]) + } + + try storage.dbQueue.write { db in + try migration.perform(db) + } + + try storage.dbQueue.read { db in + let rows = try Row.fetchAll(db, sql: "SELECT * from spans") + XCTAssertEqual(rows.count, 1) + + let records = try SpanRecord.fetchAll(db) + XCTAssertEqual(records.count, 1) + records.forEach { record in + XCTAssertEqual(record.processIdentifier, ProcessIdentifier(hex: "c0ffee")) + } + } + } + + func test_perform_migratesExistingEntries_whenMultiple() throws { + let migration = AddProcessIdentifierToSpanRecordMigration() + try storage.performMigration(migrations: .current.upTo(identifier: migration.identifier)) + + let count = 10 + for _ in 0.. Self { + let idx = firstIndex { element in + type(of: element).identifier == identifier + } + + guard let idx = idx else { + return [] + } + + return Array(prefix(idx)) + } +} diff --git a/Tests/EmbraceStorageTests/TestSupport/ThrowingMigrationService.swift b/Tests/EmbraceStorageTests/TestSupport/ThrowingMigration.swift similarity index 77% rename from Tests/EmbraceStorageTests/TestSupport/ThrowingMigrationService.swift rename to Tests/EmbraceStorageTests/TestSupport/ThrowingMigration.swift index 6587db60..e23a9761 100644 --- a/Tests/EmbraceStorageTests/TestSupport/ThrowingMigrationService.swift +++ b/Tests/EmbraceStorageTests/TestSupport/ThrowingMigration.swift @@ -6,27 +6,29 @@ import Foundation import EmbraceStorage import GRDB -class ThrowingMigrationService: MigrationServiceProtocol { +class ThrowingMigration: Migration { enum MigrationServiceError: Error { case alwaysError } + static var identifier = "AlwaysThrowingMigration" + let performsToThrow: [Int] private(set) var currentPerformCount: Int = 0 /// - Parameters: /// - performToThrow: The invocation of `perform` that should throw. Defaults to 1, the first call to `perform`. - init(performToThrow: Int = 1) { - self.performsToThrow = [performToThrow] + init(performsToThrow: [Int]) { + self.performsToThrow = performsToThrow } /// - Parameters: /// - performToThrow: The invocation of `perform` that should throw. Defaults to 1, the first call to `perform`. - init(performsToThrow: [Int]) { - self.performsToThrow = performsToThrow + convenience init(performToThrow: Int = 1) { + self.init(performsToThrow: [performToThrow]) } - func perform(_ dbQueue: DatabaseWriter, migrations: [Migration]) throws { + func perform(_ db: GRDB.Database) throws { currentPerformCount += 1 if performsToThrow.contains(currentPerformCount) { throw MigrationServiceError.alwaysError