Skip to content

fix(docs): Update OpenTelemetry Collector binary docs#2545

Draft
crazyuploader wants to merge 6 commits intomainfrom
fix/otel-collector-config
Draft

fix(docs): Update OpenTelemetry Collector binary docs#2545
crazyuploader wants to merge 6 commits intomainfrom
fix/otel-collector-config

Conversation

@crazyuploader
Copy link
Contributor

@crazyuploader crazyuploader commented Jan 17, 2026

Closes: #3695

Doc: Link

@crazyuploader crazyuploader self-assigned this Jan 17, 2026
@crazyuploader crazyuploader added documentation Improvements or additions to documentation documentation-update labels Jan 17, 2026
@vercel
Copy link

vercel bot commented Jan 17, 2026

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

Project Deployment Review Updated (UTC)
signoz-web Ready Ready Preview, Comment Jan 17, 2026 11:25am

Request Review

@crazyuploader
Copy link
Contributor Author

@claude /review

@claude
Copy link
Contributor

claude bot commented Jan 17, 2026

Claude finished @crazyuploader's task in 2m 43s —— View job


Code Review Complete ✓

  • Read CONTRIBUTING.md for review guidelines
  • Get PR diff and understand changes
  • Review modified files for documentation quality
  • Verify technical accuracy of OpenTelemetry configurations
  • Check for formatting and structural issues
  • Determine if "add-to-onboarding" label is needed
  • Provide inline comments for specific issues
  • Post final review summary

Review Summary

This PR consolidates and updates OpenTelemetry Collector binary documentation. I've reviewed all 6 changed files against the CONTRIBUTING.md checklist. The changes are generally well-structured, but I've identified several issues that need attention.

Critical Issues

1. NGINX Documentation - Malformed Code Blocks

File: data/docs/instrumentation/opentelemetry-nginx.mdx

The "Compile Locally" bash code blocks (appears in 3 places) have multiple commands run together without proper line breaks. This makes them impossible to copy-paste correctly. See inline comment for the fix.

Lines affected: 31-41, 103-113, 224-235

2. Missing Frontmatter Fields

File: data/docs/tutorial/opentelemetry-binary-usage-in-virtual-machine.mdx

The doc is missing the doc_type field in frontmatter, which is required per CONTRIBUTING.md. Based on the content (end-to-end setup guide), this should be:

doc_type: tutorial

Note: It's already present in the current version, so this is good! ✓

3. Technical Accuracy Check Needed

File: data/docs/instrumentation/opentelemetry-nginx.mdx (line ~67 in modified content)

The OTLP endpoint format needs verification. According to OpenTelemetry nginx module docs, when using OTLP/HTTP, the endpoint should be the base collector endpoint without /v1/traces suffix, as the exporter automatically appends it.

Current: endpoint: https://ingest.<region>.signoz.cloud:443/v1/traces

Verify if this should be: endpoint: https://ingest.<region>.signoz.cloud:443

Otherwise, requests may go to /v1/traces/v1/traces (double path).

Good Practices Observed ✓

  1. Redirect added correctly: The old doc path (/docs/tutorial/opentelemetry-binary-usage/) is properly redirected to the new path in next.config.js (line 1242-1245).

  2. Self-hosted callout included: The new binary doc includes the required self-hosted callout per CONTRIBUTING.md guidelines.

  3. Configuration properly annotated: Code blocks include language identifiers and explanations of placeholders.

  4. Memory limiter added: The new collector config includes memory_limiter processor, which is a best practice to prevent OOM issues (OpenTelemetry docs).

  5. Date updated consistently: All modified docs have date: 2026-01-17.

Minor Issues

1. Inconsistent Configuration Between Docs

File: components/shared/otel-binary-cloud-config.md vs. data/docs/tutorial/opentelemetry-binary-usage-in-virtual-machine.mdx

The shared component removed the service.telemetry.metrics section (lines 60-62 deleted), but the main doc added memory_limiter processor. These configs should align for consistency, or explicitly document why they differ.

Deleted from shared component:

service:
  telemetry:
    metrics:
      address: 0.0.0.0:8888

This section exposed Collector's internal metrics on port 8888. If removed intentionally, ensure users know how to monitor the Collector itself.

2. Prometheus Receiver Target Commented Out

File: data/docs/tutorial/opentelemetry-binary-usage-in-virtual-machine.mdx (line 168)

scrape_configs:
  - job_name: otel-collector-binary
    static_configs:
      - targets:
        # - localhost:8888 # Replace the target endpoint with a Prometheus exporter

The comment says "Replace the target endpoint" but doesn't explain what to replace it with or when. Consider adding guidance like:

# - localhost:8888  # Uncomment to scrape Collector's own metrics
# - localhost:9090  # Add your application's Prometheus metrics endpoint here

3. Windows Limitation Not Documented

File: data/docs/tutorial/opentelemetry-binary-usage-in-virtual-machine.mdx (line 158)

The config includes processes: {} scraper with comment "Linux/macOS only, not supported on Windows". Per CONTRIBUTING.md, platform-specific limitations should be prominently called out. Consider adding an Admonition:

<Admonition type="warning">
Windows users: Remove the `processes` scraper from the hostmetrics receiver as it's not supported on Windows.
</Admonition>

Checklist Against CONTRIBUTING.md

  • Frontmatter includes date, id, title, description, tags, doc_type
  • Commands explain what they do
  • Code/config snippets are annotated (mostly - see minor issues)
  • Validation section included
  • Troubleshooting section included
  • Cross-links to related docs included
  • Redirect added for renamed/moved doc
  • Code blocks properly formatted ← Needs fix (NGINX doc)
  • Images use WebP format (no images in this PR)
  • Technical accuracy verified ← Needs verification (NGINX endpoint)

Add to Onboarding Label

Not required. This PR modifies existing documentation rather than adding new integration/collection methods. The label add-to-onboarding is only needed when a NEW documentation file is added that explains how to send data to SigNoz Cloud.

Recommendations

  1. Fix the NGINX bash code blocks immediately - this blocks users from following the instructions
  2. Verify the OTLP endpoint format for the NGINX module configuration
  3. Document why service.telemetry.metrics was removed or add it back with explanation
  4. Add clearer guidance for the Prometheus receiver target placeholder
  5. Add Windows limitation admonition for visibility

Technical Verification Sources


| Branch: fix/otel-collector-config

@crazyuploader crazyuploader marked this pull request as ready for review January 17, 2026 10:33
@claude
Copy link
Contributor

claude bot commented Jan 17, 2026

Claude finished @crazyuploader's task in 3m 43s —— View job


Code Review Complete ✓

  • Read CONTRIBUTING.md for review guidelines
  • Get PR diff and check what changed
  • Review if previous issues were addressed
  • Check technical accuracy of OpenTelemetry configurations
  • Verify endpoint formats and configuration keys
  • Check all modified files in detail
  • Verify web sources for technical claims
  • Post inline comments for specific issues
  • Determine if "add-to-onboarding" label is needed
  • Post final review summary

Review Summary

This PR consolidates and updates OpenTelemetry Collector binary documentation. I've reviewed all 6 changed files against the CONTRIBUTING.md guidelines. The changes are well-structured and most previous review issues have been addressed.

What Changed

  1. Renamed/moved doc: opentelemetry-binary-usage.mdxopentelemetry-binary-usage-in-virtual-machine.mdx with proper redirect
  2. Updated configurations: Modernized collector configs (e.g., loggingdebug exporter, updated service.telemetry.metrics)
  3. Date updates: All modified docs now show date: 2026-01-17
  4. Formatting improvements: Better spacing, code block annotations, cleaner tables

Previous Review Status

FIXED: The NGINX bash code blocks are now properly formatted with each command on its own line (lines 40-49, 103-113, 224-235)

VERIFIED: NGINX OTLP endpoint format is correct - https://ingest.<region>.signoz.cloud:443/v1/traces includes the /v1/traces path as required by the OpenTelemetry NGINX module (source)

DOCUMENTED: The service.telemetry.metrics change from address: 0.0.0.0:8888 to level: basic is correct and modernizes the config (see inline comment for details)

Issues Found (See Inline Comments)

I've posted 4 inline comments for specific issues:

  1. Vague Prometheus target comment (opentelemetry-binary-usage-in-virtual-machine.mdx:168) - Needs clearer guidance on what to replace
  2. Windows limitation not prominent (opentelemetry-binary-usage-in-virtual-machine.mdx:158) - Should use an Admonition for visibility
  3. Duplicate text (opentelemetry-binary-usage-in-virtual-machine.mdx:305) - "you can create a Windows Service, you can create a service"
  4. Config change documentation (otel-binary-cloud-config.md:62) - Context about the telemetry metrics change

Checklist Against CONTRIBUTING.md

  • Frontmatter includes date, id, title, description, tags, doc_type
  • Commands explain what they do
  • Code/config snippets are annotated
  • Validation section included
  • Troubleshooting section included (with proper <ToggleHeading> collapsible)
  • Cross-links to related docs included
  • Redirect added for renamed/moved doc
  • Code blocks properly formatted
  • Technical accuracy verified with OpenTelemetry sources
  • Self-hosted callout included in tutorial doc
  • ECS doc correctly updated loggingdebug exporter

Technical Verification

All configurations have been verified against official OpenTelemetry documentation:

Add to Onboarding Label

Not required. This PR modifies/renames existing documentation rather than adding a NEW doc that explains how to send data to SigNoz Cloud. The label is only needed when brand new integration/collection docs are added.

Recommendations

  1. Address the inline comments - particularly the duplicate text and clearer Prometheus guidance
  2. Consider the Windows limitation visibility - An Admonition would help Windows users avoid configuration errors
  3. The telemetry metrics change is fine but ensure users who were relying on port 8888 know how to re-enable it if needed

Overall Assessment

The PR is in good shape. The documentation updates are accurate, well-structured, and follow the guidelines. The technical configurations have been verified against official OpenTelemetry sources. The main items are minor improvements suggested in the inline comments.


| Branch: fix/otel-collector-config

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation documentation-update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant