segment-boneyard / integration-amplitude

Amplitude server-side integration
segment.com
MIT License
4 stars 7 forks source link

Correctly pick platform #9

Closed f2prateek closed 8 years ago

f2prateek commented 8 years ago

Uses the library name currently https://github.com/segmentio/integration-amplitude/blob/master/lib/mapper.js#L91.

Should use context.os or something equivalent.

theoephraim commented 8 years ago

In our use case, we have clients (android, ios, and web) as well as the an API sending data to amplitude via segment.

When requests hit our API we know where they came from (which device, app version, etc) and I would like to pass that platform value through to amplitude.

If you were to just use context.os, things could get weird trying to distinguise an ios app via mobile web requests coming from ios devices.

djih commented 8 years ago

@f2prateek would it be possible to use something like context.device.type? We have a couple of customers who are seeing values like 'analytics-ruby' for platform, but are expecting Android, iOS, Web.

f2prateek commented 8 years ago

yup context.device.type will work - although it's not populated by a.js afaik. https://segment.com/docs/spec/common/

djih commented 8 years ago

@f2prateek we could fall back on something like 'Web' if the value doesn't exist for device type, something like this:

platform: facade.proxy('context.device.type') || 'Web',

What do you think? If that looks good I'll submit a PR for it

f2prateek commented 8 years ago

I wouldn't assume that it's from the web if it's not provided. None of our server side libraries send it by default and all those would get attribtued to web.

djih commented 8 years ago

@f2prateek is there a way for the server-side libraries to know what the platform is? To give some context, say we have a customer that uses the Javascript integration. They have a user that visits their site on an Android mobile browser. For events logged, the platform should be Web. device type, device family, those would describe the Android device. OS would show like the browser.

f2prateek commented 8 years ago

You'd want to use a combination of context.userAgent, context.device.type, context.os.name.

The flow would be something like :

  1. Use context.device.type if provided.
  2. Use context.os.name if provided.
  3. Try to parse from userAgent
  4. Optional: map known values of context.library.name (i.e. analytics.js -> web, analytics-android -> android, etc.)
f2prateek commented 8 years ago

You'll have to map context.os.name to the appropriate platform as well.

djih commented 8 years ago

@f2prateek thanks for pushing this live. We're still seeing events with platform analytics-node and such. Does that mean those events just don't have the device info? Or is there another library I need to update? Thanks!

f2prateek commented 8 years ago

Yup that would mean those events don't have the device info.