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

Set user agent span attribute on standalone spans #12689

Closed
mydea opened this issue Jun 28, 2024 · 7 comments · Fixed by #12896
Closed

Set user agent span attribute on standalone spans #12689

mydea opened this issue Jun 28, 2024 · 7 comments · Fixed by #12896

Comments

@mydea
Copy link
Member

mydea commented Jun 28, 2024

While investigating #11646, we found the root of the problem:

INP ingestion depends on the user agent (because it differs by browser). When using a tunnel the user-agent may often not be forwarded, leading to INP not being measured correctly.

We should fix this server-side by not ignoring INP spans without a user agent, but we should also add the user agent as an attribute to these spans so they can be used (if set) to improve handling.

Based on https://opentelemetry.io/docs/specs/semconv/attributes-registry/user-agent/, we should set user_agent.original to the full user agent string.

@cleptric
Copy link
Member

cleptric commented Jul 9, 2024

@jjbayer @edwardgou-sentry we tested if setting event.request.headers: {'user-agent': '...'} on the INP span would do the trick, but it looks like it does not. In any case, I would propose we move forward with user_agent.original inside span.data. Sounds good to you?

@edwardgou-sentry
Copy link
Contributor

edwardgou-sentry commented Jul 9, 2024

@jjbayer @edwardgou-sentry we tested if setting event.request.headers: {'user-agent': '...'} on the INP span would do the trick, but it looks like it does not.

Did you test on an AM3 plan org? There was another issue with standalone span ingestion on AM3 plans that we just found and fixed today: https://github.com/getsentry/getsentry/pull/14570

In any case, I would propose we move forward with user_agent.original inside span.data. Sounds good to you?

👍 sounds good to me!

@Lms24
Copy link
Member

Lms24 commented Jul 12, 2024

As discussed yesterday with @jjbayer I opened getsentry/relay#3815 to track the Relay changes.

@reyoucat
Copy link

👍

@jjbayer
Copy link
Member

jjbayer commented Jul 18, 2024

@Lms24 @mydea for events, we also try to extract the browser name from client hints, should we do the same for spans? Any idea if there's an OTel convention for client hints attributes?

@mydea
Copy link
Member Author

mydea commented Jul 18, 2024

@jjbayer there is https://opentelemetry.io/docs/specs/semconv/resource/browser/ which specifies some stuff that should be taken from the client hints, is that would you're thinking about?

@Lms24
Copy link
Member

Lms24 commented Jul 25, 2024

Update: I tested this change with a NextJS up using tunnelRoute and I can confirm that INP values show up in the Sentry UI. Thanks to everyone involved for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants