Skip to content

RUM-8314 allow disabling 404 span redaction#2496

Merged
xgouchet merged 1 commit into
developfrom
xgouchet/RUM-8314/unredact_404
Feb 3, 2025
Merged

RUM-8314 allow disabling 404 span redaction#2496
xgouchet merged 1 commit into
developfrom
xgouchet/RUM-8314/unredact_404

Conversation

@xgouchet

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds an option to keep the resource name for network requests ending in 404 as original, instead of having them redacted to "404".

@xgouchet
xgouchet requested review from a team as code owners January 30, 2025 10:34
0xnm
0xnm previously approved these changes Jan 30, 2025
Comment on lines +892 to +898
/**
* Set whether network requests returning a 404 status code should have their resource name redacted.
* In order to reduce the cardinality of resource names in APM, 404 urls are automatically redacted to
* "404"
* @param redacted if true, all 404 requests will have a resource name set to "404", else the resource name
* will be the URL
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/**
* Set whether network requests returning a 404 status code should have their resource name redacted.
* In order to reduce the cardinality of resource names in APM, 404 urls are automatically redacted to
* "404"
* @param redacted if true, all 404 requests will have a resource name set to "404", else the resource name
* will be the URL
*/
/**
* Set whether network requests returning a 404 status code should have their resource name redacted.
* In order to reduce the cardinality of resource names in APM, 404 URLs are automatically redacted to
* "404".
* @param redacted if true, all 404 requests will have a resource name set to "404", else the resource name
* will be the URL
*/

* @param redacted if true, all 404 requests will have a resource name set to "404", else the resource name
* will be the URL
*/
fun set404ResourcesRedacted(redacted: Boolean): R {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

side thought: This API may be confusing for DatadogInterceptor users, because this option applies only to APM actually, but they may think that they will see RUM Resource name as "404". But since it is a shared builder, we cannot remove this method from Builder for DatadogInterceptor.

@xgouchet
xgouchet force-pushed the xgouchet/RUM-8314/unredact_404 branch 2 times, most recently from 38da1db to f82e09e Compare January 30, 2025 19:14
@xgouchet
xgouchet requested a review from 0xnm January 30, 2025 19:24
@xgouchet
xgouchet force-pushed the xgouchet/RUM-8314/unredact_404 branch from f82e09e to 021b813 Compare January 31, 2025 11:15
@xgouchet
xgouchet force-pushed the xgouchet/RUM-8314/unredact_404 branch from 021b813 to 6a01d80 Compare January 31, 2025 11:33
@codecov-commenter

codecov-commenter commented Jan 31, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 22.85714% with 27 lines in your changes missing coverage. Please review.

Project coverage is 69.93%. Comparing base (6c54695) to head (6a01d80).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
...datadog/android/okhttp/trace/TracingInterceptor.kt 20.00% 24 Missing ⚠️
...n/com/datadog/android/okhttp/DatadogInterceptor.kt 40.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2496      +/-   ##
===========================================
- Coverage    69.95%   69.93%   -0.02%     
===========================================
  Files          788      788              
  Lines        29647    29660      +13     
  Branches      4962     4962              
===========================================
+ Hits         20739    20742       +3     
- Misses        7536     7538       +2     
- Partials      1372     1380       +8     
Files with missing lines Coverage Δ
...n/com/datadog/android/okhttp/DatadogInterceptor.kt 59.12% <40.00%> (-0.54%) ⬇️
...datadog/android/okhttp/trace/TracingInterceptor.kt 74.93% <20.00%> (-0.57%) ⬇️

... and 29 files with indirect coverage changes

@xgouchet
xgouchet merged commit 7387cc9 into develop Feb 3, 2025
@xgouchet
xgouchet deleted the xgouchet/RUM-8314/unredact_404 branch February 3, 2025 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants