RUM-2558: Let AndroidTracer.logErrorMessage() send an ERROR log by aleksandr-gringauz · Pull Request #2605 · DataDog/dd-sdk-android

aleksandr-gringauz · 2025-04-16T14:56:29Z

What does this PR do?

Makes AndroidTracer.logErrorMessage create a log with ERROR severity that is attached to the span.

demo

I discussed with Xavier that we shouldn't make the span itself have an error state, just change the log severity.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

codecov-commenter · 2025-04-16T15:44:05Z

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.02%. Comparing base (72376a1) to head (3af91fa).
⚠️ Report is 2162 commits behind head on develop.

Files with missing lines	Patch %	Lines
.../trace/internal/handlers/AndroidSpanLogsHandler.kt	33.33%	0 Missing and 2 partials ⚠️
...in/com/datadog/android/log/internal/LogsFeature.kt	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2605      +/-   ##
===========================================
+ Coverage    69.98%   70.02%   +0.04%     
===========================================
  Files          819      819              
  Lines        30579    30559      -20     
  Branches      5121     5120       -1     
===========================================
- Hits         21399    21397       -2     
+ Misses        7740     7738       -2     
+ Partials      1440     1424      -16

Files with missing lines	Coverage Δ
.../kotlin/com/datadog/android/trace/AndroidTracer.kt	`95.74% <100.00%> (ø)`
...c/main/kotlin/com/datadog/android/trace/SpanExt.kt	`88.24% <ø> (ø)`
...in/com/datadog/android/log/internal/LogsFeature.kt	`85.34% <50.00%> (-1.61%)`	⬇️
.../trace/internal/handlers/AndroidSpanLogsHandler.kt	`84.31% <33.33%> (-3.44%)`	⬇️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

0xnm · 2025-04-17T07:25:57Z

        /**
         * Helper method to attach an error message to a specific Span.
-         * The error message will be added to the provided Span as a standard Error Tag.
+         * The error message will be logged with ERROR severity and can be seen in logs attached to the span.


in facets it is called status

Suggested change

* The error message will be logged with ERROR severity and can be seen in logs attached to the span.

* The error message will be logged with ERROR status and can be seen in logs attached to the span.

now I use "status" everywhere

0xnm · 2025-04-17T07:27:04Z

 /**
 * Helper method to attach an error message to this [Span].
- * The error message will be added to this [Span] as a standard Error Tag.
+ * The error message will be logged with ERROR severity and can be seen in logs attached to the span.


Suggested change

* The error message will be logged with ERROR severity and can be seen in logs attached to the span.

* The error message will be logged with ERROR status and can be seen in logs attached to the span.

now I use "status" everywhere

0xnm · 2025-04-17T07:28:23Z

        @JvmStatic
        fun logErrorMessage(span: Span, message: String) {
-            val fieldsMap = mapOf(Fields.MESSAGE to message)
+            val fieldsMap = mapOf(Fields.MESSAGE to message, "severity" to Log.ERROR)


we need to call this either level (user in codebase) or status (facet name) and also extract it as a constant.

now I use "status" everywhere

0xnm · 2025-04-17T07:31:12Z

        timestampMicroseconds: Long? = null
    ) {
        val logsFeature = sdkCore.getFeature(Feature.LOGS_FEATURE_NAME)
+        val spanLogSeverity = fields["severity"] as? Int


we need to remove this from the attributes, since it is a technical property which shouldn't end in the list of the user attributes

Suggested change

val spanLogSeverity = fields["severity"] as? Int

val spanLogSeverity = fields.remove("severity") as? Int

also we need to rename it from severity

thanks for spotting this

renamed to "status"

now I remove it from the fields

0xnm · 2025-04-17T07:32:49Z

            verify(mockSpan).log(capture())
            assertThat(firstValue)
                .containsEntry(Fields.MESSAGE, anErrorMessage)
-                .containsOnlyKeys(Fields.MESSAGE)


we need to put this assertion back as well. Basically we want to assert that there are only 2 keys.

Same for the similar tests below.

thanks for spotting it

fixed

aleksandr-gringauz force-pushed the aleksandr-gringauz/rum-2558/androidtracer-logerrormessage-error-log-2 branch from 4646b02 to 76b4133 Compare April 16, 2025 15:16

aleksandr-gringauz marked this pull request as ready for review April 16, 2025 15:21

aleksandr-gringauz requested review from a team as code owners April 16, 2025 15:21

0xnm reviewed Apr 17, 2025

View reviewed changes

aleksandr-gringauz force-pushed the aleksandr-gringauz/rum-2558/androidtracer-logerrormessage-error-log-2 branch from 76b4133 to 997a4c9 Compare April 17, 2025 08:13

RUM-2558: Let AndroidTracer.logErrorMessage() send an ERROR log

3af91fa

aleksandr-gringauz force-pushed the aleksandr-gringauz/rum-2558/androidtracer-logerrormessage-error-log-2 branch from 997a4c9 to 3af91fa Compare April 17, 2025 08:42

0xnm approved these changes Apr 17, 2025

View reviewed changes

aleksandr-gringauz merged commit 5506381 into develop Apr 17, 2025

aleksandr-gringauz deleted the aleksandr-gringauz/rum-2558/androidtracer-logerrormessage-error-log-2 branch April 17, 2025 11:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RUM-2558: Let AndroidTracer.logErrorMessage() send an ERROR log#2605

RUM-2558: Let AndroidTracer.logErrorMessage() send an ERROR log#2605
aleksandr-gringauz merged 1 commit into
developfrom
aleksandr-gringauz/rum-2558/androidtracer-logerrormessage-error-log-2

aleksandr-gringauz commented Apr 16, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented Apr 16, 2025 •

edited

Loading

Uh oh!

0xnm Apr 17, 2025

Uh oh!

aleksandr-gringauz Apr 17, 2025

Uh oh!

0xnm Apr 17, 2025

Uh oh!

aleksandr-gringauz Apr 17, 2025

Uh oh!

0xnm Apr 17, 2025

Uh oh!

aleksandr-gringauz Apr 17, 2025

Uh oh!

0xnm Apr 17, 2025

Uh oh!

aleksandr-gringauz Apr 17, 2025

Uh oh!

0xnm Apr 17, 2025

Uh oh!

aleksandr-gringauz Apr 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	* The error message will be logged with ERROR severity and can be seen in logs attached to the span.
	* The error message will be logged with ERROR status and can be seen in logs attached to the span.

	val spanLogSeverity = fields["severity"] as? Int
	val spanLogSeverity = fields.remove("severity") as? Int

Uh oh!

Conversation

aleksandr-gringauz commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

codecov-commenter commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aleksandr-gringauz commented Apr 16, 2025 •

edited

Loading

codecov-commenter commented Apr 16, 2025 •

edited

Loading