-
Notifications
You must be signed in to change notification settings - Fork 7
[rum] Inject browser sdk #119
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
Conversation
47ec98f
to
73486c7
Compare
a305acd
to
fceab29
Compare
fceab29
to
3c35484
Compare
84950be
to
74d2575
Compare
e401db0
to
d66990f
Compare
d66990f
to
5f3ea86
Compare
5f3ea86
to
f158564
Compare
README.md
Outdated
datadogWebpackPlugin({ | ||
rum?: { | ||
disabled?: boolean, | ||
sdk?: { |
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.
Maybe we could use the RUM type directly here, no?
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.
That's a very good idea, I'll check if I can.
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 put that on the wrong file, oops, this is the README, not the source types
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.
yeah nice Idea!
packages/plugins/rum/README.md
Outdated
- [rum.sdk.applicationId](#rumsdkapplicationid) | ||
- [rum.sdk.clientToken](#rumsdkclienttoken) | ||
- [rum.sdk.site](#rumsdksite) |
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.
Similarly, maybe we don't need to document all of those. Instead we could reference the general doc in the public documentation website, WDYT?
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.
Makes sense, I'm afraid of getting out of sync, but I do prefer having a single source of truth.
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.
If you want only 1 source of truth, you'd need to add a dependency on @datadog/browser-rum
instead of checking the global DD_RUM
(right now you're only using a devDep)
DD_RUM.addAction('custom_click', { | ||
bundler: '{{bundler}}', | ||
}); |
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.
❤️
|
||
import { datadogRum } from '@datadog/browser-rum'; | ||
|
||
// To please TypeScript. |
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.
😂
17e30b5
to
73662a4
Compare
68f129f
to
378742a
Compare
378742a
to
d8d6d43
Compare
Co-authored-by: Benjamin Koltes <[email protected]>
d8d6d43
to
8784499
Compare
- run: yarn typecheck:all | ||
|
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.
This is done part of the yarn cli integrity
now.
What and why?
Automatically inject RUM's Browser SDK into the bundle.
How?
rum-browser-sdk.js
which is used for the injection.TODO