Skip to content

Commit

Permalink
Add test and fix issues in log binary image attachement
Browse files Browse the repository at this point in the history
  • Loading branch information
fuzzybinary committed May 1, 2024
1 parent 5e71a57 commit 00fd8f7
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 16 deletions.
6 changes: 4 additions & 2 deletions DatadogCore/Tests/Datadog/Mocks/LogsMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ extension LogsFeature {
logEventMapper: LogEventMapper? = nil,
requestBuilder: FeatureRequestBuilder = RequestBuilder(),
messageReceiver: FeatureMessageReceiver = NOPFeatureMessageReceiver(),
dateProvider: DateProvider = SystemDateProvider()
dateProvider: DateProvider = SystemDateProvider(),
backtraceReporter: BacktraceReporting? = nil
) -> Self {
return .init(
logEventMapper: logEventMapper,
requestBuilder: requestBuilder,
messageReceiver: messageReceiver,
dateProvider: dateProvider
dateProvider: dateProvider,
backtraceReporter: backtraceReporter
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@ public struct BinaryImage: Codable, PassthroughAnyCodable {
case maxAddress = "max_address"
}
}

extension BinaryImage: Equatable {}
12 changes: 9 additions & 3 deletions DatadogLogs/Sources/Feature/LogsFeature.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ internal struct LogsFeature: DatadogRemoteFeature {

let logEventMapper: LogEventMapper?

let backtraceReporter: BacktraceReporting?

@ReadWriteLock
private var attributes: [String: Encodable] = [:]

Expand All @@ -26,7 +28,8 @@ internal struct LogsFeature: DatadogRemoteFeature {
logEventMapper: LogEventMapper?,
dateProvider: DateProvider,
customIntakeURL: URL? = nil,
telemetry: Telemetry = NOPTelemetry()
telemetry: Telemetry = NOPTelemetry(),
backtraceReporter: BacktraceReporting? = nil
) {
self.init(
logEventMapper: logEventMapper,
Expand All @@ -39,20 +42,23 @@ internal struct LogsFeature: DatadogRemoteFeature {
CrashLogReceiver(dateProvider: dateProvider, logEventMapper: logEventMapper),
WebViewLogReceiver()
),
dateProvider: dateProvider
dateProvider: dateProvider,
backtraceReporter: backtraceReporter
)
}

init(
logEventMapper: LogEventMapper?,
requestBuilder: FeatureRequestBuilder,
messageReceiver: FeatureMessageReceiver,
dateProvider: DateProvider
dateProvider: DateProvider,
backtraceReporter: BacktraceReporting?
) {
self.logEventMapper = logEventMapper
self.requestBuilder = requestBuilder
self.messageReceiver = messageReceiver
self.dateProvider = dateProvider
self.backtraceReporter = backtraceReporter
}

internal func addAttribute(forKey key: AttributeKey, value: AttributeValue) {
Expand Down
3 changes: 2 additions & 1 deletion DatadogLogs/Sources/Log/LogEventBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ internal struct LogEventBuilder {
message: $0.message,
stack: $0.stack,
sourceType: $0.sourceType,
fingerprint: errorFingerprint
fingerprint: errorFingerprint,
binaryImages: binaryImages
)
},
serviceName: service,
Expand Down
2 changes: 1 addition & 1 deletion DatadogLogs/Sources/Log/LogEventEncoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ internal struct LogEventEncoder {
try container.encode(someError.sourceType, forKey: .errorSourceType)
try container.encode(someError.fingerprint, forKey: .errorFingerprint)
if let binaryImages = someError.binaryImages {
try container.encode(someError.binaryImages, forKey: .errorBinaryImages)
try container.encode(binaryImages, forKey: .errorBinaryImages)
}
}

Expand Down
20 changes: 15 additions & 5 deletions DatadogLogs/Sources/RemoteLogger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ internal final class RemoteLogger: LoggerProtocol {
return
}

let globalAttributes = self.core?.get(feature: LogsFeature.self)?.getAttributes()
let logsFeature = self.core?.get(feature: LogsFeature.self)

let globalAttributes = logsFeature?.getAttributes()

// on user thread:
let date = dateProvider.now
Expand All @@ -111,6 +113,7 @@ internal final class RemoteLogger: LoggerProtocol {
var logAttributes = attributes
let isCrash = logAttributes?.removeValue(forKey: CrossPlatformAttributes.errorLogIsCrash) as? Bool ?? false
let errorFingerprint = logAttributes?.removeValue(forKey: Logs.Attributes.errorFingerprint) as? String
let addBinaryImages = logAttributes?.removeValue(forKey: CrossPlatformAttributes.includeBinaryImages) as? Bool ?? false
let userAttributes = self.attributes
.merging(logAttributes ?? [:]) { $1 } // prefer message attributes
let combinedAttributes: [String: any Encodable]
Expand Down Expand Up @@ -153,9 +156,9 @@ internal final class RemoteLogger: LoggerProtocol {

// When binary images are requested, add them
var binaryImages: [BinaryImage]?
if let addBinaryImages = logAttributes?.removeValue(forKey: CrossPlatformAttributes.includeBinaryImages) {
// Don't try to get binary images if we already have them.
binaryImages = try? self.core?.backtraceReporter.generateBacktrace()?.binaryImages
if addBinaryImages {
// TODO: RUM-4072 Replace full backtrace reporter with simpler binary image fetcher
binaryImages = try? logsFeature?.backtraceReporter?.generateBacktrace()?.binaryImages
}

let builder = LogEventBuilder(
Expand Down Expand Up @@ -186,14 +189,21 @@ internal final class RemoteLogger: LoggerProtocol {
return
}

// Bit of a hack - add the `includeBinaryImages` attribute back in if it was in the original log
// TODO: RUM-4072 This double generates a backtrace - this should be optimized to use cached binary images instead
var rumAttributes: [String: Encodable] = combinedAttributes
if addBinaryImages {
rumAttributes[CrossPlatformAttributes.includeBinaryImages] = true
}

self.core?.send(
message: .baggage(
key: ErrorMessage.key,
value: ErrorMessage(
message: log.error?.message ?? log.message,
type: log.error?.kind,
stack: log.error?.stack,
attributes: .init(combinedAttributes)
attributes: .init(rumAttributes)
)
)
)
Expand Down
6 changes: 4 additions & 2 deletions DatadogLogs/Tests/Mocks/LoggingFeatureMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ extension LogsFeature {
sampler: Sampler = .mockKeepAll(),
requestBuilder: FeatureRequestBuilder = RequestBuilder(),
messageReceiver: FeatureMessageReceiver = NOPFeatureMessageReceiver(),
dateProvider: DateProvider = SystemDateProvider()
dateProvider: DateProvider = SystemDateProvider(),
backtraceReporter: BacktraceReporting = BacktraceReporterMock(backtrace: nil)
) -> Self {
return .init(
logEventMapper: logEventMapper,
requestBuilder: requestBuilder,
messageReceiver: messageReceiver,
dateProvider: dateProvider
dateProvider: dateProvider,
backtraceReporter: backtraceReporter
)
}
}
Expand Down
32 changes: 32 additions & 0 deletions DatadogLogs/Tests/RemoteLoggerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,38 @@ class RemoteLoggerTests: XCTestCase {
XCTAssertEqual(log.error?.fingerprint, randomErrorFingerprint)
}

func testWhenAttributesContainIncludeBinaryImages_itAddsBinaryImagesToLogEvent() throws {
let stubBacktrace: BacktraceReport = .mockRandom()
let logsFeature = LogsFeature.mockWith(
backtraceReporter: BacktraceReporterMock(backtrace: stubBacktrace)
)
let core = SingleFeatureCoreMock(
feature: logsFeature,
expectation: expectation(description: "Send log")
)
let logger = RemoteLogger(
core: core,
configuration: .mockAny(),
dateProvider: RelativeDateProvider(),
rumContextIntegration: false,
activeSpanIntegration: false
)

// When
logger.error("Information message", error: ErrorMock(), attributes: [CrossPlatformAttributes.includeBinaryImages: true])

// Then
waitForExpectations(timeout: 0.5, handler: nil)

let logs = core.events(ofType: LogEvent.self)
XCTAssertEqual(logs.count, 1)

let log = try XCTUnwrap(logs.first)
XCTAssertNil(log.attributes.userAttributes[CrossPlatformAttributes.includeBinaryImages])
XCTAssertNotNil(log.error?.binaryImages)
XCTAssertEqual(log.error?.binaryImages, stubBacktrace.binaryImages)
}

// MARK: - RUM Integration

func testWhenRUMIntegrationIsEnabled_itSendsLogWithRUMContext() throws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ internal struct RUMScopeDependencies {
firstPartyHosts: FirstPartyHosts?,
eventBuilder: RUMEventBuilder,
rumUUIDGenerator: RUMUUIDGenerator,
backtraceReporter: BacktraceReporting?,
ciTest: RUMCITest?,
syntheticsTest: RUMSyntheticsTest?,
vitalsReaders: VitalsReaders?,
Expand All @@ -72,6 +73,7 @@ internal struct RUMScopeDependencies {
self.firstPartyHosts = firstPartyHosts
self.eventBuilder = eventBuilder
self.rumUUIDGenerator = rumUUIDGenerator
self.backtraceReporter = backtraceReporter
self.ciTest = ciTest
self.syntheticsTest = syntheticsTest
self.vitalsReaders = vitalsReaders
Expand Down
4 changes: 2 additions & 2 deletions DatadogRUM/Sources/RUMMonitor/Scopes/RUMViewScope.swift
Original file line number Diff line number Diff line change
Expand Up @@ -554,10 +554,10 @@ internal class RUMViewScope: RUMScope, RUMContextProvider {
let errorFingerprint = commandAttributes.removeValue(forKey: RUM.Attributes.errorFingerprint) as? String

var binaryImages = command.binaryImages?.compactMap { $0.toRUMDataFormat }
if let addBinaryImages = commandAttributes.removeValue(forKey: CrossPlatformAttributes.includeBinaryImages) {
if commandAttributes.removeValue(forKey: CrossPlatformAttributes.includeBinaryImages) != nil {
// Don't try to get binary images if we already have them.
if binaryImages == nil {
// TODO: RUM-000 Replace full backtrace reporter with simpler binary image fetcher
// TODO: RUM-4072 Replace full backtrace reporter with simpler binary image fetcher
binaryImages = try? dependencies.backtraceReporter?.generateBacktrace()?.binaryImages.compactMap { $0.toRUMDataFormat }
}
}
Expand Down
68 changes: 68 additions & 0 deletions DatadogRUM/Tests/RUMMonitor/Scopes/RUMViewScopeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1557,6 +1557,74 @@ class RUMViewScopeTests: XCTestCase {
XCTAssertNil(error.context!.contextInfo[RUM.Attributes.errorFingerprint])
}

func testGivenStartedView_whenErrorWithIncludeBinaryImagesAttributesIsAdded_itAddsBinaryImagesToError() throws {
// Given
let mockBacktrace: BacktraceReport = .mockRandom()
let hasReplay: Bool = .mockRandom()
var context = self.context
context.baggages = try .mockSessionReplayAttributes(hasReplay: hasReplay)
var currentTime: Date = .mockDecember15th2019At10AMUTC()

let scope = RUMViewScope(
isInitialView: .mockRandom(),
parent: parent,
dependencies: .mockWith(
backtraceReporter: BacktraceReporterMock(backtrace: mockBacktrace)
),
identity: .mockViewIdentifier(),
path: "UIViewController",
name: "ViewName",
attributes: [:],
customTimings: [:],
startTime: currentTime,
serverTimeOffset: .zero
)

XCTAssertTrue(
scope.process(
command: RUMStartViewCommand.mockWith(time: currentTime, attributes: [:], identity: .mockViewIdentifier()),
context: context,
writer: writer
)
)

currentTime.addTimeInterval(1)

// When
XCTAssertTrue(
scope.process(
command: RUMAddCurrentViewErrorCommand.mockWithErrorMessage(
time: currentTime,
message: "view error",
source: .source,
stack: nil,
attributes: [
CrossPlatformAttributes.includeBinaryImages: true
]
),
context: context,
writer: writer
)
)

// Then
let error = try XCTUnwrap(writer.events(ofType: RUMErrorEvent.self).last)
XCTAssertNil(error.context?.contextInfo[CrossPlatformAttributes.includeBinaryImages])
XCTAssertNotNil(error.error.binaryImages)
XCTAssertEqual(error.error.binaryImages?.count, mockBacktrace.binaryImages.count)
for i in 0..<mockBacktrace.binaryImages.count {
let expected = mockBacktrace.binaryImages[i]
if let actual = error.error.binaryImages?[i] {
XCTAssertEqual(actual.arch, expected.architecture)
XCTAssertEqual(actual.isSystem, expected.isSystemLibrary)
XCTAssertEqual(actual.loadAddress, expected.loadAddress)
XCTAssertEqual(actual.maxAddress, expected.maxAddress)
XCTAssertEqual(actual.name, expected.libraryName)
XCTAssertEqual(actual.uuid, expected.uuid)
}
}
}

func testWhenResourceIsFinishedWithError_itSendsViewUpdateEvent() throws {
let scope = RUMViewScope(
isInitialView: .mockRandom(),
Expand Down

0 comments on commit 00fd8f7

Please sign in to comment.