Skip to content
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

Add library version into headers of integrations #852

Open
mariusandra opened this issue May 26, 2020 · 10 comments
Open

Add library version into headers of integrations #852

mariusandra opened this issue May 26, 2020 · 10 comments
Labels
enhancement New feature or request feature/ingestion libraries Related to client libraries (except posthog-js)

Comments

@mariusandra
Copy link
Collaborator

mariusandra commented May 26, 2020

Is your feature request related to a problem? Please describe.
There's an error in sentry that's sent to the /e endpoint, which doesn't contain any request data. I'd like to know what integration causes it, but details about that should be in the data that's missing.

Describe the solution you'd like
I'd like the $lib and $lib_version properties to be in the headers as well. Doing so can greatly help pinpoint issues.

Describe alternatives you've considered
Guessing based on other data or patterns, flipping a coin.

@mariusandra mariusandra added the enhancement New feature or request label May 26, 2020
@paolodamico
Copy link
Contributor

@mariusandra do we still want to do this?

@mariusandra
Copy link
Collaborator Author

@paolodamico work is ongoing but somehow stalled for the JS integration: PostHog/posthog-js#351

We should do this for others as well, once we decide on a safe header to put this information in.

@paolodamico paolodamico added libraries Related to client libraries (except posthog-js) and removed team-platform labels Feb 23, 2022
@pauldambra
Copy link
Member

In that PR I've proposed posthog-user-agent holding ${lib}/${version} e.g. web/1.23.45

I don't think we need to split them because if the body can be processed we will still receive lib and version from it

@pauldambra
Copy link
Member

The solution we ended up with in #351 was to add a ver query parameter to the URL the library uses.

But that's pretty passive and we should tag the version in the sentry scope

@neilkakkar
Copy link
Contributor

For feature flag definitions, all server-side libraries send the user-agent as a header: https://github.com/PostHog/posthog-js-lite/blob/master/posthog-node/src/feature-flags.ts#L305

I think we can re-use this for capture endpoints as well?

I quite like the idea of sentry tags as well, for a bit deeper analytics, but this needs to happen only in one place: the servers where we capture exceptions, so no new library bloat there.

@pauldambra
Copy link
Member

I should trust myself more :) If compression fails we grab the ver header and send that as the tag. So we should always be tagging Sentry exceptions with either library.version from event properties or the ver param if compression fails

@pauldambra
Copy link
Member

So, the task here becomes

  • do all SDKs send a custom user agent
  • do they use compression
  • do they include $lib and $lib_version in body

Knowing that and with #11301 we can assert whether this is complete and whether they will be tagged in Sentry

👀

@pauldambra
Copy link
Member

pauldambra commented Aug 15, 2022

very brief pass over the libraries... -> not all of them send library version

sends ver param uses custom user agent uses compression tags version to sentry with #11301 if decompression fails
posthog-js Y N Y Y
posthog react native N? N https://github.com/PostHog/posthog-js-lite/blob/d5088cf194425451cd34a1da5537ac33ea6fff8d/posthog-react-native/src/posthog-rn.ts#L76 N doesn't need it
posthog-js-lite N N N doesn't need it
posthog-go N Y posthog-go (version: "+version+") N doesn't need it
posthog-php N Y "User-Agent: posthog-php/" . PostHog::VERSION, Y Y
posthog-node N Y posthog-node/${version}
posthog-web N N https://github.com/PostHog/posthog-js-lite/blob/d5088cf194425451cd34a1da5537ac33ea6fff8d/posthog-web/src/posthog-web.ts#L56
posthog-ruby N Y "posthog-ruby/#{PostHog::VERSION}" (feature flags overrides with slightly different format https://github.com/PostHog/posthog-ruby/blob/4abf672d4b1e7123e5f2ea3506577a3d26a3a207/lib/posthog/feature_flags.rb#L359) N doesn't need it
posthog-python N Y "posthog-python/"+VERSION Y Y
posthog-ios N Y (v3) Y Y
posthog-android N Y (v3) Y needs custom user agent
posthog-rs (rust) N N N looks unfinished?
posthog-flutter N uses posthog-android, ios and web
posthog-java N N N doesn't need it

@marandaneto
Copy link
Member

@pauldambra do you remember why we don't need for react-native? its definitely as native as android and ios, so it should I guess.

@pauldambra
Copy link
Member

blast from the past 🤣

the "doesn't need" column I think is about the double compression that the web SDK does. So I bet I meant that the body is available without the decompression step you have to add for the web SDK.

But I completely don't remember. Totally fine to over-rule me if it doesn't make sense 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature/ingestion libraries Related to client libraries (except posthog-js)
Projects
None yet
Development

No branches or pull requests

5 participants