Skip to content

👀 [Frontend] 282 adding initial instrumentation setup#293

Merged
cartersocha merged 16 commits into
open-telemetry:mainfrom
kubeshop:frontend-instrumentation
Aug 17, 2022
Merged

👀 [Frontend] 282 adding initial instrumentation setup#293
cartersocha merged 16 commits into
open-telemetry:mainfrom
kubeshop:frontend-instrumentation

Conversation

@xoscar

@xoscar xoscar commented Aug 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread .env Outdated
xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread docker-compose.yml Outdated
xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread src/frontend/Dockerfile Outdated
xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread src/frontend/next.config.js
xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread src/frontend/package.json
xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread src/frontend/pages/api/cart.ts
xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread src/frontend/utils/telemetry/BackendTracer.ts
xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread src/frontend/utils/telemetry/FrontendTracer.ts
xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread src/frontend/utils/telemetry/InstrumentationMiddleware.ts
xoscar
xoscar commented Aug 11, 2022
View reviewed changes
Comment thread src/otelcollector/otelcol-config.yml
@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
@xoscar xoscar mentioned this pull request Aug 11, 2022
Closed
puckpuck
puckpuck requested changes Aug 11, 2022
View reviewed changes
Comment thread src/frontend/utils/telemetry/Instrumentation.js Outdated
Comment thread src/frontend/utils/telemetry/Instrumentation.js Outdated
@xoscar

xoscar commented Aug 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

@puckpuck

puckpuck commented Aug 15, 2022

Copy link
Copy Markdown
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

xoscar commented Aug 15, 2022

Copy link
Copy Markdown
Contributor Author

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

puckpuck commented Aug 16, 2022

Copy link
Copy Markdown
Contributor

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

@xoscar

xoscar commented Aug 16, 2022

Copy link
Copy Markdown
Contributor Author

@puckpuck done 😃

mic-max
mic-max approved these changes Aug 16, 2022
View reviewed changes
Comment thread docker-compose.yml Outdated
Comment thread src/frontend/Dockerfile Outdated
Comment thread src/frontend/package.json
@puckpuck

puckpuck commented Aug 17, 2022

Copy link
Copy Markdown
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

xoscar commented Aug 17, 2022

Copy link
Copy Markdown
Contributor Author

@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 😄

puckpuck
puckpuck approved these changes Aug 17, 2022
View reviewed changes

@puckpuck puckpuck left a comment

Copy link
Copy Markdown
Contributor

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

cartersocha commented Aug 17, 2022

Copy link
Copy Markdown
Contributor

@xoscar update this branch & we'll merge next

@xoscar

xoscar commented Aug 17, 2022

Copy link
Copy Markdown
Contributor Author

@cartersocha Done 😉

@cartersocha

cartersocha commented Aug 17, 2022

Copy link
Copy Markdown
Contributor

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

Also please add a change log entry 😀

@xoscar

xoscar commented Aug 17, 2022

Copy link
Copy Markdown
Contributor Author

@cartersocha Yes and Done

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

cartersocha commented Aug 17, 2022

Copy link
Copy Markdown
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
@xoscar
👀 [Frontend] 282 adding initial instrumentation setup (open-telemetry…
5b45030 
…#293)
mohamed3637 added a commit to mohamed3637/opentelemetry-demo that referenced this pull request Oct 7, 2025
@mohamed3637
👀 [Frontend] 282 adding initial instrumentation setup (open-telemetry…
085d50f 
…#293)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@puckpuck puckpuck puckpuck approved these changes
+1 more reviewer
@mic-max mic-max mic-max approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Frontend] Implement Frontend instrumentation

4 participants

@xoscar @puckpuck @cartersocha @mic-max