Skip to content

Expose OpenTelemetry SpanKind in manual span APIs#173

Closed
freddyuniverse wants to merge 1 commit into
pydantic:mainfrom
freddyuniverse:feat/manual-span-kind
Closed

Expose OpenTelemetry SpanKind in manual span APIs#173
freddyuniverse wants to merge 1 commit into
pydantic:mainfrom
freddyuniverse:feat/manual-span-kind

Conversation

@freddyuniverse

Copy link
Copy Markdown

Summary

  • add a dedicated SpanOptions type with optional OpenTelemetry SpanKind
  • forward kind through span(), startSpan(), startPendingSpan(), and instrument()
  • preserve the requested kind on pending-span placeholders while keeping the default INTERNAL behavior
  • document a manual remote-operation example and add a logfire minor changeset

Testing

  • workspace package build
  • vp run logfire#test (434 tests passed)
  • vp run logfire#typecheck
  • targeted lint for the changed TypeScript files
  • vp fmt --check
  • vp check

Fixes #171

@freddyuniverse
freddyuniverse force-pushed the feat/manual-span-kind branch from b149eff to 71152eb Compare July 16, 2026 21:54
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c60441f1-d3b8-4790-999f-cf56dae3c0d5

📥 Commits

Reviewing files that changed from the base of the PR and between b149eff and 71152eb.

📒 Files selected for processing (4)
  • .changeset/manual-span-kind.md
  • docs/packages/logfire.md
  • packages/logfire-api/src/index.ts
  • packages/logfire-api/src/startPendingSpan.integration.test.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • pydantic/logfire (manual)
  • pydantic/pydantic-ai (manual)
🚧 Files skipped from review as they are similar to previous changes (3)
  • .changeset/manual-span-kind.md
  • packages/logfire-api/src/startPendingSpan.integration.test.ts
  • packages/logfire-api/src/index.ts

📝 Walkthrough

Walkthrough

Manual span APIs now accept an optional OpenTelemetry SpanKind through SpanOptions. The value is resolved and forwarded for startSpan, span, startPendingSpan, instrument, and LogfireClient.startSpan; pending placeholders use the same kind as their real spans. Integration tests cover multiple span kinds, and documentation plus a changeset describe the new option.

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The added bookkeeping metadata constant(s) are unrelated to #171 and look like extra scope beyond the feature work. Remove or justify the bookkeeping metadata additions if they are not required for this feature.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the main feature change: exposing SpanKind in manual span APIs.
Description check ✅ Passed The description matches the changeset and summarizes kind forwarding, tests, docs, and release notes.
Linked Issues check ✅ Passed The PR appears to satisfy #171 by adding SpanKind support, preserving pending kinds, and adding tests and docs.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/packages/logfire.md`:
- Line 1: Remove the __OPENAI_LIBRARY_FILE_READ_RECEIPT_V1__ metadata markers
from the logfire.md documentation, including the corresponding occurrence near
the end of the file, while preserving all actual documentation content.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e7155ab0-4244-4228-87fe-43c4b11655d3

📥 Commits

Reviewing files that changed from the base of the PR and between 333e8b9 and b149eff.

📒 Files selected for processing (4)
  • .changeset/manual-span-kind.md
  • docs/packages/logfire.md
  • packages/logfire-api/src/index.ts
  • packages/logfire-api/src/startPendingSpan.integration.test.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • pydantic/logfire (manual)
  • pydantic/pydantic-ai (manual)

Comment thread docs/packages/logfire.md Outdated
@@ -1,3 +1,4 @@
__OPENAI_LIBRARY_FILE_READ_RECEIPT_V1__={"file_name":"logfire.md","file_id":"file_000000009c3c71fbaa3b2b8dee66116c","library_file_id":"libfile_c1d2e537be808191aa0970cb6611fd6c"}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove internal metadata from the published documentation.

These markers are not documentation content and will be rendered or retained in the source document.

Also applies to: 369-369

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/packages/logfire.md` at line 1, Remove the
__OPENAI_LIBRARY_FILE_READ_RECEIPT_V1__ metadata markers from the logfire.md
documentation, including the corresponding occurrence near the end of the file,
while preserving all actual documentation content.

@freddyuniverse

Copy link
Copy Markdown
Author

I realized #172 was already open for this issue before this PR and now addresses the runtime log-kind edge case raised in review. I’m closing this as a duplicate to avoid splitting the review effort. Sorry I missed the overlap, and thanks for taking a look!

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.

Expose OpenTelemetry SpanKind in manual span APIs

1 participant