Skip to content

Conversation

@Varada
Copy link

@Varada Varada commented Jan 17, 2015

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.

@johnhidey
Copy link
Owner

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?

@Varada
Copy link
Author

Varada commented Jan 17, 2015

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
Copy link
Author

Varada commented Jan 27, 2015

Did you get a chance to think more about this?

@johnhidey
Copy link
Owner

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
Copy link
Owner

@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.

@Varada
Copy link
Author

Varada commented Feb 21, 2015

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
#6 (comment)
.

@johnhidey
Copy link
Owner

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
Copy link
Owner

@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.

@Varada
Copy link
Author

Varada commented Feb 21, 2015

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
#6 (comment)
.

@gligoran
Copy link

Any ETA on when this will be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants