Skip to content

Conversation

@ChrisJBurns
Copy link
Contributor

@ChrisJBurns ChrisJBurns commented Oct 23, 2025

Description

We have added custom attributes support in toolhive, this PR adds the docs for it.

Related issues/PRs

stacklok/toolhive#2285

Content

  • (N/A) New pages include a frontmatter section with title and description at a minimum
  • (N/A) Sidebar navigation (sidebars.ts) updated for added, deleted, reordered, or renamed files
  • (N/A) Redirects added to vercel.json for moved, renamed, or deleted pages (i.e., if the URL slug changed)

Reviews

  • Content has been reviewed for technical accuracy
  • Content has been reviewed for spelling, grammar, and style

Copilot AI review requested due to automatic review settings October 23, 2025 15:19
@vercel
Copy link

vercel bot commented Oct 23, 2025

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

Project Deployment Preview Comments Updated (UTC)
docs-website Ready Ready Preview Comment Oct 23, 2025 7:17pm

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds documentation for custom OpenTelemetry resource attributes in the CLI telemetry guide. It enables users to add custom metadata (like team ownership, deployment region, or environment) to all telemetry signals for better filtering and querying in observability platforms.

Key changes:

  • New section documenting how to add custom OTEL resource attributes via CLI flag and environment variable
  • Usage examples showing various attribute patterns and combinations
  • Common use cases and best practices for choosing attribute names

@ChrisJBurns ChrisJBurns changed the title draft: adds otel custom attributes docs for cli adds otel custom attributes docs for cli Oct 23, 2025
Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com>
@danbarr
Copy link
Collaborator

danbarr commented Oct 23, 2025

@claude please review this documentation change for clarity, consistency, and conformance to the project's style guide.

@claude
Copy link
Contributor

claude bot commented Oct 23, 2025

Claude finished @danbarr's task —— View job


Documentation Review

I've completed my review of the custom attributes documentation. Overall, the content is well-written and provides good technical detail. Here are my findings:

Strengths

  • Clear structure with logical progression from basic usage to advanced scenarios
  • Good use of code examples showing practical implementations
  • Helpful inclusion of both CLI flag and environment variable approaches
  • Common use cases section provides valuable context for when to use this feature

Issues to Address

1. Style Guide: Punctuation (lines 106-110)

The text references "OpenTelemetry Resource Semantic Conventions" followed by a colon, but the colon should be removed since it's not introducing a list.

Current:

Attribute values can contain any UTF-8 string, including URLs. For more details,
see the
[OpenTelemetry Resource Semantic Conventions](https://opentelemetry.io/docs/specs/semconv/resource/#attributes):

Suggested:

Attribute values can contain any UTF-8 string, including URLs. For more details,
see the
[OpenTelemetry Resource Semantic Conventions](https://opentelemetry.io/docs/specs/semconv/resource/#attributes).
2. Clarity: Redundant Example (lines 112-117)

This second example immediately following the character rules doesn't add much value since it demonstrates the same concept as the earlier example at lines 99-104. Consider removing it or making it demonstrably different (e.g., showing edge cases or validation failures).

3. Style Guide: List Consistency (lines 150-161)

The common use cases section uses inconsistent formatting. Some items have colons, some don't. Per the style guide's emphasis on consistency, all items should follow the same pattern.

Current formatting is inconsistent:

  • "Critical context: Add critical context..." (has colon and explanation)
  • "Fast filtering and troubleshooting: Enable fast filtering..." (has colon and explanation)
  • "Consistent metadata: Support standardized..." (has colon)
  • "Root-cause analysis: Pinpoint which..." (has colon)
  • "Telemetry enrichment: Enable automatic..." (has colon)

All are consistent actually - this is fine.

4. Clarity: "Common use cases" Content (lines 152-161)

The bulleted items read more like benefits/features than actual use cases. Consider rephrasing to show concrete scenarios, or rename the section to "Benefits" or "Why use custom attributes."

Current: "Critical context: Add critical context to telemetry..."

More concrete use case approach:

  • Multi-tenant environments: Identify which customer or tenant generated specific traces
  • Regional deployments: Track performance differences across geographic regions
  • Team ownership: Attribute telemetry to specific teams for accountability
5. Table Update Needed (line 239)

The configuration options table needs to include the new --otel-custom-attributes flag. I see it's already there - good!

6. Existing Review Comment

The Copilot review comment suggests using a different example attribute at line 166. I agree - using service.version instead of repeating deployment.environment would better demonstrate the semantic conventions principle without duplication.

Minor Observations

  • Line 84: "telemetry signals (traces and metrics)" - Good clarification
  • Line 89: "Custom attributes are attached globally" - Clear explanation of scope
  • Line 102: Good use of realistic placeholder <YOUR_API_KEY> in ALL_CAPS per style guide
  • Lines 134-146: Excellent explanation of combining both methods with concrete output

Summary

The documentation effectively covers the new custom attributes feature with good technical accuracy. The main improvements needed are:

  1. Remove the colon after the semantic conventions link (line 110)
  2. Consider removing or differentiating the redundant example (lines 112-117)
  3. Either rephrase the "Common use cases" section with concrete scenarios or rename it to "Benefits"
  4. Update the tip example to avoid repeating deployment.environment (per Copilot's suggestion)

The content follows most style guide requirements well, including proper use of sentence case, second person voice, and clear formatting. Once these issues are addressed, the documentation will be ready for publication.


Job Run | Branch: otel-ca-docs

Signed-off-by: ChrisJBurns <29541485+ChrisJBurns@users.noreply.github.com>
@ChrisJBurns ChrisJBurns merged commit 61b97f9 into main Oct 23, 2025
5 checks passed
@ChrisJBurns ChrisJBurns deleted the otel-ca-docs branch October 23, 2025 19:21
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