-
Notifications
You must be signed in to change notification settings - Fork 237
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
[Master] Add local storage-based implementation #1419 #2037
Conversation
d790830
to
cefddab
Compare
I've pushed a full rewrite of this PR @MSNev. Once you're happy with the prod code, I'll add some tests proving the configuration options all behave nicely 🙂 I can appreciate major releases are many moving parts, so am happy to keep tweaking this until it fits nicely into your release plans! |
952a894
to
d92af76
Compare
…osoft#1419 This can be used like: ``` const appInsights = new ApplicationInsights({ config: { enableSessionStorageBuffer: true, bufferOverride: { getItem: (logger, key) => localStorage.getItem(key), setItem: (logger, key, value) => localStorage.setItem(key, value), } } }); ```
d92af76
to
d2b9c67
Compare
I've added tests exercising the new config paths now. |
_self._buffer = (_self._senderConfig.enableSessionStorageBuffer() && utlCanUseSessionStorage()) | ||
? new SessionStorageSendBuffer(diagLog, _self._senderConfig) : new ArraySendBuffer(diagLog, _self._senderConfig); | ||
const useSessionStorage = _self._senderConfig.enableSessionStorageBuffer() && | ||
(_self._senderConfig.bufferOverride() || utlCanUseSessionStorage()) |
Check notice
Code scanning / CodeQL
Semicolon insertion
#2037) This can be used like: ``` const appInsights = new ApplicationInsights({ config: { enableSessionStorageBuffer: true, bufferOverride: { getItem: (logger, key) => localStorage.getItem(key), setItem: (logger, key, value) => localStorage.setItem(key, value), } } }); ``` Co-authored-by: Nev <54870357+MSNev@users.noreply.github.com>
* [BUG] The documentation for enableDebug is incorrect, it should reference enableDebugExceptions #2014 (#2022) * [BUG] SDK LOAD Failure reporting not working #2027 (#2038) * Add a simple interface to enable custom buffer storage solutions #1419 (#2037) This can be used like: ``` const appInsights = new ApplicationInsights({ config: { enableSessionStorageBuffer: true, bufferOverride: { getItem: (logger, key) => localStorage.getItem(key), setItem: (logger, key, value) => localStorage.setItem(key, value), } } }); ``` Co-authored-by: Nev <54870357+MSNev@users.noreply.github.com> * [Master] Add readme documentation for IStorageBuffer (#2045) * [Release] Increase version to 2.8.12 (#2046) --------- Co-authored-by: Philip Peitsch <philip.peitsch@gmail.com>
I'm aiming for the smallest diff with this to allow local storage.
This can be used like:
UPDATED
I wasn't sure if I should expose this on config just yet (e.g.,
enableLocalStorageBuffer
). Ideally this eventually becomes a pluggable component, so that a hypotheticalIndexedDb
or completely custom storage mechanism can be inserted trivially. I thought about adding a simple factory method toIConfig
, but theISendBuffer
interface is not available to that package currently.Open to any feedback on this, including rejection if it's not taking this mechanism in a direction you're comfortable with.
Longer term, I'm starting to poke at how to nicely make the
ISendBuffer
async to allow more flexible storage mechanisms to be built, but enabling local storage seemed a very useful first step regardless.