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.

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 override of domain metadata attribute #453

Merged

Conversation

qhanam
Copy link
Member

@qhanam qhanam commented Sep 22, 2023

In #430, we propose allowing all metadata attributes to be overridden. The primary motivation for this change is to work around the fact that CloudWatch RUM does not currently support associating multiple domains with an app monitor. Specifically, by allowing applications to overwrite the domain read from window.location, the application can spoof the domain and pass the domain check in CloudWatch RUM.

However, allowing all metadata attributes to be overridden could make it more difficult to provide backwards compatibility in the future, or cause users pain if they accidentally override a built-in metadata value.

For example, if the user agent is parsed by the service, instead of here, the service may choose overwrite the metadata values provided in the request. A counter argument is that allowing metadata attributes to be overridden makes it easier for applications to set values in cases where the built-in metadata values are incorrect.

Instead, this change allows only the domain attribute to be overridden.


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

@qhanam qhanam force-pushed the feature/initiator-resource-type branch from a70a2fc to 573781b Compare September 22, 2023 15:43
@williazz
Copy link
Contributor

williazz commented Sep 22, 2023

Question: so just so I fully understand, Cx can do something like this to use multiple apps to the same app monitor?

cwr('addSessionAttributes', {
    domain: 'override', // spoof
    trueDomain: window.location.origin //  for actual tracking
}) 

@qhanam
Copy link
Member Author

qhanam commented Nov 8, 2023

just so I fully understand, Cx can do something like this to use multiple apps to the same app monitor?

Correct. Alternatively, a RUM user could differentiate user flows across domains by adding the domain to the page Id.

@qhanam qhanam requested review from williazz and ps863 November 8, 2023 02:00
Copy link
Contributor

@williazz williazz left a comment

Choose a reason for hiding this comment

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

I'm good with this change if there's an immediate need, though I think we should think of a no-configuration solution in the long run.

@qhanam qhanam merged commit b658d45 into aws-observability:main Nov 8, 2023
3 checks passed
@qhanam qhanam deleted the feature/initiator-resource-type branch November 8, 2023 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants