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

👀 [Frontend] 282 adding initial instrumentation setup #293

Merged
merged 16 commits into from
Aug 17, 2022

Conversation

xoscar
Copy link
Contributor

@xoscar xoscar commented Aug 11, 2022

Fixes #283.

Changes

Adds the instrumentation code for the front-end application. Changes include:

  1. Adding auto instrumentation and propagation for the browser side.
  2. Adding manual and auto instrumentation for the server (node.js) side.

https://www.loom.com/share/81c1c009c7e3492087ceecf8a0c6f21d

Note

This is just the initial instrumentation wire up, for quality attributes, spans, etc we can have a discussion later on

.env Outdated Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
src/frontend/Dockerfile Outdated Show resolved Hide resolved
@xoscar xoscar marked this pull request as ready for review August 11, 2022 18:06
@xoscar xoscar requested a review from a team August 11, 2022 18:06
@xoscar xoscar changed the title 👀 282 adding initial instrumentation setup 👀 [Frontend] 282 adding initial instrumentation setup Aug 11, 2022
src/frontend/utils/telemetry/Instrumentation.js Outdated Show resolved Hide resolved
src/frontend/utils/telemetry/Instrumentation.js Outdated Show resolved Hide resolved
@xoscar
Copy link
Contributor Author

xoscar commented Aug 12, 2022

Hey @puckpuck thanks for the review! 😄 I just updated the PR with your comments. Let me know what you think 😄

@puckpuck
Copy link
Contributor

I'm running your branch and getting several errors in the frontend logs. I'm unable to use the UI to place an order as well.

Unhandled Promise rejection: Cannot find module for page: /product/9SIQT8TOJO ; Zone: <root> ; Task: Promise.then ; Value: PageNotFoundError: Cannot find module for page: /product/9SIQT8TOJO
    at getPagePath (/app/node_modules/next/dist/server/require.js:50:15)
    at Object.requirePage (/app/node_modules/next/dist/server/require.js:55:22)
    at /app/node_modules/next/dist/server/load-components.js:56:50
    at ZoneDelegate.invoke (/app/node_modules/zone.js/bundles/zone.umd.js:400:30)
    at Zone.run (/app/node_modules/zone.js/bundles/zone.umd.js:160:47)
    at /app/node_modules/zone.js/bundles/zone.umd.js:1318:38
    at ZoneDelegate.invokeTask (/app/node_modules/zone.js/bundles/zone.umd.js:434:35)
    at Zone.runTask (/app/node_modules/zone.js/bundles/zone.umd.js:205:51)
    at drainMicroTaskQueue (/app/node_modules/zone.js/bundles/zone.umd.js:620:39)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  code: 'ENOENT'
} Error: Cannot find module for page: /product/9SIQT8TOJO
    at getPagePath (/app/node_modules/next/dist/server/require.js:50:15)
    at Object.requirePage (/app/node_modules/next/dist/server/require.js:55:22)
    at /app/node_modules/next/dist/server/load-components.js:56:50
    at ZoneDelegate.invoke (/app/node_modules/zone.js/bundles/zone.umd.js:400:30)
    at Zone.run (/app/node_modules/zone.js/bundles/zone.umd.js:160:47)
    at /app/node_modules/zone.js/bundles/zone.umd.js:1318:38
    at ZoneDelegate.invokeTask (/app/node_modules/zone.js/bundles/zone.umd.js:434:35)
    at Zone.runTask (/app/node_modules/zone.js/bundles/zone.umd.js:205:51)
    at drainMicroTaskQueue (/app/node_modules/zone.js/bundles/zone.umd.js:620:39)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

@xoscar
Copy link
Contributor Author

xoscar commented Aug 15, 2022

Hey @puckpuck I just updated the PR with a fix for the error you were seeing, we still need to fix the currency service issue but I think that can be done in a separate PR as the problem exists in the main branch.

The problem was that we instantiated two zone managers and Next.js already has one.

@puckpuck
Copy link
Contributor

@xoscar, now that we fixed the other services, can you update this branch so we have a clean setup to work from?

@xoscar
Copy link
Contributor Author

xoscar commented Aug 16, 2022

@puckpuck done 😃

docker-compose.yml Outdated Show resolved Hide resolved
src/frontend/Dockerfile Outdated Show resolved Hide resolved
src/frontend/package.json Show resolved Hide resolved
@puckpuck
Copy link
Contributor

Root spans for traces initiated from the browser are missing.
Screen Shot 2022-08-15 at 11 38 52 PM

Using chrome dev tools, you can see 405 errors to send a payload of what seems to be an OTLP JSON payload.

Screen Shot 2022-08-15 at 11 54 10 PM
Screen Shot 2022-08-15 at 11 54 39 PM

@xoscar
Copy link
Contributor Author

xoscar commented Aug 17, 2022

@puckpuck the latest commit I submitted has some fixes for that problem, it should now have the correct root span starting from the client side 😄

Copy link
Contributor

@puckpuck puckpuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it end to end on my side. Works wonderfully!

@cartersocha
Copy link
Contributor

@xoscar update this branch & we'll merge next

@xoscar
Copy link
Contributor Author

xoscar commented Aug 17, 2022

@cartersocha Done 😉

@cartersocha
Copy link
Contributor

Did you implement Michael's suggestions? I'm about to resolve the conversations.

Also please add a change log entry 😀

@xoscar
Copy link
Contributor Author

xoscar commented Aug 17, 2022

@cartersocha Yes and Done

@cartersocha cartersocha merged commit 99dafb8 into open-telemetry:main Aug 17, 2022
@cartersocha
Copy link
Contributor

FYI @austinlparker you might want to pull latest for your demo

jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Frontend] Implement Frontend instrumentation
4 participants