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.

Sign up for GitHub

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

Jump to bottom

RUMM-2735 Add methods to pass error info to logs as decomposed strings #1051

Merged
merged 4 commits into from
Nov 16, 2022
Merged

RUMM-2735 Add methods to pass error info to logs as decomposed strings #1051

merged 4 commits into from
Nov 16, 2022

Conversation

fuzzybinary
Copy link
Member

What and why?

Adding methods to loggers so we can pass in error information (kind, message, and stack trace) as Strings instead of relying on the Error type from Swift.

This is mostly for Cross Platform SDKs that need to send in serialized error information.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • Run unit tests
  • Run integration tests
  • Run smoke tests

@fuzzybinary fuzzybinary requested a review from a team as a code owner November 14, 2022 19:10
@fuzzybinary fuzzybinary force-pushed the jward/RUMM-2735-string-error-reporting branch from 3519e91 to 3873751 Compare November 14, 2022 19:12
plousada
plousada reviewed Nov 15, 2022
View reviewed changes
Sources/Datadog/Logger.swift Outdated Show resolved Hide resolved
Sources/Datadog/Logger.swift Outdated
/// - stackTrace: a string representation of the error's stack trace
/// - attributes: a dictionary of attributes (optional) to add for this message. If an attribute with
/// the same key already exist in this logger, it will be overridden (only for this message).
func log(level: LogLevel, message: String, errorKind: String?, errorMessage: String?,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for us to not create an object conforming to Error on Flutter's side as use the existing API?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔 I hadn't thought about that (it wasn't possible in Android)

The reasons I can think not to are based on the strings that are created here:

dd-sdk-ios/Sources/Datadog/Core/Utils/DDError.swift

Line 30 in 4fee0f2

init(error: Error) {

If I create an NSError, I'll "{Domain} - {Code}" as the type, whereas with anything conforming to Error I'll just get the Swift type, which will always be the same (maybe not an issue as Obfuscation breaks getting the error type anyway).

I think this way gives us a lot more precision and allows Flutter / RN to specify the full object.

@fuzzybinary fuzzybinary force-pushed the jward/RUMM-2735-string-error-reporting branch from 8ca1bbe to 1a27200 Compare November 15, 2022 13:48
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Nov 15, 2022 •
edited
Loading

Datadog Report

Branch report: jward/RUMM-2735-string-error-reporting
Commit report: 396b512

❄️ dd-sdk-ios 0 Failed, 2 New Flaky, 1984 Passed, 0 Skipped, 5m 3.67s Wall Time

New Flaky Tests (2)

  • testWhenCPUUnderHeavyLoadItMeasuresHigherCPUTicks - VitalCPUReaderTest - Last Failure

    Expand for error 
     Assertion Failure at VitalCPUReaderTests.swift:24: XCTAssertGreaterThan failed: ("268.5927104043294") is not greater than ("284.9532585521021")

  • testItDoesSamplePeriodicallyWithLowFrequency - VitalInfoSamplerTests - Last Failure

    Expand for error 
     
 Assertion Failure at VitalInfoSamplerTests.swift:65: XCTAssertEqual failed: ("0") is not equal to ("1")
 Assertion Failure at VitalInfoSamplerTests.swift:66: XCTAssertEqual failed: ("0") is not equal to ("1")

@plousada plousada self-requested a review November 16, 2022 08:48
fuzzybinary and others added 3 commits November 16, 2022 09:59
@fuzzybinary
Add methods to pass error info to logs as decomposed strings
9933785 
This is mostly for Cross Platform SDKs that need to send in serialized error information.
@fuzzybinary @plousada
Update Sources/Datadog/Logger.swift
140f437 
Co-authored-by: Pedro Lousada <pedro.lousada@datadoghq.com>
@fuzzybinary
Swiftlint fixes
f9738c2
@fuzzybinary fuzzybinary force-pushed the jward/RUMM-2735-string-error-reporting branch from 1a27200 to f9738c2 Compare November 16, 2022 15:00
@fuzzybinary fuzzybinary merged commit 04bd5ac into develop Nov 16, 2022
@maciejburda maciejburda mentioned this pull request Dec 20, 2022
Merged
@maciejburda maciejburda changed the title Add methods to pass error info to logs as decomposed strings RUMM-2735 Add methods to pass error info to logs as decomposed strings Dec 20, 2022
@fuzzybinary fuzzybinary deleted the jward/RUMM-2735-string-error-reporting branch March 13, 2023 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@plousada plousada plousada approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fuzzybinary @plousada