aws-observability / aws-rum-web

Amazon CloudWatch RUM Web Client
Apache License 2.0
114 stars 65 forks source link

fix: Allow title override in page attributes #508

Closed qhanam closed 4 months ago

qhanam commented 4 months ago

For every page view, the web client updates the title attribute with the value in window.document.title. This means that the title attribute still cannot be overridden (i.e., #430 did not actually solve #493).

This change allows the title attribute to be overridden when recordPageView is used.

Before

document.title = "sensitive";
pageManager.recordPageView({
            pageId: '/home',
            pageAttributes: {
                title: 'override'
            }
        });
// attributes: { pageId: '/home', title: 'sensitive' }

After

document.title = "sensitive";
pageManager.recordPageView({
            pageId: '/home',
            pageAttributes: {
                title: 'override'
            }
        });
// attributes: { pageId: '/home', title: 'override' }

Discussion

Options for fixing this include the following.

  1. Stop recording the page title.
  2. Record title at the start of the session only.
  3. Session attributes cannot be overwritten.
  4. Allow page attributes to be overwritten.

All of these options have problems.

I am recommending option (4), because it has the least risk with respect to backwards compatibility.


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

williazz commented 4 months ago

Question: can the override logic be implemented by swapping these two lines instead? https://github.com/aws-observability/aws-rum-web/pull/508/files#diff-93123c2ab6bd111166aa9a50cf26e70a2ee8db05649d6f23e8a4c819a6c3a56fR208-R209

qhanam commented 4 months ago

can the override logic be implemented by swapping these two lines instead?

The only other attributes that could be overridden are pageId and pageTags. These are passed as arguments to recordPageView along with the custom attributes, and so in the original spec we decided that the arguments would take precedence over the attributes.