Allow configuring heap integration settings #734
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The heap integration previously did not allow configuring the CDN that serves the heap snippet or passing any heap options. This change introduces new options to allow configuring both.
mwudka
commented
Feb 14, 2023
| @@ -21,7 +21,9 @@ var toString = Object.prototype.toString; // in case this method has been overri | |||
| var Heap = (module.exports = integration('Heap') | |||
| .global('heap') | |||
| .option('appId', '') | |||
| .tag('<script src="//cdn.heapanalytics.com/js/heap-{{ appId }}.js">')); | |||
| .option('hostname', 'cdn.heapanalytics.com') | |||
There was a problem hiding this comment.
There seems to be some variation in how different integrations name this sort of parameter, so I wasn't sure of the best name. I'm happy to change to something else.
mwudka
commented
Feb 14, 2023
| @@ -21,7 +21,9 @@ var toString = Object.prototype.toString; // in case this method has been overri | |||
| var Heap = (module.exports = integration('Heap') | |||
| .global('heap') | |||
| .option('appId', '') | |||
| .tag('<script src="//cdn.heapanalytics.com/js/heap-{{ appId }}.js">')); | |||
| .option('hostname', 'cdn.heapanalytics.com') | |||
| .option('options', {}) | |||
There was a problem hiding this comment.
The heap JS docs list some of the options, but there are additional options which are not documented. That leads me to believe that it might be best to just provide a single options object, since the allowed options might be account-specific and it might be confusing to provide some configuration options here that aren't available to all accounts. However, I'm happy to split this out into specific configuration options.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
The current heap integration has two limitations which are proving challenging for our segment deployment:
secureCookieordisableTextCapture.This PR adds additional configuration options to the heap integration to support changing both.
Are there breaking changes in this PR?
No.
Testing
I used the (very slick and excellent) local runner to test manually. I didn't see an easy way to add any tests, since it's hard to mock the underlying heap object. I'm also not sure the tests would be hugely valuable, since they would essentially be asserting that the code exists as written rather than really testing behavior. I am open to suggestions about a better way to handle this!
Any background context you want to provide?
No
Is there parity with the server-side/android/iOS integration components (if applicable)?
N/A
Does this require a new integration setting? If so, please explain how the new setting works
This change introduces two new optional settings:
hostname(string): Allows changing the URL from which the heap snippet is loaded. It defaults to "cdn.heapanalytics.com", the current hard-coded value, to preserve backwards compatibility.options(object): Allows passing an arbitrary set of configuration options to heap. They are passed to the heap JS code as is. Defaults to{}to preserve backwards compatibility.Links to helpful docs and other external resources
Heap JS options