-
Notifications
You must be signed in to change notification settings - Fork 20
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
Platform is always overriden to Node.js #79
Comments
For some reason your built files are incorrect. When I go to node_modules/@amplitude/dist/src/nodeClient.js I see: Yet this code has been patched:
Yet the version I have installed is the latest version: |
Hey @tommedema ! We haven't published those changes - apologies. I'll go ahead and publish the newest version and close this once Node 1.3.3 is available! |
Ok, thanks @kelvin-lu -- and now that I have you, where am I supposed to set the user's browser and browser version? If os_name and os_version (which would be really odd naming though seems to be what your documentation at https://developers.amplitude.com/docs/http-api-v2 suggests), where am I supposed to define the operating system name and version? This is what I'm currently doing but it seems ugly and doesn't distinguish between browser name and version: platform: `${userAgent.browser.name}@${userAgent.browser.major}`,
os_name: userAgent.os.name,
os_version: userAgent.os.version,
device_brand: userAgent.device.vendor,
device_model: userAgent.device.model, And this is the data that I have available:
But it is unclear how to map these props to what Amplitude expects in the case of the browser name and version. |
@tommedema Let me confer with our dataPL team if we have any best practices but it looks close to what you have here - linking to what our Web SDK does here; we put the browser in the OS space and the OS info in the device. what is |
Hmm, what you referred to me is actually different from what we're doing. It seems a bit awkward to say that the device model is MacOS, Windows, etc.? MacOS is an operating system, not a device. A device might be an iPhone, Samsung Galaxy S8, etc. Even though it might not make sense to set the OS as the device, should we do this anyway since this seems to be what the web sdk does? FYI - seems like the web guys realize this is not correct userAgent.device.vendor will be undefined unless you are on a mobile device, in which case it will be e.g. "Samsung", "Nokia", etc. We use ua-parser-js to retrieve these properties from the user agent |
Expected Behavior
When setting platform it is never received as sent
Current Behavior
Platform is always overridden to "Node.js"
Possible Solution
Steps to Reproduce
Node.js
:Environment
The text was updated successfully, but these errors were encountered: