RUM-13514: Put Request tag with UUID in DatadogInterceptor#3086
Conversation
|
@codex review |
154d21c to
0c8d772
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
0c8d772 to
37b6802
Compare
ac84fb0 to
cc54e1b
Compare
|
🎯 Code Coverage 🔗 Commit SHA: 3fd3f53 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3086 +/- ##
========================================
Coverage 71.40% 71.40%
========================================
Files 881 881
Lines 32397 32405 +8
Branches 5462 5463 +1
========================================
+ Hits 23131 23138 +7
+ Misses 7722 7716 -6
- Partials 1544 1551 +7
🚀 New features to boost your workflow:
|
| } | ||
| } | ||
|
|
||
| private fun Request.Builder.safeAddUUIDTag(uuid: UUID): Request.Builder { |
There was a problem hiding this comment.
s this needed? although it may throw ClassCastException here in theory, but we know that the class will match in our code.
There was a problem hiding this comment.
There was a problem hiding this comment.
Actually, why not. Done.
c66dddd
c66dddd to
3fd3f53
Compare
|
|
||
| @Suppress("DEPRECATION") | ||
| val requestId = request.buildResourceId(generateUuid = true) | ||
| val requestId = request.buildResourceId(generateUuid = false) |
There was a problem hiding this comment.
In theory we can still keep it as true, right? Since UUID tag is already attached to the request.
There was a problem hiding this comment.
yes, we can leave generateUuid as true. I changed to false because it looks a bit clearer. We put UUID into tags a few lines above and it would look weird if we had generateUuid = true here.
4 participants
What does this PR do?
See description of the bug in the ticket (RUM-13514).
This fix is about putting UUID into the Request tags, so that
buildResourceIdfunction can use it when it is called beforestopResource.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)