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

✨ [Browser SDK][RUM-291] Allow logs when cookies are disabled #255

Merged
merged 14 commits into from
Feb 11, 2020

Conversation

mquentin
Copy link
Member

@mquentin mquentin commented Feb 6, 2020

What it does
Capture d’écran 2020-02-06 à 14 23 45
When the cookies are disabled, only the RUM events are blocked. All collected log events will be sent without a session id.

Context
https://datadoghq.atlassian.net/browse/RUMF-291
https://trello.com/c/ttMmQ7vw/70-browser-logs-datadog-browser-agent-doesnt-work-in-an-environment-where-cookies-are-disabled

Client have a native iOS app that opens up a web app in UIWebView that blocks cookies. As a result the DD_LOGS.logger.log does not send anything as neither the session id, neither the 'Tracked or not' mode can be read from the cookies.

Changes

  • init process isValidBrowsingContext has now two modes for each package that allow the cookies=OFF for logs and block it for rum
  • The mode "Tracked/ not Tracked" is read from the cookies (current behavior) or computed on the fly from the configurations if it is not available (added behavior)
  • If the cookies are off, the session id will only be a random id reloaded in every pages. When this case happens, the logs will not pass a session_id to the backend

… a different RUM and LOGS valid browsing context / do not pass a sessionId when the cookies are off
@bits-bot
Copy link

bits-bot commented Feb 6, 2020

CLA assistant check
All committers have signed the CLA.

@codecov-io
Copy link

codecov-io commented Feb 6, 2020

Codecov Report

Merging #255 into master will increase coverage by 0.39%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #255      +/-   ##
==========================================
+ Coverage   85.46%   85.86%   +0.39%     
==========================================
  Files          24       24              
  Lines        1321     1323       +2     
  Branches      276      276              
==========================================
+ Hits         1129     1136       +7     
+ Misses        192      187       -5
Impacted Files Coverage Δ
packages/rum/src/rum.ts 81.92% <0%> (ø) ⬆️
packages/rum/src/rumSession.ts 100% <100%> (ø) ⬆️
packages/rum/src/viewTracker.ts 90.54% <70%> (-3.99%) ⬇️
packages/rum/src/rum.entry.ts 71.42% <0%> (+2.04%) ⬆️
packages/core/src/requestCollection.ts 96.15% <0%> (+13.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a1395e...327df9e. Read the comment docs.

packages/core/src/init.ts Outdated Show resolved Hide resolved
packages/logs/src/logger.ts Outdated Show resolved Hide resolved
@mquentin mquentin marked this pull request as ready for review February 10, 2020 15:16
@mquentin mquentin requested a review from a team as a code owner February 10, 2020 15:16
@mquentin mquentin requested a review from bcaudan February 10, 2020 15:16
Copy link
Contributor

@bcaudan bcaudan left a comment

Choose a reason for hiding this comment

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

This PR is actually not a bug fix but a new feature, can you rename your PR to something like:
✨ Allow logs when cookies are disabled

packages/logs/src/loggerSession.ts Outdated Show resolved Hide resolved
packages/logs/src/loggerSession.ts Show resolved Hide resolved
…of github.com:DataDog/browser-sdk into maxime.quentin/RUM-291-fix-logs-with-disabled-cookies
@mquentin mquentin changed the title 🐛[Browser SDK][RUM-291] when cookies are disabled, fix the issue that prevent Logs to be collected ✨ [Browser SDK][RUM-291] Allow logs when cookies are disabled Feb 11, 2020
@mquentin mquentin merged commit 6c3f55f into master Feb 11, 2020
@mquentin mquentin deleted the maxime.quentin/RUM-291-fix-logs-with-disabled-cookies branch February 11, 2020 12:17
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