feat:referrer #327

ps863 · 2022-12-12T18:10:46Z

Currently, AWS CloudWatch RUM does not support referrer. Information about the referrer for page views is not presently available.

This change adds the ability for the RUM web client to record and dispatch referrer information. The referrer will be stored in the referrer field in event details of a page view event. The referrer's domain will be stored in the referrerDomain field in event details of a page view event.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

limhjgrace · 2022-12-13T16:20:37Z

src/sessions/PageManager.ts

+                document.referrer !== undefined && document.referrer !== ''
+                    ? document.referrer
+                    : null,


suggestion: We can extract this check&set to a separate method since we're reusing it in 3 places.

limhjgrace · 2022-12-13T16:21:04Z

src/sessions/PageManager.ts

+                document.referrer !== undefined && document.referrer !== ''
+                    ? document.referrer
+                    : null,


suggestion: Use method mentioned in comment above

limhjgrace · 2022-12-13T16:21:10Z

src/sessions/PageManager.ts

+                document.referrer !== undefined && document.referrer !== ''
+                    ? document.referrer
+                    : null,


suggestion: Use method mentioned in comment above

limhjgrace · 2022-12-13T16:59:34Z

src/sessions/PageManager.ts

+                '([A-Za-z]+://[^/]+)[/]?'
+            );
+
+            if (domainName) {
+                return domainName[1];
+            }


questions:

Is the purpose of the regex match to exclude trailing slashes (/)?

For domains, we typically exclude thehttps:// prefix. Should we also exclude the prefix here as well?

Do we not want to include localhost in the regex match?

Yes. However, will be using URL interface instead of regex for this. (See next revision)

Excluded in next revision

Next revision use of URL interface should handle localhost as well

limhjgrace · 2022-12-13T17:28:06Z

src/sessions/__tests__/PageManager.test.ts

@@ -487,3 +488,86 @@ describe('PageManager tests', () => {
        expect(pageManager.getPage().start).toEqual(2500);
    });
 });
+
+test('when complete referrer is available from the DOM', async () => {


suggestion: when complete referrer is available from the DOM --> when complete referrer is available from the DOM then is recorded in page view event

limhjgrace · 2022-12-13T17:28:38Z

src/sessions/__tests__/PageManager.test.ts

+    );
+});
+
+test('when only domain level referrer is available from the DOM', async () => {


suggestion: when only domain level referrer is available from the DOM --> when only domain level referrer is available from the DOM then is recorded in page view event

limhjgrace · 2022-12-13T17:30:29Z

src/sessions/__tests__/PageManager.test.ts

+    );
+});
+
+test('when referrer is not available from the DOM', async () => {


suggestion: when referrer is not available from the DOM --> when referrer from the DOM is not valid then it is not recorded in page view event

qhanam · 2022-12-14T13:59:05Z

src/sessions/PageManager.ts

+        if (document.referrer !== undefined && document.referrer !== '') {
+            const domainName = document.referrer.match(
+                '([A-Za-z]+://[^/]+)[/]?'
+            );
+
+            if (domainName) {
+                return domainName[1];
+            }
+        } else {
+            return null;
+        }


Suggested change

if (document.referrer !== undefined && document.referrer !== '') {

const domainName = document.referrer.match(

'([A-Za-z]+://[^/]+)[/]?'

);

if (domainName) {

return domainName[1];

}

} else {

return null;

}

try {

return new URL(document.referrer).hostname;

} catch (e) {

return "";

}

added in next revision

qhanam · 2022-12-14T14:01:16Z

src/sessions/PageManager.ts

+    /*
+    Get the referrer, if it can be read from the DOM
+    */
+    private getReferrer() {
+        if (document.referrer !== undefined && document.referrer !== '') {
+            return document.referrer;
+        } else {
+            return null;
+        }
+    }


issue: Do we need this helper function? document.referrer should always be a URI or an empty string.

fixed in next revision. The helper function is not needed

qhanam · 2022-12-14T14:25:57Z

src/sessions/PageManager.ts

+            if (page.referrer !== null) {
+                pageViewEvent.referrer = page.referrer;
+
+                if (page.referrerDomain !== null) {
+                    pageViewEvent.referrerDomain = page.referrerDomain;
+                }
+            }


Suggested change

if (page.referrer !== null) {

pageViewEvent.referrer = page.referrer;

if (page.referrerDomain !== null) {

pageViewEvent.referrerDomain = page.referrerDomain;

}

}

pageViewEvent.referrer = document.referrer;

pageViewEvent.referrerDomain = getDomainFromReferrer()

added in next revision

limhjgrace · 2023-01-11T21:05:42Z

src/sessions/__tests__/PageManager.test.ts

+    expect(pageManager.getAttributes()).toMatchObject({
+        'aws:referrer': '',
+        'aws:referrerDomain': ''
+    });


question: do we want to record in metadata if it's empty? is this a conscience decision we're making?

https://developer.mozilla.org/en-US/docs/Web/API/Document/referrer
States that -The value is an empty string if the user navigated to the page directly (not through a link, but, for example, by using a bookmark). , so we should record it

limhjgrace · 2023-01-11T21:07:41Z

src/sessions/__tests__/PageManager.test.ts

+        'popstate',
+        (pageManager as any).popstateListener
+    );
+});


question: do we need a unit test for when localhost is the referrer?

handled in next revision

adebayor123 · 2023-01-12T00:26:16Z

src/sessions/__integ__/PageManager.test.ts

@@ -1,6 +1,7 @@
 import { Selector } from 'testcafe';
 import { REQUEST_BODY } from '../../test-utils/integ-test-utils';
 import { PAGE_VIEW_EVENT_TYPE } from '../../plugins/utils/constant';
+import { create } from 'lodash';


nit: unused

removed in next revision

adebayor123 · 2023-01-12T00:30:53Z

src/sessions/__tests__/PageManager.test.ts

+    expect(pageManager.getAttributes()).toMatchObject({
+        'aws:referrer': '',
+        'aws:referrerDomain': ''
+    });


https://developer.mozilla.org/en-US/docs/Web/API/Document/referrer
States that -The value is an empty string if the user navigated to the page directly (not through a link, but, for example, by using a bookmark). , so we should record it

qhanam · 2023-01-24T05:25:19Z

src/sessions/PageManager.ts

+            if (document.referrer === 'localhost') {
+                // Handle special case for localhost
+                return 'localhost';
+            }
+            return '';


Suggested change

if (document.referrer === 'localhost') {

// Handle special case for localhost

return 'localhost';

}

return '';

return document.referrer === 'localhost' ? document.referrer : '';

qhanam · 2023-01-26T05:26:57Z

src/sessions/PageManager.ts

+    private getDomainFromReferrer() {
+        try {
+            return new URL(document.referrer).hostname;
+        } catch (e) {
+            return document.referrer === 'localhost' ? document.referrer : '';
+        }
+    }


nit: Consider covering these (localhost and empty referrer) with unit tests.

feat:Referrer

b93f349

ps863 requested review from qhanam, limhjgrace and adebayor123 December 12, 2022 18:11

ps863 changed the title ~~feat:Referrer~~ feat:referrer Dec 12, 2022

limhjgrace reviewed Dec 13, 2022

View reviewed changes

Revision 2

a6d93aa

qhanam reviewed Dec 14, 2022

View reviewed changes

ps863 added 4 commits December 14, 2022 11:45

Revision 3

ca4b499

Adding referrer to metadata instead of event details

2efa934

Updated referrer field names with new convention

fbc10ac

Updated referrer field names with new convention

5a1190d

limhjgrace reviewed Jan 11, 2023

View reviewed changes

adebayor123 approved these changes Jan 12, 2023

View reviewed changes

handle localhost referrer

4e283df

limhjgrace approved these changes Jan 13, 2023

View reviewed changes

ps863 added 2 commits January 19, 2023 11:53

Revert metadata changes

b251bde

Handle localhost

e05dbd5

qhanam reviewed Jan 24, 2023

View reviewed changes

ps863 added 2 commits January 25, 2023 01:39

Address comments

f2f5fc2

remove unneeded import

72bee17

qhanam approved these changes Jan 26, 2023

View reviewed changes

Adding localhost unit test for referrer

9a52c10

ps863 merged commit a414c92 into aws-observability:main Jan 26, 2023

ps863 mentioned this pull request Feb 8, 2023

chore(release): 1.13.0 #355

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat:referrer #327

feat:referrer #327

ps863 commented Dec 12, 2022 •

edited

limhjgrace Dec 13, 2022

limhjgrace Dec 13, 2022

limhjgrace Dec 13, 2022

limhjgrace Dec 13, 2022

ps863 Dec 14, 2022

limhjgrace Dec 13, 2022

limhjgrace Dec 13, 2022

limhjgrace Dec 13, 2022

qhanam Dec 14, 2022

ps863 Dec 14, 2022

qhanam Dec 14, 2022

ps863 Dec 14, 2022

qhanam Dec 14, 2022

ps863 Dec 14, 2022

limhjgrace Jan 11, 2023

adebayor123 Jan 12, 2023

limhjgrace Jan 11, 2023

ps863 Jan 12, 2023

adebayor123 Jan 12, 2023

ps863 Jan 12, 2023

adebayor123 Jan 12, 2023

qhanam Jan 24, 2023

qhanam Jan 26, 2023

feat:referrer #327

feat:referrer #327

Conversation

ps863 commented Dec 12, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ps863 commented Dec 12, 2022 •

edited