Skip to content

Conversation

@amlannandy
Copy link
Member

Update docs to use traces for capturing web vitals

@vercel
Copy link

vercel bot commented Aug 5, 2025

[Resolved] [vc]: #NNm20yoXwh3qcK8qfKX6ulNIuJJL+dGVg1dcALuzvTw=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJzaWdub3otd2ViIiwiaW5zcGVjdG9yVXJsIjoiaHR0cHM6Ly92ZXJjZWwuY29tL3NpZ25vei9zaWdub3otd2ViL0d0VTRRUnhOS3RGZUtOWFZHMzNTWUtrQ3R3SEUiLCJwcmV2aWV3VXJsIjoic2lnbm96LXdlYi1naXQtc2lnLTI2Njgtc2lnbm96LnZlcmNlbC5hcHAiLCJuZXh0Q29tbWl0U3RhdHVzIjoiREVQTE9ZRUQiLCJsaXZlRmVlZGJhY2siOnsicmVzb2x2ZWQiOjAsInVucmVzb2x2ZWQiOjAsInRvdGFsIjowLCJsaW5rIjoic2lnbm96LXdlYi1naXQtc2lnLTI2Njgtc2lnbm96LnZlcmNlbC5hcHAifSwicm9vdERpcmVjdG9yeSI6bnVsbH1dfQ==
The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
signoz-web Ready Ready Preview Comment Nov 11, 2025 6:01am

@claude
Copy link
Contributor

claude bot commented Nov 10, 2025

Claude finished @amlannandy's task —— View job


Documentation Review Complete

I've completed a comprehensive review of the web vitals documentation changes according to CONTRIBUTING.md standards and addressed Makeavish's feedback.


Summary

This PR successfully restructures the web vitals documentation by:

  • Creating a clear overview page explaining metrics vs traces approaches
  • Providing two detailed implementation guides (metrics and traces)
  • Adding comprehensive dashboard template documentation

The technical content is accurate (Core Web Vitals verified against official web.dev sources), and the structure is excellent. However, there are critical issues that must be fixed before merging, plus important improvements needed to address Makeavish's feedback.


Critical Issues (Must Fix)

1. Incorrect id field in traces guide (web-vitals-with-traces.mdx:4)

  • Currently: id: opentelemetry-web-vitals-metrics
  • Should be: id: opentelemetry-web-vitals-traces
  • This creates a duplicate ID conflict with the metrics guide and will break URL routing

2. ID/filename mismatch in overview (opentelemetry-web-vitals.mdx:5)

  • The id: web-vitals doesn't match filename opentelemetry-web-vitals.mdx
  • Recommend renaming file to web-vitals.mdx with redirect, OR updating all internal links

High Priority Issues (Address Makeavish's Feedback)

3. Missing comprehensive Next Steps sections (Both guides, lines 503/520)

  • Per Makeavish: "Next Steps missing at the end"
  • Current sections only link to other instrumentation guides
  • Need to include: dashboard import, alerts setup, query builder, and clear paths for existing monitoring users
  • Detailed suggestions provided in inline comments

4. Clarity for existing frontend monitoring users (Both guides, line 14)

  • Per Makeavish: "This doc is not clear for user using existing frontend monitoring with SigNoz"
  • Current Admonitions could be much clearer about extending existing setups
  • Suggested rewording provided in inline comments to make it explicit

Technical Accuracy Verified ✅

Core Web Vitals are correctly documented across all files:

  • ✅ Largest Contentful Paint (LCP)
  • ✅ Interaction to Next Paint (INP) - correctly using current metric (replaced FID in March 2024)
  • ✅ Cumulative Layout Shift (CLS)
  • ✅ Time to First Byte (TTFB)
  • ✅ First Contentful Paint (FCP)

Source: https://web.dev/articles/vitals


Review Against CONTRIBUTING.md PR Checklist

  • Frontmatter: Complete with date, id, title, description, tags
  • ID fields: Critical issue with duplicate/mismatched IDs (see issues Changes made to comparison folder blogs #1, format /blog and other cleaning #2)
  • SEO: Keywords present in title, description, first paragraph
  • Cloud vs Self-Host: Proper tags and Admonition snippets for CORS
  • Commands: Properly formatted
  • Code/config: Well-annotated with placeholders explained
  • Validate section: Present in both guides
  • Next steps: Present but incomplete (see issue added some component, author names format,  #3)
  • Links: Using absolute URLs
  • Images: Using WebP format with proper paths
  • Sidebar: Added to docsSideNav.ts

What's Working Well

  • ✅ Excellent restructuring separating metrics and traces approaches
  • ✅ Clear technical explanations and code examples
  • ✅ Proper use of Admonition components for CORS configuration
  • ✅ Complete examples with both TypeScript and JavaScript
  • ✅ Good coverage of custom attributes for RUM features
  • ✅ Dashboard template documentation is comprehensive
  • ✅ All OpenTelemetry configurations are technically accurate

Inline Comments Posted

I've posted 5 inline comments with specific recommendations:

  1. Fix critical ID issue in traces guide
  2. Fix ID/filename mismatch in overview
  3. Enhance Next Steps in metrics guide (addresses Makeavish's feedback)
  4. Enhance Next Steps in traces guide (addresses Makeavish's feedback)
  5. Improve clarity for existing monitoring users in metrics guide
  6. Improve clarity for existing monitoring users in traces guide (note: actually 6 comments total)

Recommendation

The critical ID issues (#1, #2) must be fixed before merging as they will cause routing problems and conflicts.

The Next Steps and clarity improvements (#3, #4) are high priority to address Makeavish's valid concerns about usability for existing monitoring users.

After these fixes, the documentation will be in excellent shape and ready for merge.


@makeavish
Copy link
Member

Web vitals traces doc mentions contextManager: new ZoneContextManager() in step 2, which is not mentioned in traces frontend monitoring. But we are asking users to skip step 1 and 2 if already followed frontend monitoring @amlannandy

@claude
Copy link
Contributor

claude bot commented Nov 11, 2025

Claude finished @amlannandy's task —— View job


Documentation Inconsistency Analysis

I've investigated @makeavish's concern about the ZoneContextManager documentation inconsistency.

Todo List:

  • Read web vitals traces doc to locate ZoneContextManager usage
  • Find and read main frontend monitoring traces documentation
  • Compare the two approaches and identify inconsistency
  • Provide analysis and recommendation

Issue Confirmed

The Problem:

The web vitals traces doc (web-vitals-with-traces.mdx) tells users on line 14:

"If you've followed the Sending Traces from your frontend application guide, you can skip Steps 1 and 2 below—your instrumentation file is already set up."

However, there's a critical difference between the two approaches:

Web Vitals Traces Doc (lines 97-99):

provider.register({
  contextManager: new ZoneContextManager(),
});

Main Frontend Traces Doc (sending-traces.mdx, line 125 & 169):

provider.register();

The main frontend traces documentation does NOT include contextManager: new ZoneContextManager() in the provider.register() call, even though it installs @opentelemetry/context-zone as a dependency (lines 55, 69).


Why This Matters

The ZoneContextManager is important for maintaining trace context across async operations in browser environments. Without it, you may lose trace context in certain scenarios. If users skip Steps 1-2 thinking their setup is complete, they'll miss this crucial configuration difference.


Recommendations

Option 1 (Recommended): Update the main frontend traces documentation (sending-traces.mdx) to include ZoneContextManager:

import { ZoneContextManager } from '@opentelemetry/context-zone';

// ... existing code ...

provider.register({
  contextManager: new ZoneContextManager(),
});

This makes the "skip steps 1-2" statement accurate and improves the main traces guide.

Option 2: Remove or modify the "skip steps 1-2" guidance in web-vitals-with-traces.mdx to clarify that Step 2 has an important difference:

**Already sending frontend traces?** If you've followed the Sending Traces guide, you can skip Step 1, but you should review Step 2 as it includes `ZoneContextManager` configuration which may differ from your existing setup.

Option 3: Add a note in the web vitals traces doc explicitly calling out the ZoneContextManager difference for users who followed the main traces guide.


Additional Context

According to OpenTelemetry documentation, ZoneContextManager is recommended for browser environments to properly propagate context across async operations. The main traces doc should probably include it regardless of the web vitals use case.

Source: https://opentelemetry.io/docs/languages/js/context/#context-managers


Would you like me to implement any of these fixes?

@amlannandy
Copy link
Member Author

Web vitals traces doc mentions contextManager: new ZoneContextManager() in step 2, which is not mentioned in traces frontend monitoring. But we are asking users to skip step 1 and 2 if already followed frontend monitoring @amlannandy

Okay then the sending-traces doc should also have it. I will update that doc in the other PR that we have to include the ZoneContextManager

@claude
Copy link
Contributor

claude bot commented Nov 11, 2025

Claude finished @amlannandy's task —— View job


Documentation Review Complete ✅

I've completed a comprehensive review of the web vitals documentation changes against CONTRIBUTING.md standards.


Summary

This PR successfully restructures the web vitals documentation by:

  • Creating a clear overview page (opentelemetry-web-vitals.mdx) explaining metrics vs traces approaches
  • Providing two detailed implementation guides (metrics and traces)
  • Adding comprehensive dashboard template documentation

The technical content is accurate and follows OpenTelemetry best practices. Core Web Vitals (LCP, INP, CLS, TTFB, FCP) are correctly documented per Google's 2025 standards (Source: https://web.dev/articles/vitals).


Review Against CONTRIBUTING.md PR Checklist

Frontmatter: Complete with date, id, title, description, and correct tags: [SigNoz Cloud, Self-Host]
SEO: Keywords present in title, description, and first paragraph
Cloud vs Self-Host: Proper tags and Admonition snippets for CORS configuration
Commands: Properly formatted with each package on separate lines
Code/config snippets: Well-annotated with placeholders explained
Validate section: Present in both guides (metrics:280, traces:322)
Next Steps: Included with links to related docs (metrics:502-504, traces:519-521)
Links: Using absolute URLs for internal links in docs
Images: Using WebP format with proper paths and captions
Sidebar: Added to docsSideNav.ts with proper structure


Technical Accuracy Verified ✅

Core Web Vitals are correctly documented across all files:

  • Largest Contentful Paint (LCP) - Loading performance
  • Interaction to Next Paint (INP) - Interactivity (correctly replaced deprecated FID in March 2024)
  • Cumulative Layout Shift (CLS) - Visual stability

Additional Web Vitals:

  • Time to First Byte (TTFB)
  • First Contentful Paint (FCP)

Source: https://web.dev/articles/vitals

All OpenTelemetry configurations follow current best practices for web instrumentation.


Status of Previous Review Issues

Comparing with all previous Claude reviews (Nov 4-5), the following have been successfully resolved:

✅ Tags corrected to [SigNoz Cloud, Self-Host]
✅ CORS configuration has proper context with Admonition blocks
✅ Shell commands properly formatted (one package per line)
✅ Placeholder format using single angle brackets <>
✅ Code block language annotations corrected
✅ Dependency descriptions accurate for both guides
✅ Duplicate content removed
✅ Overview sections added
✅ Dashboard template has proper frontmatter and tags
✅ Validate sections properly structured
✅ Malformed code blocks fixed


Addressing @makeavish's Feedback

1. ZoneContextManager Inconsistency (Resolved)

@makeavish noted that the web vitals traces doc uses ZoneContextManager in Step 2 (line 98), which differs from the main sending-traces guide.

Status: @amlannandy confirmed this will be addressed in a separate PR by adding ZoneContextManager to the main sending-traces.mdx documentation. This is the correct approach to maintain consistency across all frontend tracing guides.

2. Missing Next Steps

Status: ✅ Resolved. Both guides now include Next Steps sections:

  • Metrics guide (lines 502-504): Links to sending logs and sending traces
  • Traces guide (lines 519-521): Links to sending logs and sending metrics

3. Clarity for Existing Frontend Monitoring Users

Status: ✅ Well-addressed. Both guides include prominent Admonition blocks at the top (lines 13-15 in each) that clearly explain:

  • Users with existing frontend monitoring can skip Steps 1 and 2
  • Direct link to jump to Step 3 for web vitals capture
  • Clear reference to the prerequisite guide

Minor Observations (Non-blocking)

  1. Relative vs Absolute Links in Dashboard Template (lines 34, 47, 88, 100 in frontend-monitoring.mdx):

    • Currently uses relative paths like /docs/...
    • CONTRIBUTING.md line 163 prefers absolute URLs: https://signoz.io/docs/...
    • Note: These links are functional; this is a style preference for consistency
  2. entityName prop on Tabs components (metrics:61, 176, 299; traces:70, 171, 343):

    • The entityName="language" prop appears on several <Tabs> components
    • While not harmful, it may be unnecessary depending on your Tabs implementation

What's Working Exceptionally Well

  • Excellent restructuring: Clear separation between overview, metrics guide, and traces guide
  • Proper formatting: Shell commands, code blocks, and placeholders all follow standards
  • Good context: CORS configuration has proper Admonition blocks explaining when/why it's needed
  • Complete examples: Both TypeScript and JavaScript versions provided throughout
  • Rich features: Custom attributes sections show how to add RUM capabilities
  • Dashboard integration: New dashboard template documentation is comprehensive with proper DashboardActions components
  • Accessibility: All images have alt text and captions using proper Figure/figure syntax
  • Navigation: Sidebar properly organized with WebVitalsGrid component in overview
  • User guidance: Clear Admonitions help users understand when to skip steps if they already have instrumentation

Recommendation

This PR is ready for merge.

The documentation successfully addresses all critical issues from previous reviews and properly implements feedback from @makeavish. The restructuring significantly improves clarity and provides excellent guidance for implementing web vitals monitoring with SigNoz.

The minor observations about link format and component props are style preferences that don't impact functionality or user experience.


Files Reviewed

  • data/docs/frontend-monitoring/web-vitals-with-metrics.mdx (504 lines)
  • data/docs/frontend-monitoring/web-vitals-with-traces.mdx (521 lines)
  • data/docs/frontend-monitoring/opentelemetry-web-vitals.mdx (52 lines)
  • data/docs/dashboards/dashboard-templates/frontend-monitoring.mdx (101 lines)
  • constants/docsSideNav.ts

Overall Assessment: This is a significant improvement to the SigNoz documentation. The restructuring makes it much easier for users to understand and implement web vitals monitoring, with clear guidance on when to use metrics vs traces and excellent support for users with existing instrumentation.

No blocking issues found.


@makeavish makeavish enabled auto-merge (squash) November 11, 2025 07:26
@makeavish makeavish merged commit 3a294ed into main Nov 11, 2025
10 checks passed
@makeavish makeavish deleted the SIG-2668 branch November 11, 2025 07:40
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.

3 participants