Skip to content

Fix stopResourceWithError overloads to use getEventTime(attributes)#3155

Merged
hamorillo merged 2 commits into
developfrom
hector.morilloprieto/RUM-14051
Jan 28, 2026
Merged

Fix stopResourceWithError overloads to use getEventTime(attributes)#3155
hamorillo merged 2 commits into
developfrom
hector.morilloprieto/RUM-14051

Conversation

@hamorillo

@hamorillo hamorillo commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR fixes what was mentioned in this comment #3154 (comment).

For all overloads of the method stopResourceWithError we need to extract the eventTime from the attributes when possible so we should call the method getEventTime(attributes) similar to what is done in other events.

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)

@hamorillo

hamorillo commented Jan 27, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jan 27, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@datadog-official

This comment has been minimized.

Sign in to view
@codecov-commenter

codecov-commenter commented Jan 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.97%. Comparing base (5910f8b) to head (2616b0d).
⚠️ Report is 462 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #3155      +/-   ##
===========================================
+ Coverage    70.89%   70.97%   +0.08%     
===========================================
  Files          912      912              
  Lines        33542    33548       +6     
  Branches      5640     5640              
===========================================
+ Hits         23779    23809      +30     
+ Misses        8183     8172      -11     
+ Partials      1580     1567      -13
Files with missing lines Coverage Δ
.../android/rum/internal/monitor/DatadogRumMonitor.kt 88.51% <100.00%> (-0.08%) ⬇️

... 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.

@hamorillo
hamorillo marked this pull request as ready for review January 27, 2026 16:18
@hamorillo
hamorillo requested a review from a team as a code owner January 27, 2026 16:18
0xnm
0xnm previously approved these changes Jan 27, 2026
View reviewed changes
@0xnm

0xnm commented Jan 27, 2026

Copy link
Copy Markdown
Member

minor note: we maybe can think about added assertions to the existing tests for the custom time

jonathanmos
jonathanmos previously approved these changes Jan 28, 2026
View reviewed changes
@hamorillo
hamorillo force-pushed the hector.morilloprieto/RUM-14051 branch from aa18605 to 2616b0d Compare January 28, 2026 09:35
@hamorillo

hamorillo commented Jan 28, 2026

Copy link
Copy Markdown
Contributor Author

minor note: we maybe can think about added assertions to the existing tests for the custom time

@0xnm Nice suggestion! I've added some asserts to existing tests. 🙂

@hamorillo

hamorillo commented Jan 28, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Jan 28, 2026 •
edited
Loading

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-01-28 09:53:53 UTC ℹ️ Start processing command /merge

2026-01-28 09:54:00 UTC ℹ️ MergeQueue: waiting for PR to be ready

This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
It will be added to the queue as soon as checks pass and/or get approvals. View in MergeQueue UI.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2026-01-28 09:54:54 UTC ⚠️ MergeQueue: This merge request was unqueued

[email protected] unqueued this merge request

@hamorillo

hamorillo commented Jan 28, 2026

Copy link
Copy Markdown
Contributor Author

/merge -c

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Jan 28, 2026 •
edited
Loading

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-01-28 09:54:48 UTC ℹ️ Start processing command /merge -c

@hamorillo
hamorillo merged commit 2e38b97 into develop Jan 28, 2026
26 of 27 checks passed
@hamorillo
hamorillo deleted the hector.morilloprieto/RUM-14051 branch January 28, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aleksandr-gringauz aleksandr-gringauz aleksandr-gringauz approved these changes
@0xnm 0xnm 0xnm left review comments
@jonathanmos jonathanmos jonathanmos left review comments

Assignees

No one assigned

Labels

mergequeue-status: removed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@hamorillo @codecov-commenter @0xnm @jonathanmos @aleksandr-gringauz