-
Notifications
You must be signed in to change notification settings - Fork 644
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
Log product information on push+verify events #4860
Conversation
} | ||
|
||
return result; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we not return the user agent value as it is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ad hoc parsing may be better here as this heuristic may lose valuable information. It'd be easy to recreate this value in Application Insight's Analytics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not new code.. it was copied from ClientInformationTelemetryEnricher, and moved here to allow sharing.
User-Agent header contains not only the client name and version (which is the info we actually need), but also platform information, which we don't care about. The reason to split and not log the entire value is to reduce size, since this data is logged for every request. The approach has proved itself over the course of several months, and you can see this data in AI.
@@ -31,6 +31,7 @@ public class TelemetryService : ITelemetryService | |||
public const string AccountCreationDate = "AccountCreationDate"; | |||
public const string ClientVersion = "ClientVersion"; | |||
public const string ProtocolVersion = "ProtocolVersion"; | |||
public const string ProductInformation = "ProductInformation"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this key be "ClientInfo" like before? "ProductInformation" seems confusing when taken outside the context of a user-agent string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
🔔 |
Extracts client information from user agent and logs on push/verify id events.
https://github.com/NuGet/Engineering/issues/797