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.

Sign up for GitHub

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

Jump to bottom

✨ Implement a developer extension #686

Merged
merged 16 commits into from
Feb 17, 2021
Merged

✨ Implement a developer extension #686

merged 16 commits into from
Feb 17, 2021

Conversation

BenoitZugmeyer
Copy link
Member

@BenoitZugmeyer BenoitZugmeyer commented Jan 20, 2021 •
edited
Loading

Motivation

When working on the Browser SDK project or its integration in websites, it may be tough to see what is actually sent to the server or use local bundles on production websites.

Changes

Implement a browser extension to address developer needs. A few features are implemented, but more could be added in the future.

Testing

Future features

  • configuration page to set a list of custom intakes or CDN URLs
  • log events as soon as they are generated by the SDK (needs SDK change)
  • inject the SDK on any page
  • log Replay records
  • devtools tab integration + activate the extension only on tabs where devtools is open

I have gone over the contributing documentation.

@BenoitZugmeyer BenoitZugmeyer requested review from a team as code owners January 20, 2021 11:37
BenoitZugmeyer
BenoitZugmeyer commented Jan 20, 2021
View reviewed changes
tsconfig.json Outdated
@@ -1,5 +1,6 @@
// This tsconfig is only used for tooling (ex: typecheck in code editors)
{
"extends": "./tsconfig.base.json",
"include": ["./packages/**/src/**/*.ts", "./packages/**/test/**/*.ts"]
"include": ["./packages/**/src", "./packages/**/test"],
"exclude": ["./packages/developer-extension"]
Copy link
Member Author

Choose a reason for hiding this comment

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

The developer extension tsconfig is not entirely compatible with the base tsconfig, notably it doesn't support strictNullChecks, so it needs to be kept appart from the root tsconfig.

apigirl
apigirl reviewed Jan 20, 2021
View reviewed changes
Copy link
Contributor

@apigirl apigirl left a comment

Choose a reason for hiding this comment

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

no docs team review needed

webNeat
webNeat suggested changes Jan 26, 2021
View reviewed changes
Copy link
Contributor

@webNeat webNeat left a comment

Choose a reason for hiding this comment

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

Looking good, just some remarks.

packages/developer-extension/src/background/domain/flushEvents.ts Outdated Show resolved Hide resolved
packages/developer-extension/package.json Outdated Show resolved Hide resolved
packages/developer-extension/src/background/domain/replaceBundles.ts Outdated
Comment on lines 28 to 30
'https://www.datadoghq-browser-agent.com/datadog-logs.js',
'https://www.datadoghq-browser-agent.com/datadog-rum.js',
'https://www.datadoghq-browser-agent.com/datadog-rum-recorder.js',
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the SDK is configured to use a different intake domain? Is it possible generate these URLs based on the actual RUM and logs configurations?

Copy link
Member Author

Choose a reason for hiding this comment

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

This list is not intake domains, but I agree it would be great to be able to configure bundle URLs and intake lists. I'll add a list of feature ideas somewhere.

I'm not sure how to discover such URLs based on page the RUM and Logs configurations though.

packages/developer-extension/src/popup/useStore.ts Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Jan 29, 2021 •
edited
Loading

Codecov Report

Merging #686 (9b2c818) into master (4304434) will increase coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #686      +/-   ##
==========================================
+ Coverage   80.51%   80.65%   +0.13%     
==========================================
  Files          71       71              
  Lines        3732     3732              
  Branches      884      884              
==========================================
+ Hits         3005     3010       +5     
+ Misses        727      722       -5
Impacted Files Coverage Δ
...ckages/core/src/domain/automaticErrorCollection.ts 100.00% <0.00%> (+1.58%) ⬆️
packages/rum-core/src/transport/batch.ts 77.14% <0.00%> (+11.42%) ⬆️

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 4304434...9b2c818. Read the comment docs.

bcaudan
bcaudan approved these changes Feb 17, 2021
View reviewed changes
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.

Great!

@BenoitZugmeyer BenoitZugmeyer merged commit 0b9ff57 into master Feb 17, 2021
@BenoitZugmeyer BenoitZugmeyer deleted the benoit/dev-extension branch February 17, 2021 09:46
@dependabot dependabot bot mentioned this pull request Mar 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apigirl apigirl apigirl left review comments

@webNeat webNeat webNeat approved these changes

@bcaudan bcaudan bcaudan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@BenoitZugmeyer @codecov-io @bcaudan @webNeat @apigirl