[Enhancement] Add a sender that uses fetch when XMLHttpRequest is not available #1556
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
Contributor
xiao-lix
commented
Apr 27, 2021
•
xiao-lix
commented
- Add a fetch sender - Address Investigate and add a sender that uses fetch when XMLHttpRequest is not available #1268 ITelemetryContext.user is sometimes null - setAuthenticatedUserContext throws #1541
- Add the ability for the user to provide extra headers - Address Cannot modify the request headers and cookies when using a custom endpoint #1545
xiao-lix
commented
Apr 27, 2021
xiao-lix
commented
Apr 27, 2021
| QUnit.assert.equal(0, this._getXhrRequests().length, "xhr sender is not called"); | ||
| QUnit.assert.ok(fetchstub.called, "fetch sender is called"); | ||
| // store it back | ||
| (window as any).XMLHttpRequest = fakeXMLHttpRequest; |
There was a problem hiding this comment.
Need to store the xhr request back to fake request - otherwise it impacts the following tests.
xiao-lix
commented
Apr 27, 2021
xiao-lix
commented
Apr 27, 2021
| @@ -229,11 +237,17 @@ export class Sender extends BaseTelemetryPlugin implements IChannelControlsAI { | |||
| _InternalMessageId.InvalidInstrumentationKey, "Invalid Instrumentation key "+config.instrumentationKey); | |||
| } | |||
|
|
|||
| if (!isInternalApplicationInsightsEndpoint(_self._senderConfig.endpointUrl()) && _self._senderConfig.customHeaders() && _self._senderConfig.customHeaders().length > 0) { | |||
There was a problem hiding this comment.
customHeaders are only added to request when endpointUrl is not Breeze.
MSNev
reviewed
Apr 28, 2021
MSNev
reviewed
Apr 28, 2021
MSNev
reviewed
Apr 28, 2021
kryalama
reviewed
Apr 29, 2021
…Insights-JS into lxiao/fetch-sender
xiao-lix
commented
Apr 29, 2021
xiao-lix
commented
Apr 29, 2021
README.md
Outdated
| @@ -310,6 +310,7 @@ Most configuration fields are named such that they can be defaulted to falsey. A | |||
| | enablePerfMgr | boolean | false | [Optional] When enabled (true) this will create local perfEvents for code that has been instrumented to emit perfEvents (via the doPerf() helper). This can be used to identify performance issues within the SDK based on your usage or optionally within your own instrumented code. [More details are available by the basic documentation](./docs/PerformanceMonitoring.md). Since v2.5.7 | |||
| | perfEvtsSendAll | boolean | false | [Optional] When _enablePerfMgr_ is enabled and the [IPerfManager](https://github.com/microsoft/ApplicationInsights-JS/blob/master/shared/AppInsightsCore/src/JavaScriptSDK.Interfaces/IPerfManager.ts) fires a [INotificationManager](https://github.com/microsoft/ApplicationInsights-JS/blob/master/shared/AppInsightsCore/src/JavaScriptSDK.Interfaces/INotificationManager.ts).perfEvent() this flag determines whether an event is fired (and sent to all listeners) for all events (true) or only for 'parent' events (false <default>).<br />A parent [IPerfEvent](https://github.com/microsoft/ApplicationInsights-JS/blob/master/shared/AppInsightsCore/src/JavaScriptSDK.Interfaces/IPerfEvent.ts) is an event where no other IPerfEvent is still running at the point of this event being created and it's _parent_ property is not null or undefined. Since v2.5.7 | |||
| | idLength | numeric | 22 | [Optional] Identifies the default length used to generate new random session and user id's. Defaults to 22, previous default value was 5 (v2.5.8 or less), if you need to keep the previous maximum length you should set this value to 5. | |||
| | customHeaders | `[{header: string, value: string}]` | undefined | [Optional] The ability for the user to provide extra headers when using a custom endpoint. | |||
There was a problem hiding this comment.
Add new configuration here.
MSNev
approved these changes
Apr 29, 2021
|
I've updated the issues to tag them with the next milestone "2.x.x (Next Release)" as this makes it a LOT easier to keep track of the items for the release notes. Also once this is checked in, please keep the issues Open (re-open if git-hub closes them) and add the tag "fixed - waiting for release" as again this is for tracking and it makes it obvious that the issue still exists in all released versions. |
MSNev
approved these changes
Apr 29, 2021
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.