Skip to content

Comments

feat: distributed tracing - 3#25092

Merged
hariombalhara merged 17 commits intomainfrom
feat/dist-tracing-3
Dec 4, 2025
Merged

feat: distributed tracing - 3#25092
hariombalhara merged 17 commits intomainfrom
feat/dist-tracing-3

Conversation

@Udit-takkar
Copy link
Contributor

@Udit-takkar Udit-takkar commented Nov 12, 2025

What does this PR do?

This PR is on top of #24861

This PR displays the trace reference id on the frontend

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?


Summary by cubic

Adds end-to-end tracing for errors and shows a trace reference in the booking UI. Improves server error normalization across TRPC, Zod, Prisma, and Stripe, with tests.

  • New Features
    • Server: getServerErrorFromUnknown now unwraps TracedError, maps TRPCError to HTTP status, and attaches traceId and traced data to HttpError responses. Adds clearer handling for Zod, Prisma (incl. P2025 -> 404), and Stripe errors.
    • UI: BookEventForm shows a “trace reference id” when available. Recurring booking errors scroll into view. Types updated to carry traceId.
    • Tests: Added coverage for TracedError, TRPCError, ZodError, Prisma errors, and unknown fallbacks.

Written for commit c176be2. Summary will update automatically on new commits.

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Nov 12, 2025
@vercel
Copy link

vercel bot commented Nov 13, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
cal-companion Ready Ready Preview Comment Dec 4, 2025 5:23am
2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Dec 4, 2025 5:23am
cal-eu Ignored Ignored Dec 4, 2025 5:23am

Base automatically changed from feat/dist-tracing-2 to main November 13, 2025 11:51
@Udit-takkar Udit-takkar marked this pull request as ready for review November 13, 2025 12:00
@Udit-takkar Udit-takkar requested a review from a team as a code owner November 13, 2025 12:00
@graphite-app graphite-app bot requested a review from a team November 13, 2025 12:00
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 5 files

Prompt for AI agents (all 2 issues)

Understand the root cause of the following 2 issues and fix them.


<file name="packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx">

<violation number="1" location="packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx:328">
`t(&quot;trace_reference_id&quot;)` uses a translation key that does not exist in the locale files, so users will see the raw key text instead of a localized label. Please add the corresponding translation entry before referencing it.</violation>
</file>

<file name="packages/lib/server/getServerErrorFromUnknown.ts">

<violation number="1" location="packages/lib/server/getServerErrorFromUnknown.ts:129">
This branch now returns a plain Error instead of an HttpError, so callers lose statusCode/message fields and will fail when a string cause is passed.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

Copy link
Member

https://github.com/calcom/cal.com/blob/feat/dist-tracing-3/packages/lib/server/defaultResponder.ts#L64

We aren't passing tracedError and thus any uncaught error(e.g. I just did a throw new Error() in RegularBookingService) isn't sending the traceId back to client.

Also, if we make that change, we won't need to ensure to send traceId in HttpError data like this https://github.com/calcom/cal.com/blob/feat/dist-tracing-3/packages/features/bookings/lib/service/RegularBookingService.ts#L1852 as it would be automatically handled.

Copy link
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

Left a few small suggestions and a possible bug in functionality

@github-actions github-actions bot marked this pull request as draft November 18, 2025 05:22
@Udit-takkar Udit-takkar marked this pull request as ready for review November 25, 2025 09:26
@Udit-takkar Udit-takkar marked this pull request as draft November 25, 2025 14:56
@Udit-takkar Udit-takkar marked this pull request as ready for review November 25, 2025 20:06
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 6 files

@Udit-takkar
Copy link
Contributor Author

Udit-takkar commented Nov 27, 2025

https://github.com/calcom/cal.com/blob/feat/dist-tracing-3/packages/lib/server/defaultResponder.ts#L64

We aren't passing tracedError and thus any uncaught error(e.g. I just did a throw new Error() in RegularBookingService) isn't sending the traceId back to client.

Also, if we make that change, we won't need to ensure to send traceId in HttpError data like this https://github.com/calcom/cal.com/blob/feat/dist-tracing-3/packages/features/bookings/lib/service/RegularBookingService.ts#L1852 as it would be automatically handled

Done and removed traceId from HttpError

@hariombalhara
Copy link
Member

Can we also remove existing code from RegularBookingService that passes traceId manually. That would simplify the service a bit and also won't set a wrong precedent that could be copied elsewhere.

image

hariombalhara
hariombalhara previously approved these changes Dec 2, 2025
@hariombalhara hariombalhara enabled auto-merge (squash) December 2, 2025 09:42
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 7 files

@hariombalhara hariombalhara merged commit 843329c into main Dec 4, 2025
38 of 40 checks passed
@hariombalhara hariombalhara deleted the feat/dist-tracing-3 branch December 4, 2025 05:51
@Udit-takkar Udit-takkar mentioned this pull request Dec 8, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants