-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 session ID attribute to frontend spans #795
Add session ID attribute to frontend spans #795
Conversation
Please add a changelog item too when you get a chance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Should we note this only appears on non-synthetic requests in our docs? @open-telemetry/demo-approvers Also maybe it's just me but I find front-end, front-end-web, and front-end-proxy confusing. Might be worth re-naming one of the user facing services or the synthetic pair. I'd expect to see my user generated traces in the general front-end bucket but you have to go to another service that starts reporting post website interaction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @martinkuba 👋🏽 ,
Thanks for taking care of this 🤩
I've tried it out, but couldn't make it work.
Where is the app.session.id
supposed to be?
I've re-built the sample, started it, stopped the loadgen, and accessed the frontend manually.
When navigating through the generated spans, I could only find the id
as part of the http.url
and there was no app.session.id
attribute, as we can see in the screenshot below:
Am I missing something?
@julianocosta89 it only shows on user generated traces rn in the front-end-web service. See my comments above |
@cartersocha I stopped the loadgen and navigated manually. |
@julianocosta89 I have pulled down the codebase to a different directory, rebuilt from my branch, and I am seeing the @cartersocha Are you seeing this on your end? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @martinkuba!
Looks like I did something wrong. Re-built it again and now it worked.
LGTM!
Please add the changelog item and we're ready to merge! |
Changes
This adds back the
app.session.id
span attribute that was originally handled in the Go frontend server. The attribute is added for browser spans as well as some spans from the frontend server.Resolves #758