-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: allow session tracking across subdomains #38
Conversation
In order to gain the ability to track our sessions, funnels, conversions, journeys, etc. across all of our various applications and subdomains, we need to add a couple of settings to RUM and Logs SDK setup. From [Datadog Browser Log Collection](https://docs.datadoghq.com/real_user_monitoring/browser/setup/#initialization-parameters) and [Datadog Log Collection initialization parameters](https://docs.datadoghq.com/logs/log_collection/javascript/#initialization-parameters): * `trackSessionAcrossSubdomains`: Preserve the session across subdomains for the same site. * `usePartitionedCrossSiteSessionCookie`: Use a partitioned secure cross-site session cookie. This allows the logs SDK to run when the site is loaded from another one (iframe). Implies `useSecureSessionCookie`. Per DD support: > The `useCrossSiteSessionCookie` parameter specifies the type of cookie the SDK will set, it's not a reference to cross-domain tracking. You would want this to be set to `true` to track events when the page is rendered as an iframe. Note: `useCrossSiteSessionCookie` is marked in the documents as a deprecated perimeter which we should replace with`usePartitionedCrossSiteSessionCookie`.
reminding deployers to update the package version number.
README.md
Outdated
@@ -15,6 +15,10 @@ You can also add your own custom metrics as an additional argument, or see the c | |||
|
|||
General Logging service interface is defined in [frontend-platform](https://openedx.github.io/frontend-platform/module-Logging.LoggingService.html). | |||
|
|||
## Deployment | |||
|
|||
When you make changes, [draft a new release](https://github.com/edx/frontend-logging/releases) (and, if necessary, change the imported version number in importing packages). |
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.
[clarification] semantic-release should be creating GitHub releases automatically after merging a PR. For example, the existing releases are created by the edx-semantic-release
user.
[clarification x2] Is if necessary, change the imported version number in importing packages
referring to bumping versions of frontend-logging in edx-internal?
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.
- ohhh thanks, that's not happening in every repo we import as a package so I didn't realize
- Yeah, but if I'm taking out (1) I might as well take out the bumping version reference, don't you think?
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, I agree with (2); I think the version bumping is also largely implied as a consumer of this package (for the most part?). I feel if we keep a "Deployment" section, it might largely serve to document that this repository uses Semantic Release to automate the release process (creating Git tags, creating GitHub releases, and publishing to NPM).
src/DatadogLoggingService.js
Outdated
@@ -60,6 +60,8 @@ class DatadogLoggingService extends NewRelicLoggingService { | |||
sessionSampleRate: parseInt(process.env.DATADOG_SESSION_SAMPLE_RATE || 0, 10), | |||
sessionReplaySampleRate: parseInt(process.env.DATADOG_SESSION_REPLAY_SAMPLE_RATE || 0, 10), | |||
trackUserInteractions: true, | |||
trackSessionAcrossSubdomains: true, |
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.
Based on the documentation:
Options that must have matching configuration when you are using the Logs Browser SDK:
Options that must have a matching configuration when using the RUM SDK:
Given this, I wonder if it might make sense to abstract common options across datadogRum.init
and datadogLogs.init
so it doesn't necessitate manually supplying the same values in both; might help to guarantee they are always matching, for example. Something like:
const datadogVersion = process.env.DATADOG_VERSION || process.env.APP_VERSION || '1.0.0';
const commonInitOptions = {
clientToken: process.env.DATADOG_CLIENT_TOKEN,
site: process.env.DATADOG_SITE || '',
env: process.env.DATADOG_ENV || '',
service: process.env.DATADOG_SERVICE || '',
version: datadogVersion,
trackSessionAcrossSubdomains: true,
usePartitionedCrossSiteSessionCookie: true,
};
datadogRum.init({
...commonInitOptions,
beforeSend: this.beforeSend,
sessionSampleRate: parseInt(process.env.DATADOG_SESSION_SAMPLE_RATE || 0, 10),
sessionReplaySampleRate: parseInt(process.env.DATADOG_SESSION_REPLAY_SAMPLE_RATE || 0, 10),
trackUserInteractions: true,
trackResources: true,
trackLongTasks: true,
defaultPrivacyLevel: process.env.DATADOG_PRIVACY_LEVEL || 'mask-user-input',
});
datadogLogs.init({
...commonInitOptions,
forwardErrorsToLogs: true,
sessionSampleRate: parseInt(process.env.DATADOG_LOGS_SESSION_SAMPLE_RATE || 0, 10),
});
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 like this, implementing.
Per code review suggestions, * pulling common options out so we don't have repetition (that can get out of sync) * simplifying the documentation, since I hadn't realized we used semantic releases
In order to gain the ability to track our sessions, funnels, conversions, journeys, etc. across all of our various applications and subdomains, we need to add a couple of settings to RUM and Logs SDK setup.
From Datadog Browser Log Collection and Datadog Log Collection initialization parameters:
trackSessionAcrossSubdomains
: Preserve the session across subdomains for the same site.usePartitionedCrossSiteSessionCookie
: Use a partitioned secure cross-site session cookie. This allows the logs SDK to run when the site is loaded from another one (iframe). ImpliesuseSecureSessionCookie
.Per DD support:
Note:
useCrossSiteSessionCookie
is marked in the documents as a deprecated parameter which we should replace withusePartitionedCrossSiteSessionCookie
.