Skip to content

Comments

feat: distributed tracing - 2#24861

Merged
Udit-takkar merged 13 commits intomainfrom
feat/dist-tracing-2
Nov 13, 2025
Merged

feat: distributed tracing - 2#24861
Udit-takkar merged 13 commits intomainfrom
feat/dist-tracing-2

Conversation

@Udit-takkar
Copy link
Contributor

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

What does this PR do?

Ref:- #22969

Splitting PR feat: distributed tracing #24055

This is Part two after #24717

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 distributed tracing to API and tRPC requests. Propagates a trace context, adds structured logging, and returns an X-Trace-Id header to improve debugging and meet tracing rollout requirements.

  • New Features
    • Create and attach TraceContext per request in defaultResponder and tRPC context; propagate into booking service.
    • Add X-Trace-Id to all responses; include traceId in error payloads; structured logs for booking create flow (eventTypeId, userId, slug).
    • Centralize error handling with TracedError; capture non-4xx in Sentry; preserve correct status codes (409, 429).
    • Update tests to handle headers and validate status codes.

Written for commit cfc9bd4. 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 3, 2025
@pull-request-size pull-request-size bot added size/L and removed size/M labels Nov 3, 2025
@vercel
Copy link

vercel bot commented Nov 3, 2025

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

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Nov 13, 2025 11:18am
cal-eu Ignored Ignored Nov 13, 2025 11:18am

@Udit-takkar Udit-takkar added this to the v5.9 milestone Nov 3, 2025
@Udit-takkar Udit-takkar marked this pull request as ready for review November 3, 2025 14:15
@Udit-takkar Udit-takkar requested a review from a team as a code owner November 3, 2025 14:15
@graphite-app graphite-app bot requested a review from a team November 3, 2025 14:16
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 6 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/lib/service/RegularBookingService.ts">

<violation number="1" location="packages/features/bookings/lib/service/RegularBookingService.ts:1282">
Rule violated: **Avoid Logging Sensitive Information**

Remove the log of guestsRemoved; it outputs guest email addresses and violates the Avoid Logging Sensitive Information rule.</violation>
</file>

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

<violation number="1" location="packages/lib/server/defaultResponder.ts:54">
Wrapping every error in TracedError before calling getServerErrorFromUnknown strips the original error type (ErrorWithCode/HttpError), so status-specific responses (409, 404, etc.) now come back as 500. Please pass the original error into getServerErrorFromUnknown or adjust it to unwrap TracedError before classification.</violation>
</file>

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

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.

3 issues found across 6 files

Prompt for AI agents (all 3 issues)

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


<file name="packages/features/bookings/lib/service/RegularBookingService.ts">

<violation number="1" location="packages/features/bookings/lib/service/RegularBookingService.ts:1282">
Rule violated: **Avoid Logging Sensitive Information**

Do not emit guest email addresses to the trace logs. Logging guestsRemoved writes the raw attendee emails, which are PII and must be withheld from logs to comply with the sensitive-data logging policy.</violation>
</file>

<file name="apps/web/pages/api/book/event.ts">

<violation number="1" location="apps/web/pages/api/book/event.ts:71">
Rule violated: **Avoid Logging Sensitive Information**

Do not log bookingUid; it directly identifies a booking record and leaks sensitive data. Remove this field from the log payload to comply with the &quot;Avoid Logging Sensitive Information&quot; rule.</violation>
</file>

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

<violation number="1" location="packages/lib/server/defaultResponder.ts:55">
Wrapping errors in TracedError before getServerErrorFromUnknown breaks HttpError/TRPCError handling, returning 500 instead of their intended status codes.</violation>
</file>

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

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

E2E results are ready!

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 some comments, major problem is something went wrong while resolving conflicts it seems

@github-actions github-actions bot marked this pull request as draft November 4, 2025 08:50
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 3 files

@Udit-takkar Udit-takkar marked this pull request as ready for review November 6, 2025 10:15
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.

1 issue found across 7 files

Prompt for AI agents (all 1 issues)

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


<file name="packages/features/bookings/lib/service/RegularBookingService.ts">

<violation number="1" location="packages/features/bookings/lib/service/RegularBookingService.ts:1256">
Rule violated: **Avoid Logging Sensitive Information**

Logging `guestsRemoved` outputs the guests’ raw email addresses, violating our no-PII logging policy. Please avoid emitting the actual emails in logs.</violation>
</file>

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


if (guestsRemoved.length > 0) {
log.info("Removed guests from the booking", guestsRemoved);
tracingLogger.info("Removed guests from the booking", guestsRemoved);
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 6, 2025

Choose a reason for hiding this comment

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

Rule violated: Avoid Logging Sensitive Information

Logging guestsRemoved outputs the guests’ raw email addresses, violating our no-PII logging policy. Please avoid emitting the actual emails in logs.

Prompt for AI agents
Address the following comment on packages/features/bookings/lib/service/RegularBookingService.ts at line 1256:

<comment>Logging `guestsRemoved` outputs the guests’ raw email addresses, violating our no-PII logging policy. Please avoid emitting the actual emails in logs.</comment>

<file context>
@@ -1233,7 +1253,7 @@ async function handler(
 
   if (guestsRemoved.length &gt; 0) {
-    log.info(&quot;Removed guests from the booking&quot;, guestsRemoved);
+    tracingLogger.info(&quot;Removed guests from the booking&quot;, guestsRemoved);
   }
 
</file context>
Suggested change
tracingLogger.info("Removed guests from the booking", guestsRemoved);
tracingLogger.info("Removed guests from the booking", { count: guestsRemoved.length });
Fix with Cubic

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 7 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/lib/service/RegularBookingService.ts">

<violation number="1" location="packages/features/bookings/lib/service/RegularBookingService.ts:1256">
Rule violated: **Avoid Logging Sensitive Information**

Logging `guestsRemoved` leaks guest email addresses to the tracing logger, violating the &quot;Avoid Logging Sensitive Information&quot; rule. Please remove or redact the emails before logging.</violation>
</file>

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

<violation number="1" location="packages/lib/server/defaultResponder.ts:53">
Setting the X-Trace-Id header after the handler runs misses most responses because defaultResponder handlers usually call res.status(...).json(...), which ends the response before this code executes. Please set the header before invoking the handler so it is always attached.</violation>
</file>

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


if (guestsRemoved.length > 0) {
log.info("Removed guests from the booking", guestsRemoved);
tracingLogger.info("Removed guests from the booking", guestsRemoved);
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 11, 2025

Choose a reason for hiding this comment

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

Rule violated: Avoid Logging Sensitive Information

Logging guestsRemoved leaks guest email addresses to the tracing logger, violating the "Avoid Logging Sensitive Information" rule. Please remove or redact the emails before logging.

Prompt for AI agents
Address the following comment on packages/features/bookings/lib/service/RegularBookingService.ts at line 1256:

<comment>Logging `guestsRemoved` leaks guest email addresses to the tracing logger, violating the &quot;Avoid Logging Sensitive Information&quot; rule. Please remove or redact the emails before logging.</comment>

<file context>
@@ -1233,7 +1253,7 @@ async function handler(
 
   if (guestsRemoved.length &gt; 0) {
-    log.info(&quot;Removed guests from the booking&quot;, guestsRemoved);
+    tracingLogger.info(&quot;Removed guests from the booking&quot;, guestsRemoved);
   }
 
</file context>
Suggested change
tracingLogger.info("Removed guests from the booking", guestsRemoved);
tracingLogger.info("Removed guests from the booking", { count: guestsRemoved.length });
Fix with Cubic


ok = true;
if (result && !res.writableEnded) {
res.setHeader("X-Trace-Id", traceContext.traceId);
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 11, 2025

Choose a reason for hiding this comment

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

Setting the X-Trace-Id header after the handler runs misses most responses because defaultResponder handlers usually call res.status(...).json(...), which ends the response before this code executes. Please set the header before invoking the handler so it is always attached.

Prompt for AI agents
Address the following comment on packages/lib/server/defaultResponder.ts at line 53:

<comment>Setting the X-Trace-Id header after the handler runs misses most responses because defaultResponder handlers usually call res.status(...).json(...), which ends the response before this code executes. Please set the header before invoking the handler so it is always attached.</comment>

<file context>
@@ -17,22 +25,37 @@ export function defaultResponder&lt;T&gt;(
 
       ok = true;
       if (result &amp;&amp; !res.writableEnded) {
+        res.setHeader(&quot;X-Trace-Id&quot;, traceContext.traceId);
         return res.json(result);
       }
</file context>
Fix with Cubic

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.

4 participants