-
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
IConfig.appUserId appears to be unused? #139
Comments
It looks like you can set "appUserId" in the snippet header but this field is not included in a telemetry. @MaxShehovtsov do you know why we have 'appUserId" field? Is it safe to remove it?
|
Per my understanding we had old API where userId and accountId could be set in the snippet. setAuthenticatedUserContext was added later. Account id still can be used and was preserved for backward compatibility but user id is not used now from anywhere. So it should be safe to remove. |
PR: #142 |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Not sure if I'm missing something subtle here, but while looking at some of the API docs and the code itself,
IConfig.appUserId
appears to be set in a few places, but never actually read. I had assumed, due to its proximity toIConfig.accountId
, that it was related toAppInsights.setAuthenticatedUserContext
, but it appears to not be the case.If this is indeed dead code, I'd be happy to submit a PR to clean it up. If it's not dead code, I'm prepared to be enlightened. Thanks either way! :)
The text was updated successfully, but these errors were encountered: