angular-appinsights icon indicating copy to clipboard operation
angular-appinsights copied to clipboard

Support ng-strict, call start after the config phase, allow for user dat...

Open Varada opened this issue 10 years ago • 10 comments

Few different changes - I wish I had created smaller pull requests, sorry!

  1. Support running in ng-strict mode
  2. Allow start to be called after the config phase. The scenario here is that the instrumentation key is coming from a server $get - and is thus not present during the config phase
  3. Allow modification of user id. The scenario again is the server identifies the user. But there are bugs in the appinsights codebase that continues to look at the old cookie. Nuke the cookie and force use the new user id.

Varada avatar Jan 17 '15 13:01 Varada

Let me review this and provide some feedback. Just a quick glance and I'm wondering if getCookie and setCookie belong here in their current context. I say this cause they both access the document object but I am in no way a angular expert. Thoughts?

johnhidey avatar Jan 17 '15 14:01 johnhidey

I considered using ngCookies. I decided against it since I expect it to be temporary as the appinsights team will be fixing this bug.

-----Original Message----- From: "John Hidey" [email protected] Sent: ‎1/‎17/‎2015 8:13 PM To: "johnhidey/angular-appinsights" [email protected] Cc: "Varada" [email protected] Subject: Re: [angular-appinsights] Support ng-strict, call start after theconfig phase, allow for user dat... (#6)

Let me review this and provide some feedback. Just a quick glance and I'm wondering if getCookie and setCookie belong here in their current context. I say this cause they both access the document object but I am in no way a angular expert. Thoughts? — Reply to this email directly or view it on GitHub.=

Varada avatar Jan 17 '15 15:01 Varada

Did you get a chance to think more about this?

Varada avatar Jan 27 '15 13:01 Varada

I haven't had a chance, work lately has been getting in the way. I'll try to make an effort this weekend to really look things over and get back to you. Sorry for the delay.

johnhidey avatar Jan 28 '15 11:01 johnhidey

@Varada so I have been looking this over this weekend. Two things are bothering me and they are directly related. Changing the user context within a SPA application. I can't figure why this would ever been done within a single page app. Can you give me an example of why this would be done? The second issue that was bothering me was the cookie which would go away if we didn't need to support changing user context within the app.

johnhidey avatar Feb 21 '15 01:02 johnhidey

Consider the scenario where the user is unauthenticated. There is a default userId generated for him by appInsights and is saved in the cookie. Further requests are effectively routed against this userId. Now he signs up - and the service effectively is able to associate a authorized userId for him. And the service would from now on want to track the user against this userId rather than the anonymous cookie based userId since it is a more effective representation of unique users across devices.

So, in the SPA context the service returns a userId as response to a login call. The controller picks up the userId and asks appInsights to use the new userId for future communication.

Does this make sense?

On Sat, Feb 21, 2015 at 6:30 AM, John Hidey [email protected] wrote:

@Varada https://github.com/Varada so I have been looking this over this weekend. Two things are bothering me and they are directly related. Changing the user context within a SPA application. I can't figure why this would ever been done within a single page app. Can you give me an example of why this would be done? The second issue that was bothering me was the cookie which would go away if we didn't need to support changing user context within the app.

— Reply to this email directly or view it on GitHub https://github.com/johnhidey/angular-appinsights/pull/6#issuecomment-75346613 .

Varada avatar Feb 21 '15 04:02 Varada

So I think maybe you're missing something here. App Insights tracks the behavior of the application, not the behavior of the application user. The user in app insights is just your account for gathering the data and associating it with your site. Does this make sense ?

johnhidey avatar Feb 21 '15 15:02 johnhidey

@Varada BTW, the restructuring of the code that you have done, I like. I think I'm pull down your pull request tweak it a bit and merge in sometime here soon after hearing back from you.

johnhidey avatar Feb 21 '15 15:02 johnhidey

Each appinsights payload is associated with the user. The unique user for example is measured by the user id aggregation. Similarly, let us say if there is an exception, the payload will have the user id. Now, on the server side, we can correlate to understand which user is having the bad experience and reason with it.

On Sat, Feb 21, 2015 at 8:59 PM, John Hidey [email protected] wrote:

So I think maybe you're missing something here. App Insights tracks the behavior of the application, not the behavior of the application user. The user in app insights is just your account for gathering the data and associating it with your site. Does this make sense ?

— Reply to this email directly or view it on GitHub https://github.com/johnhidey/angular-appinsights/pull/6#issuecomment-75376533 .

Varada avatar Feb 21 '15 16:02 Varada

Any ETA on when this will be merged?

gligoran avatar Jan 26 '16 09:01 gligoran