-
Notifications
You must be signed in to change notification settings - Fork 134
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
Allow manually tracked resources to detect first party hosts #837
Conversation
976d24d
to
afc8c1c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of adding url filter to RUMScopeDependencies
, with that I don't think we need the isFirstPartyRequest
property in RUMStartResourceCommand
anymore. We can simply use the filter while processing the command, WDYT?
Yeah that would simplify things. I'll give it a shot. |
@maxep Updated, let me know if this is what you meant. |
Yes, exactly! I left another /nit Also, could you rebase on |
First party hosts set by `trackUrlSession` only works for automatically instrumented URLSessions. This allows manual calls to startResourceLoading and stopResourceLoading to detect first party host calls as well.
Instead, use the firstPartyURLsFilter on the dependencies to set isFirstPartyResource as we're writing the resource.
62c429d
to
e3dad80
Compare
Checking first party resource a the last possible minute, avoiding passing it through multiple scopes.
@maxep Moved it up one more level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 👌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ 👌
Co-authored-by: Maxime Epain <maxime.epain@datadoghq.com>
What and why?
First party hosts set by
trackUrlSession
only works for automatically instrumented URLSessions. This allows manual calls to startResourceLoading and stopResourceLoading to detect first party host calls as well.How?
Not sure if this is the approach we want to take, but it made sense to me. I'm pulling the configuration onto the RUM feature configuration, then using that in the dependency configuration for the application scope to create the same Filter object that the URLSessionDelegate uses.
Review checklist
Custom CI job configuration (optional)