Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUMM-2133 Register Tracing v1 in DatadogCore #857

Merged
merged 1 commit into from
May 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions Sources/Datadog/Datadog.swift
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ public class Datadog {
public static func clearAllData() {
let logging = defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName)
logging?.storage.clearAllData()
TracingFeature.instance?.storage.clearAllData()
let tracing = defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName)
tracing?.storage.clearAllData()
RUMFeature.instance?.storage.clearAllData()
}

Expand Down Expand Up @@ -270,10 +271,12 @@ public class Datadog {
directories: try obtainTracingFeatureDirectories(),
configuration: tracingConfiguration,
commonDependencies: commonDependencies,
loggingFeatureAdapter: logging.flatMap { LoggingForTracingAdapter(loggingFeature: $0) },
loggingFeatureAdapter: logging.map { LoggingForTracingAdapter(loggingFeature: $0) },
Comment on lines -273 to +274
Copy link
Member

Choose a reason for hiding this comment

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

I just wonder, was map always possible in such context or has it changed when compactMap was introduced?

Copy link
Member Author

Choose a reason for hiding this comment

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

map was always possible here as we return a non-optional value. map is just syntactically more correct!

tracingUUIDGenerator: DefaultTracingUUIDGenerator(),
telemetry: telemetry
)

defaultDatadogCore.registerFeature(named: TracingFeature.featureName, instance: tracing)
}

if let crashReportingConfiguration = configuration.crashReporting {
Expand All @@ -291,7 +294,6 @@ public class Datadog {
)
}

TracingFeature.instance = tracing
RUMFeature.instance = rum
CrashReportingFeature.instance = crashReporting

Expand Down Expand Up @@ -332,8 +334,8 @@ public class Datadog {
// Tear down and deinitialize all features:
let logging = defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName)
logging?.deinitialize()

TracingFeature.instance?.deinitialize()
let tracing = defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName)
tracing?.deinitialize()
RUMFeature.instance?.deinitialize()
CrashReportingFeature.instance?.deinitialize()

Expand Down
6 changes: 5 additions & 1 deletion Sources/Datadog/Logger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -409,13 +409,17 @@ public class Logger {

let (logBuilder, logOutput) = resolveLogBuilderAndOutput(for: loggingFeature) ?? (nil, nil)

// RUMM-2133 Note: strong feature coupling while migrating to v2.
// In v2 active span will be provided in context from feature scope.
let tracingEnabled = core.feature(TracingFeature.self, named: TracingFeature.featureName) != nil

return Logger(
logBuilder: logBuilder,
logOutput: logOutput,
dateProvider: loggingFeature.dateProvider,
identifier: resolveLoggerName(for: loggingFeature),
rumContextIntegration: (RUMFeature.isEnabled && bundleWithRUM) ? LoggingWithRUMContextIntegration() : nil,
activeSpanIntegration: (TracingFeature.isEnabled && bundleWithTrace) ? LoggingWithActiveSpanIntegration() : nil
activeSpanIntegration: (tracingEnabled && bundleWithTrace) ? LoggingWithActiveSpanIntegration() : nil
)
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/Datadog/Tracer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public class Tracer: OTTracer {
/// Initializes the Datadog Tracer.
/// - Parameters:
/// - configuration: the tracer configuration obtained using `Tracer.Configuration()`.
public static func initialize(configuration: Configuration) -> OTTracer {
public static func initialize(configuration: Configuration, in core: DatadogCoreProtocol = defaultDatadogCore) -> OTTracer {
do {
if Global.sharedTracer is Tracer {
throw ProgrammerError(
Expand All @@ -78,7 +78,7 @@ public class Tracer: OTTracer {
"""
)
}
guard let tracingFeature = TracingFeature.instance else {
guard let tracingFeature = core.feature(TracingFeature.self, named: TracingFeature.featureName) else {
throw ProgrammerError(
description: Datadog.isInitialized
? "`Tracer.initialize(configuration:)` produces a non-functional tracer, as the tracing feature is disabled."
Expand Down
7 changes: 0 additions & 7 deletions Sources/Datadog/Tracing/TracingFeature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ internal func obtainTracingFeatureDirectories() throws -> FeatureDirectories {
/// Creates and owns componetns enabling tracing feature.
/// Bundles dependencies for other tracing-related components created later at runtime (i.e. `Tracer`).
internal final class TracingFeature {
/// Single, shared instance of `TracingFeatureFeature`.
internal static var instance: TracingFeature?

/// Tells if the feature was enabled by the user in the SDK configuration.
static var isEnabled: Bool { instance != nil }

// MARK: - Configuration

let configuration: FeaturesConfiguration.Tracing
Expand Down Expand Up @@ -171,6 +165,5 @@ internal final class TracingFeature {
internal func deinitialize() {
storage.flushAndTearDown()
upload.flushAndTearDown()
TracingFeature.instance = nil
}
}
43 changes: 26 additions & 17 deletions Tests/DatadogTests/Datadog/DatadogTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,53 +115,58 @@ class DatadogTests: XCTestCase {
verify(configuration: defaultBuilder.build()) {
// verify features:
XCTAssertNotNil(defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName))
XCTAssertTrue(TracingFeature.isEnabled)
XCTAssertFalse(RUMFeature.isEnabled, "When using `defaultBuilder` RUM feature should be disabled by default")
XCTAssertFalse(CrashReportingFeature.isEnabled)
XCTAssertNil(RUMInstrumentation.instance)
XCTAssertNil(URLSessionAutoInstrumentation.instance)
// verify integrations:
XCTAssertNotNil(TracingFeature.instance?.loggingFeatureAdapter)
let tracing = defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName)
XCTAssertNotNil(tracing)
XCTAssertNotNil(tracing?.loggingFeatureAdapter)
}
verify(configuration: rumBuilder.build()) {
// verify features:
XCTAssertNotNil(defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName))
XCTAssertTrue(TracingFeature.isEnabled)
XCTAssertTrue(RUMFeature.isEnabled, "When using `rumBuilder` RUM feature should be enabled by default")
XCTAssertFalse(CrashReportingFeature.isEnabled)
XCTAssertNotNil(RUMInstrumentation.instance)
XCTAssertNil(URLSessionAutoInstrumentation.instance)
// verify integrations:
XCTAssertNotNil(TracingFeature.instance?.loggingFeatureAdapter)
let tracing = defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName)
XCTAssertNotNil(tracing)
XCTAssertNotNil(tracing?.loggingFeatureAdapter)
}

verify(configuration: defaultBuilder.enableLogging(false).build()) {
// verify features:
XCTAssertNil(defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName))
XCTAssertTrue(TracingFeature.isEnabled)
XCTAssertFalse(RUMFeature.isEnabled, "When using `defaultBuilder` RUM feature should be disabled by default")
XCTAssertFalse(CrashReportingFeature.isEnabled)
XCTAssertNil(RUMInstrumentation.instance)
XCTAssertNil(URLSessionAutoInstrumentation.instance)
// verify integrations:
XCTAssertNil(TracingFeature.instance?.loggingFeatureAdapter)
let tracing = defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName)
XCTAssertNotNil(tracing)
XCTAssertNil(tracing?.loggingFeatureAdapter)
}
verify(configuration: rumBuilder.enableLogging(false).build()) {
// verify features:
XCTAssertNil(defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName))
XCTAssertTrue(TracingFeature.isEnabled)
XCTAssertNotNil(defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName))
XCTAssertTrue(RUMFeature.isEnabled, "When using `rumBuilder` RUM feature should be enabled by default")
XCTAssertFalse(CrashReportingFeature.isEnabled)
XCTAssertNotNil(RUMInstrumentation.instance)
XCTAssertNil(URLSessionAutoInstrumentation.instance)
// verify integrations:
XCTAssertNil(TracingFeature.instance?.loggingFeatureAdapter)
let tracing = defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName)
XCTAssertNotNil(tracing)
XCTAssertNil(tracing?.loggingFeatureAdapter)
}

verify(configuration: defaultBuilder.enableTracing(false).build()) {
// verify features:
XCTAssertNotNil(defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName))
XCTAssertFalse(TracingFeature.isEnabled)
XCTAssertNil(defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName))
XCTAssertFalse(RUMFeature.isEnabled, "When using `defaultBuilder` RUM feature should be disabled by default")
XCTAssertFalse(CrashReportingFeature.isEnabled)
XCTAssertNil(RUMInstrumentation.instance)
Expand All @@ -170,7 +175,7 @@ class DatadogTests: XCTestCase {
verify(configuration: rumBuilder.enableTracing(false).build()) {
// verify features:
XCTAssertNotNil(defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName))
XCTAssertFalse(TracingFeature.isEnabled)
XCTAssertNil(defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName))
XCTAssertTrue(RUMFeature.isEnabled, "When using `rumBuilder` RUM feature should be enabled by default")
XCTAssertFalse(CrashReportingFeature.isEnabled)
XCTAssertNotNil(RUMInstrumentation.instance)
Expand All @@ -180,24 +185,26 @@ class DatadogTests: XCTestCase {
verify(configuration: defaultBuilder.enableRUM(true).build()) {
// verify features:
XCTAssertNotNil(defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName))
XCTAssertTrue(TracingFeature.isEnabled)
XCTAssertFalse(RUMFeature.isEnabled, "When using `defaultBuilder` RUM feature cannot be enabled")
XCTAssertFalse(CrashReportingFeature.isEnabled)
XCTAssertNil(RUMInstrumentation.instance)
XCTAssertNil(URLSessionAutoInstrumentation.instance)
// verify integrations:
XCTAssertNotNil(TracingFeature.instance?.loggingFeatureAdapter)
let tracing = defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName)
XCTAssertNotNil(tracing)
XCTAssertNotNil(tracing?.loggingFeatureAdapter)
}
verify(configuration: rumBuilder.enableRUM(false).build()) {
// verify features:
XCTAssertNotNil(defaultDatadogCore.feature(LoggingFeature.self, named: LoggingFeature.featureName))
XCTAssertTrue(TracingFeature.isEnabled)
XCTAssertFalse(RUMFeature.isEnabled)
XCTAssertFalse(CrashReportingFeature.isEnabled)
XCTAssertNil(RUMInstrumentation.instance)
XCTAssertNil(URLSessionAutoInstrumentation.instance)
// verify integrations:
XCTAssertNotNil(TracingFeature.instance?.loggingFeatureAdapter)
let tracing = defaultDatadogCore.feature(TracingFeature.self, named: TracingFeature.featureName)
XCTAssertNotNil(tracing)
XCTAssertNotNil(tracing?.loggingFeatureAdapter)
}

verify(configuration: rumBuilder.trackUIKitRUMViews().build()) {
Expand Down Expand Up @@ -317,9 +324,10 @@ class DatadogTests: XCTestCase {

let core = defaultDatadogCore
let logging = core.feature(LoggingFeature.self, named: LoggingFeature.featureName)
let tracing = core.feature(TracingFeature.self, named: TracingFeature.featureName)
XCTAssertEqual(logging?.configuration.common.performance, expectedPerformancePreset)
XCTAssertEqual(RUMFeature.instance?.configuration.sessionSampler.samplingRate, 100)
XCTAssertEqual(TracingFeature.instance?.configuration.common.performance, expectedPerformancePreset)
XCTAssertEqual(tracing?.configuration.common.performance, expectedPerformancePreset)
XCTAssertEqual(Datadog.verbosityLevel, .debug)

// Clear default verbosity after this test
Expand Down Expand Up @@ -440,12 +448,13 @@ class DatadogTests: XCTestCase {

let core = defaultDatadogCore
let logging = core.feature(LoggingFeature.self, named: LoggingFeature.featureName)
let tracing = core.feature(TracingFeature.self, named: TracingFeature.featureName)

// On SDK init, underlying `ConsentAwareDataWriter` performs data migration for each feature, which includes
// data removal in `unauthorised` (`.pending`) directory. To not cause test flakiness, we must ensure that
// mock data is written only after this operation completes - otherwise, migration may delete mocked files.
let loggingWriter = try XCTUnwrap(logging?.storage.writer as? ConsentAwareDataWriter)
let tracingWriter = try XCTUnwrap(TracingFeature.instance?.storage.writer as? ConsentAwareDataWriter)
let tracingWriter = try XCTUnwrap(tracing?.storage.writer as? ConsentAwareDataWriter)
let rumWriter = try XCTUnwrap(RUMFeature.instance?.storage.writer as? ConsentAwareDataWriter)
loggingWriter.queue.sync {}
tracingWriter.queue.sync {}
Expand All @@ -469,7 +478,7 @@ class DatadogTests: XCTestCase {

// Wait for async clear completion in all features:
(logging?.storage.dataOrchestrator as? DataOrchestrator)?.queue.sync {}
(TracingFeature.instance?.storage.dataOrchestrator as? DataOrchestrator)?.queue.sync {}
(tracing?.storage.dataOrchestrator as? DataOrchestrator)?.queue.sync {}
(RUMFeature.instance?.storage.dataOrchestrator as? DataOrchestrator)?.queue.sync {}

// Then
Expand Down
8 changes: 4 additions & 4 deletions Tests/DatadogTests/Datadog/LoggerBuilderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ class LoggerBuilderTests: XCTestCase {
}

func testDefaultLoggerWithTracingEnabled() throws {
TracingFeature.instance = .mockNoOp()
defer { TracingFeature.instance?.deinitialize() }
let tracing: TracingFeature = .mockNoOp()
core.registerFeature(named: TracingFeature.featureName, instance: tracing)

let logger1 = Logger.builder.build(in: core)
XCTAssertNotNil(logger1.activeSpanIntegration)
Expand All @@ -93,8 +93,8 @@ class LoggerBuilderTests: XCTestCase {
RUMFeature.instance = .mockNoOp()
defer { RUMFeature.instance?.deinitialize() }

TracingFeature.instance = .mockNoOp()
defer { TracingFeature.instance?.deinitialize() }
let tracing: TracingFeature = .mockNoOp()
core.registerFeature(named: TracingFeature.featureName, instance: tracing)

let logger = Logger.builder
.set(serviceName: "custom-service-name")
Expand Down
14 changes: 5 additions & 9 deletions Tests/DatadogTests/Datadog/LoggerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -632,15 +632,13 @@ class LoggerTests: XCTestCase {

func testGivenBundlingWithTraceEnabledAndTracerRegistered_whenSendingLog_itContainsActiveSpanAttributes() throws {
let feature: LoggingFeature = .mockByRecordingLogMatchers(directories: temporaryFeatureDirectories)
defer { feature.deinitialize() }
let tracing: TracingFeature = .mockNoOp()
core.registerFeature(named: LoggingFeature.featureName, instance: feature)

TracingFeature.instance = .mockNoOp()
defer { TracingFeature.instance?.deinitialize() }
core.registerFeature(named: TracingFeature.featureName, instance: tracing)

// given
let logger = Logger.builder.build(in: core)
Global.sharedTracer = Tracer.initialize(configuration: .init())
Global.sharedTracer = Tracer.initialize(configuration: .init(), in: core)
defer { Global.sharedTracer = DDNoopGlobals.tracer }

// when
Expand All @@ -665,11 +663,9 @@ class LoggerTests: XCTestCase {

func testGivenBundlingWithTraceEnabledButTracerNotRegistered_whenSendingLog_itPrintsWarning() throws {
let feature: LoggingFeature = .mockByRecordingLogMatchers(directories: temporaryFeatureDirectories)
defer { feature.deinitialize() }
let tracing: TracingFeature = .mockNoOp()
core.registerFeature(named: LoggingFeature.featureName, instance: feature)

TracingFeature.instance = .mockNoOp()
defer { TracingFeature.instance?.deinitialize() }
core.registerFeature(named: TracingFeature.featureName, instance: tracing)

let previousUserLogger = userLogger
defer { userLogger = previousUserLogger }
Expand Down
6 changes: 6 additions & 0 deletions Tests/DatadogTests/Datadog/Mocks/DatadogCoreMock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,9 @@ extension LoggingFeature: Flushable {
deinitialize()
}
}

extension TracingFeature: Flushable {
func flush() {
deinitialize()
}
}
15 changes: 12 additions & 3 deletions Tests/DatadogTests/Datadog/Mocks/TracingFeatureMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ extension TracingFeature {
directories: directories,
configuration: configuration,
commonDependencies: dependencies,
loggingFeatureAdapter: loggingFeature.flatMap { LoggingForTracingAdapter(loggingFeature: $0) },
loggingFeatureAdapter: loggingFeature.map { LoggingForTracingAdapter(loggingFeature: $0) },
tracingUUIDGenerator: tracingUUIDGenerator,
telemetry: telemetry
)
Expand Down Expand Up @@ -79,13 +79,22 @@ extension TracingFeature {

// MARK: - Expecting Spans Data

static func waitAndReturnSpanMatchers(count: UInt, file: StaticString = #file, line: UInt = #line) throws -> [SpanMatcher] {
guard let uploadWorker = TracingFeature.instance?.upload.uploader as? DataUploadWorkerMock else {
func waitAndReturnSpanMatchers(count: UInt, file: StaticString = #file, line: UInt = #line) throws -> [SpanMatcher] {
guard let uploadWorker = upload.uploader as? DataUploadWorkerMock else {
preconditionFailure("Retrieving matchers requires that feature is mocked with `.mockByRecordingSpanMatchers()`")
}
return try uploadWorker.waitAndReturnBatchedData(count: count, file: file, line: line)
.flatMap { batchData in try SpanMatcher.fromNewlineSeparatedJSONObjectsData(batchData) }
}

// swiftlint:disable:next function_default_parameter_at_end
static func waitAndReturnSpanMatchers(in core: DatadogCoreProtocol = defaultDatadogCore, count: UInt, file: StaticString = #file, line: UInt = #line) throws -> [SpanMatcher] {
guard let tracing = core.feature(TracingFeature.self, named: TracingFeature.featureName) else {
preconditionFailure("TracingFeature is not registered in core")
}

return try tracing.waitAndReturnSpanMatchers(count: count, file: file, line: line)
}
}

// MARK: - Span Mocks
Expand Down
Loading