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-1154 User can add attributes to auto instrumented RUM Resources #473

Conversation

ncreated
Copy link
Collaborator

@ncreated ncreated commented Apr 22, 2021

What and why?

📦 This PR adds rumResourceAttributesProvider API, which makes it possible to add extra attributes to RUM Resources collected automatically:

public func setRUMResourceAttributesProvider(
   _ provider: @escaping (URLRequest, URLResponse?, Data?, Error?) -> [AttributeKey: AttributeValue]?
)

The provider block is called for each URLSessionTask intercepted by the SDK. The implementation should look into task information and return custom attributes for RUM Resource, e.g. send redacted response headers or decode response body and send its additional context. This block is also called for RUM Errors created for resources failed on transport.

This is a counterpart of DataDog/dd-sdk-android#520 introduced in dd-sdk-android.

How?

The implementation starts with passing the provider block through configuration object to URLSessionRUMResourcesHandler as we do for other stuff. URLSessionInterceptor is extended with additional method: taskReceivedData(task:data:) which is called from two places:

  • from URLSesionSwizzler - for requests sent with completion block,
  • from DDURLSessionDelegate - for requests sent with no completion block.

About the public API, I considered few other choices:

  • adding the provider parameter to DDURLSessionDelegate.init() - this would require also exposing it in URLSessionInterceptor (for no-delegate usage including Alamofire extension),
  • adding the provider parameter to existing .trackURLSession(firstPartyHosts:) as both are mutually related - this would make the discovery of this new feature harder (the doc comment for .trackURLSession is already huge).

The proposed API should keep things simple: the feature can announced by its API name and the mutual relationship with trackURLSession is solved by installing misconfiguration warning.

All related unit tests pass on iOS 14.3, 13.4, 12.4 and 11.1.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

@ncreated ncreated self-assigned this Apr 22, 2021
@ncreated ncreated marked this pull request as ready for review April 22, 2021 13:13
@ncreated ncreated requested a review from a team as a code owner April 22, 2021 13:13
Copy link
Contributor

@xgouchet xgouchet left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ncreated ncreated merged commit cf630d6 into master Apr 26, 2021
@ncreated ncreated deleted the ncreated/RUMM-1154-user-can-add-attributes-to-auto-instrumented-rum-resources branch April 26, 2021 11:38
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.

None yet

3 participants