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

Add setUserCookie method on cookie manager #1628

Merged
merged 5 commits into from
Aug 11, 2021
Merged

Conversation

xiao-lix
Copy link
Contributor

@xiao-lix xiao-lix commented Aug 9, 2021

To address this issue: ai_user cookie not present after reenabling the cookie #1585

Reason:
ai_session cookie is present because after reenabling the cookie, renew on session is called when property plugin processes next telemetry. But ai_user cookie only "created" as part of the creating (loading) of the SDK. Thus, for SPA applications, there won't be "reloading" and therefore "creating" an instance. So ai_user cookie is not present.

Solution:
Similar to SessionManager, add a update method on User and the method is triggered in propertiesPlugin processTelemetry. Once customer re-enable the cookie usage, the previous created userId is used to generate cookie which is set by cookie manager.

Sample Usage:

const ai = new ApplicationInsights({
    config: {
        instrumentationKey: '<YOUR_IKEY>',
        disableCookiesUsage: true // disable cookie usage on initialize
    }
});
ai.loadAppInsights();
ai.getCookieMgr().setEnabled(true); // re-enable cookie usage for both session cookie and user cookie

Sample result:
Before re-enable cookies, no ai_session or ai_user cookie created.
After re-enable cookies and set user cookie, ai_session and ai_usertest cookies are created:
image

@xiao-lix xiao-lix requested a review from MSNev August 9, 2021 23:09
Copy link
Collaborator

@MSNev MSNev left a comment

Choose a reason for hiding this comment

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

This issue is not about adding another method to the cookie manager (which we should not do), as it already has all of the necessary set methods for cookies required.

The suggestion I had in the issue was to add this method to the User object (same as the setAuthenticatedUser()) as the application just wants to "re-enable" cookies and as such trigger that any configured cookies can now be re-created.

So this would apply to both ai_user and ai_session, so it might mean that we put this on the telemetry context rather than the user -- etc.

@MSNev MSNev added this to the 2.x.x (Next Release) milestone Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants