Skip to content

Commit

Permalink
Added some checks to prevent usage of SDK when is disabled (#147)
Browse files Browse the repository at this point in the history
  • Loading branch information
ArielDemarco authored Dec 10, 2024
1 parent d267192 commit 3df729f
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 8 deletions.
2 changes: 1 addition & 1 deletion Examples/BrandGame/BrandGame/Embrace/App+Embrace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ extension BrandGameApp {

private var otelExport: OpenTelemetryExport {
OpenTelemetryExport(
spanExporter: StdoutExporter(isDebug: true),
spanExporter: StdoutSpanExporter(isDebug: true),
logExporter: StdoutLogExporter(isDebug: true)
)
}
Expand Down
14 changes: 13 additions & 1 deletion Sources/EmbraceCore/Embrace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ To start the SDK you first need to configure it using an `Embrace.Options` insta
/// Returns the current `MetadataHandler` used to store resources and session properties.
@objc public let metadata: MetadataHandler

var isSDKEnabled: Bool {
if let config = config {
return config.isSDKEnabled
}
return true
}

let config: EmbraceConfig?
let storage: EmbraceStorage
let upload: EmbraceUpload?
Expand Down Expand Up @@ -149,7 +156,8 @@ To start the SDK you first need to configure it using an `Embrace.Options` insta
self.logController = logControllable ?? LogController(
storage: storage,
upload: upload,
controller: sessionController
controller: sessionController,
config: config
)
super.init()

Expand Down Expand Up @@ -258,6 +266,10 @@ To start the SDK you first need to configure it using an `Embrace.Options` insta
@objc private func onConfigUpdated() {
if let config = config {
Embrace.logger.limits = config.internalLogLimits
if !config.isSDKEnabled {
Embrace.logger.debug("SDK was disabled")
captureServices.stop()
}
}
}
}
21 changes: 20 additions & 1 deletion Sources/EmbraceCore/Internal/Logs/LogController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import EmbraceStorageInternal
import EmbraceUploadInternal
import EmbraceCommonInternal
import EmbraceSemantics
import EmbraceConfigInternal

protocol LogControllable: LogBatcherDelegate {
func uploadAllPersistedLogs()
Expand All @@ -16,16 +17,26 @@ class LogController: LogControllable {
private(set) weak var sessionController: SessionControllable?
private weak var storage: Storage?
private weak var upload: EmbraceLogUploader?
private weak var config: EmbraceConfig?
/// This will probably be injected eventually.
/// For consistency, I created a constant
static let maxLogsPerBatch: Int = 20

private var isSDKEnabled: Bool {
guard let config = config else {
return true
}
return config.isSDKEnabled
}

init(storage: Storage?,
upload: EmbraceLogUploader?,
controller: SessionControllable) {
controller: SessionControllable,
config: EmbraceConfig?) {
self.storage = storage
self.upload = upload
self.sessionController = controller
self.config = config
}

func uploadAllPersistedLogs() {
Expand All @@ -46,6 +57,10 @@ class LogController: LogControllable {

extension LogController {
func batchFinished(withLogs logs: [LogRecord]) {
guard isSDKEnabled else {
return
}

do {
guard let sessionId = sessionController?.currentSession?.id, logs.count > 0 else {
return
Expand All @@ -61,6 +76,10 @@ extension LogController {

private extension LogController {
func send(batches: [LogsBatch]) {
guard isSDKEnabled else {
return
}

guard batches.count > 0 else {
return
}
Expand Down
1 change: 0 additions & 1 deletion Sources/EmbraceCore/Public/Embrace+OTel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ extension Embrace: EmbraceOpenTelemetry {
attributes: [String: String] = [:],
autoTerminationCode: SpanErrorCode? = nil
) -> SpanBuilder {

if let autoTerminationCode = autoTerminationCode {
var attributes = attributes
attributes[SpanSemantics.keyAutoTerminationCode] = autoTerminationCode.rawValue
Expand Down
16 changes: 16 additions & 0 deletions Sources/EmbraceCore/Session/SessionController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ class SessionController: SessionControllable {
endSession()
}

guard isSDKEnabled else {
return nil
}

// detect cold start
let isColdStart = firstSession

Expand Down Expand Up @@ -149,6 +153,11 @@ class SessionController: SessionControllable {
heartbeat.stop()
let now = Date()

guard isSDKEnabled else {
delete()
return now
}

// If the session is a background session and background sessions
// are disabled in the config, we drop the session!
// +
Expand Down Expand Up @@ -244,6 +253,13 @@ extension SessionController {
currentSession = nil
currentSessionSpan = nil
}

private var isSDKEnabled: Bool {
guard let config = config else {
return true
}
return config.isSDKEnabled
}
}

// internal use
Expand Down
38 changes: 35 additions & 3 deletions Tests/EmbraceCoreTests/Internal/Logs/LogControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,18 @@ import XCTest
import EmbraceStorageInternal
import EmbraceUploadInternal
import EmbraceCommonInternal
import EmbraceConfigInternal

class LogControllerTests: XCTestCase {
private var sut: LogController!
private var storage: SpyStorage?
private var sessionController: MockSessionController!
private var upload: SpyEmbraceLogUploader!
private var config: EmbraceConfig!

override func setUp() {
givenEmbraceLogUploader()
givenConfig()
givenSessionControllerWithSession()
givenStorage()
}
Expand Down Expand Up @@ -91,6 +94,15 @@ class LogControllerTests: XCTestCase {
thenLogUploadShouldUpload(times: 1)
}

func testSDKDisabledHavingLogsForLessThanABatch_onSetup_logUploaderShouldntSendASingleBatch() throws {
givenStorage(withLogs: [randomLogRecord(), randomLogRecord()])
givenConfig(sdkEnabled: false)
givenLogController()
whenInvokingSetup()
thenLogUploadShouldUpload(times: 0)
try thenStorageShouldntCallRemoveLogs()
}

func testHavingLogsForMoreThanABatch_onSetup_logUploaderShouldSendTwoBatches() {
givenStorage(withLogs: logsForMoreThanASingleBatch())
givenLogController()
Expand Down Expand Up @@ -150,6 +162,13 @@ class LogControllerTests: XCTestCase {
try thenFetchesMetadataFromStorage(sessionId: sessionController.currentSession?.id)
}

func testSDKDisabledHavingLogs_onBatchFinished_ontTryToUploadAnything() throws {
givenConfig(sdkEnabled: false)
givenLogController()
whenInvokingBatchFinished(withLogs: [randomLogRecord()])
thenDoesntTryToUploadAnything()
}

func testHavingThrowingStorage_onBatchFinished_wontTryToUploadAnything() {
givenStorageThatThrowsException()
givenLogController()
Expand Down Expand Up @@ -184,11 +203,21 @@ class LogControllerTests: XCTestCase {

private extension LogControllerTests {
func givenLogControllerWithNoStorage() {
sut = .init(storage: nil, upload: upload, controller: sessionController)
sut = .init(
storage: nil,
upload: upload,
controller: sessionController,
config: config
)
}

func givenLogController() {
sut = .init(storage: storage, upload: upload, controller: sessionController)
sut = .init(
storage: storage,
upload: upload,
controller: sessionController,
config: config
)
}

func givenEmbraceLogUploader() {
Expand All @@ -201,6 +230,10 @@ private extension LogControllerTests {
upload.stubbedCompletion = .failure(RandomError())
}

func givenConfig(sdkEnabled: Bool = true) {
config = EmbraceConfigMock.default(sdkEnabled: sdkEnabled)
}

func givenSessionControllerWithoutSession() {
sessionController = .init()
}
Expand Down Expand Up @@ -250,7 +283,6 @@ private extension LogControllerTests {
}

func thenLogUploadShouldUpload(times: Int) {
XCTAssertTrue(upload.didCallUploadLog)
XCTAssertEqual(upload.didCallUploadLogCount, times)
}

Expand Down
17 changes: 17 additions & 0 deletions Tests/EmbraceCoreTests/Session/SessionControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ final class SessionControllerTests: XCTestCase {

var storage: EmbraceStorage!
var controller: SessionController!
var config: EmbraceConfig!
var upload: EmbraceUpload!

static let testMetadataOptions = EmbraceUpload.MetadataOptions(
Expand Down Expand Up @@ -67,6 +68,22 @@ final class SessionControllerTests: XCTestCase {
XCTAssertNotEqual(b!.id, c!.id)
}

func testSDKDisabled_startSession_doesntCreateASession() throws {
config = EmbraceConfigMock.default(sdkEnabled: false)

controller = SessionController(
storage: storage,
upload: upload,
config: config
)

let session = controller.startSession(state: .foreground)

XCTAssertNil(session)
XCTAssertNil(controller.currentSessionSpan)
}


func test_startSession_setsForegroundState() throws {
let a = controller.startSession(state: .foreground)
XCTAssertEqual(a!.state, "foreground")
Expand Down
7 changes: 6 additions & 1 deletion Tests/EmbraceCoreTests/Session/UnsentDataHandlerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,12 @@ class UnsentDataHandlerTests: XCTestCase {
defer { try? storage.teardown() }

let upload = try EmbraceUpload(options: uploadOptions, logger: logger, queue: queue, semaphore: .init(value: .max))
let logController = LogController(storage: storage, upload: upload, controller: MockSessionController())
let logController = LogController(
storage: storage,
upload: upload,
controller: MockSessionController(),
config: EmbraceConfigMock.default()
)
let otel = MockEmbraceOpenTelemetry()

// given logs in storage
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//
// Copyright © 2024 Embrace Mobile, Inc. All rights reserved.
//

@testable import EmbraceConfigInternal
import TestSupport

class EmbraceConfigMock {
static func `default`(sdkEnabled: Bool = true) -> EmbraceConfig {
EmbraceConfig(
configurable: MockEmbraceConfigurable(isSDKEnabled: sdkEnabled),
options: .init(minimumUpdateInterval: .infinity),
notificationCenter: .default,
logger: MockLogger()
)
}
}

0 comments on commit 3df729f

Please sign in to comment.