amplitude / Amplitude-Node

Server-side Node.js SDK for Amplitude
MIT License
67 stars 20 forks source link

No insert_id and other critical properties? #75

Closed tommedema closed 3 years ago

tommedema commented 3 years ago

At https://developers.amplitude.com/docs/http-api-v2 it's clearly stated that you highly recommend always passing in a insert_id, yet the sdk does not support this property? It is not part of the typescript typings.

Similarly, lots of properties are missing, e.g. os_name, app_version, etc.

kelvin-lu commented 3 years ago

@tommedema,

Thanks for raising this issue. You're totally correct that those types are missing - feel free to open a pr to @amplitude/types or we can look into resolving this issue this week; we see this as a great first issue.

tommedema commented 3 years ago

Seems like there are a lot of properties missing, and this is more than just a typings issue? We'll be reverting to using the HTTP API again

kelvin-lu commented 3 years ago

@tommedema Hi! Could you explain more by what you mean by this being more than just a typings issue - have you run into issues using the Node SDK itself because of the event shape (adding insert id, os name, etc.), outside of type compilation issue?

tommedema commented 3 years ago

I haven't been able to try and add more properties since typescript won't compile, so didn't realize you allow for passing in arbitrary event properties. Still, we'd like to deploy today so cannot wait for the typings to be updated.

tommedema commented 3 years ago

We'd expect all properties at https://developers.amplitude.com/docs/http-api-v2 to be supported

kelvin-lu commented 3 years ago

Right, sorry about the lack of typings - why not cast the event sent into logEvent as any for the time being until this is resolved?

I think the call to use the HTTP API is also valid if the Node SDK is not providing any value here - we wanted to help remove the need to do your own HTTP + Batching + Retrying logic but those can be replicated

tommedema commented 3 years ago

Ok -- we will be reverting back to the SDK then. We've never had to use any in our codebase so hopefully this can be resolved soon.

yuhao900914 commented 3 years ago

@tommedema, Hi there, we want to give you an update about this issue. The issue has been resolved in our most resent node v1.5.0. It includes the types of insert_id and other properties. Again, thanks for raise the issue up.