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

✨ Allow logs collection on file:// URL #709

Merged
merged 2 commits into from
Feb 2, 2021

Conversation

kcaffrey
Copy link
Contributor

@kcaffrey kcaffrey commented Jan 29, 2021

Motivation

The motivation for this change is to allow logging from an Electron app, where it is standard to bundle the JS with webpack and serve the bundle from a file locally.

Changes

This change remove the local file check to allow logs SDK initialization when the page is served from file:// protocol.

Testing

There are no automated tests for this change, as I did not see any pre-existing tests for the local file initialization check, but I tested by building the bundle and pointing my app's dependency to that version rather than the live version of the browser-sdk.


I have gone over the contributing documentation.

The motivation for this change is to allow logging from an Electron app,
where it is standard to bundle the JS with webpack and serve the bundle
from a file locally.
@kcaffrey kcaffrey requested review from a team as code owners January 29, 2021 20:17
@bits-bot
Copy link

bits-bot commented Jan 29, 2021

CLA assistant check
All committers have signed the CLA.

packages/rum/README.md Outdated Show resolved Hide resolved
packages/logs/README.md Outdated Show resolved Hide resolved
packages/rum/README.md Outdated Show resolved Hide resolved
packages/rum/README.md Outdated Show resolved Hide resolved
@bcaudan bcaudan changed the title ✨ Add an option to allow initialization when served from a file:// URL ✨ Allow logs collection on file:// URL Feb 2, 2021
@bcaudan bcaudan merged commit 353f6dc into DataDog:master Feb 2, 2021
@bcaudan
Copy link
Contributor

bcaudan commented Feb 2, 2021

Thanks for your contribution @kcaffrey, it has been released with 2.5.0!

@johnpyp
Copy link

johnpyp commented Feb 3, 2021

I'm also using electron, and this change allows it to run, but I'm still getting a Cookies are not authorized, we will not send any data when serving from the file.

I tried to enable useCrossSiteSessionCookie to fix it, however according to the docs that implies requiring https which obviously isn't met here.

Any suggestions for this specific usecase if anyone else has run into would be appreciated, and I'll update this comment if I figure it out.

@bcaudan
Copy link
Contributor

bcaudan commented Feb 4, 2021

Hello @johnpyp,

Are you using only Logs SDK on 2.5.0 version?
If further help is needed, would you mind opening a new issue?

Thanks!

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.

6 participants