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.

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

RUMM-1151 Use the Local first-party host detector #515

Merged
merged 2 commits into from
Mar 4, 2021

Conversation

xgouchet
Copy link
Collaborator

@xgouchet xgouchet commented Mar 4, 2021

What does this PR do?

Ensure the local first party hosts provided in the OkHttp Interceptors are actually used.

Fixes #513

Additional Notes

We also removed the warnDeprecated calls that existed and weren't not removed in the un-deprecation of the related constructors

@xgouchet xgouchet requested a review from a team as a code owner March 4, 2021 09:27
@xgouchet xgouchet changed the base branch from master to release/1.8.0 March 4, 2021 09:27
@xgouchet xgouchet added the size-small This PR is small sized label Mar 4, 2021
@codecov-io
Copy link

codecov-io commented Mar 4, 2021

Codecov Report

Merging #515 (cf6fe44) into release/1.8.0 (315008f) will increase coverage by 0.26%.
The diff coverage is 75.00%.

@@                 Coverage Diff                 @@
##             release/1.8.0     #515      +/-   ##
===================================================
+ Coverage            89.43%   89.69%   +0.26%     
- Complexity            1372     1374       +2     
===================================================
  Files                  160      160              
  Lines                 4685     4676       -9     
  Branches               530      532       +2     
===================================================
+ Hits                  4190     4194       +4     
+ Misses                 321      307      -14     
- Partials               174      175       +1     
Impacted Files Coverage Δ Complexity Δ
...n/kotlin/com/datadog/android/DatadogInterceptor.kt 72.31% <ø> (+5.16%) 14.00 <0.00> (ø)
...ndroid/core/internal/net/FirstPartyHostDetector.kt 77.78% <0.00%> (-11.11%) 11.00 <1.00> (ø)
...n/com/datadog/android/core/internal/CoreFeature.kt 98.33% <100.00%> (ø) 42.00 <0.00> (ø)
.../com/datadog/android/tracing/TracingInterceptor.kt 75.96% <100.00%> (+3.74%) 33.00 <0.00> (+2.00)
...oid/src/main/kotlin/com/datadog/android/Datadog.kt 87.36% <0.00%> (-1.15%) 30.00% <0.00%> (-1.00%)
...c/main/kotlin/com/datadog/android/DatadogConfig.kt 94.03% <0.00%> (-0.50%) 10.00% <0.00%> (-1.00%)
...l/net/info/BroadcastReceiverNetworkInfoProvider.kt 82.11% <0.00%> (ø) 29.00% <0.00%> (ø%)
...tadog/android/log/internal/domain/LogSerializer.kt 93.06% <0.00%> (+1.39%) 29.00% <0.00%> (ø%)
...ernal/instrumentation/gestures/GesturesListener.kt 91.45% <0.00%> (+2.56%) 46.00% <0.00%> (+1.00%)
...droid/rum/tracking/ActivityViewTrackingStrategy.kt 91.11% <0.00%> (+4.44%) 14.00% <0.00%> (+1.00%)

Copy link
Collaborator

@mariusc83 mariusc83 left a comment

Choose a reason for hiding this comment

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

Do you want to update the Sample App too by adding the tracedHosts to RumInterceptor or you want to do this in a second PR ?

@@ -102,7 +102,7 @@ internal object CoreFeature {
initializeClockSync(appContext)
setupInfoProviders(appContext, consent)
setupOkHttpClient(configuration.needsClearTextHttp)
firstPartyHostDetector = FirstPartyHostDetector(configuration.firstPartyHosts)
firstPartyHostDetector.addKnownHosts(configuration.firstPartyHosts)
Copy link
Contributor

Choose a reason for hiding this comment

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

so I guess this is the main reason of the issue?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Mostly this one but sometimes our clients can have multiple OkHttp clients and they want to be able to configure separated traced hosts for them. Having this in mind we need a global traced hosts entry + the possibility to add custom traced hosts/RumInterceptor and for that we were using the localFirstPartyHostDetector which was not used anymore.

@xgouchet xgouchet force-pushed the xgouchet/RUMM-1151/fix_interceptors branch from f895eba to cf6fe44 Compare March 4, 2021 10:53
@xgouchet xgouchet merged commit c7a00be into release/1.8.0 Mar 4, 2021
@xgouchet xgouchet deleted the xgouchet/RUMM-1151/fix_interceptors branch March 4, 2021 11:08
@xgouchet xgouchet added this to the 1.8.1 milestone May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size-small This PR is small sized
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants