Skip to content

docs: Add OTLP arch docs #3063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 22, 2025

Conversation

scottgerring
Copy link
Contributor

@scottgerring scottgerring commented Jul 11, 2025

Fixes #3062 , documenting the architecture of the OTLP subsystem. I've written this as I dive into OTLP as part of reviewing it against the specification and figure it is helpful context for others working on opentelemetry-rust.

Changes

Please provide a brief description of the changes here.

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@scottgerring scottgerring requested a review from cijothomas July 11, 2025 09:32
@scottgerring scottgerring requested a review from a team as a code owner July 11, 2025 09:32
@scottgerring scottgerring added priority:p3 Lowest priority issues and bugs. M-exporter-otlp labels Jul 11, 2025
@scottgerring scottgerring marked this pull request as draft July 11, 2025 09:34
Copy link

codecov bot commented Jul 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.1%. Comparing base (5e447d0) to head (f83b803).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #3063     +/-   ##
=======================================
+ Coverage   80.0%   80.1%   +0.1%     
=======================================
  Files        126     126             
  Lines      21879   21949     +70     
=======================================
+ Hits       17519   17603     +84     
+ Misses      4360    4346     -14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@scottgerring scottgerring force-pushed the chore/add-otlp-docs branch from a195591 to ab1458e Compare July 11, 2025 09:41
@scottgerring scottgerring changed the title chore: Add OTLP arch docs doc: Add OTLP arch docs Jul 11, 2025
@scottgerring scottgerring marked this pull request as ready for review July 11, 2025 09:42
@scottgerring scottgerring changed the title doc: Add OTLP arch docs docs: Add OTLP arch docs Jul 11, 2025
Copy link
Contributor

@TommyCpp TommyCpp left a comment

Choose a reason for hiding this comment

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

Nice document. The only concern I have with those kind of doc is they become stale over time if we don't actively maintain them. Could we add the commit that this is based on? At least so that we have some reference to what's the last update time

@lalitb lalitb requested a review from Copilot July 14, 2025 19:39
Copy link

@Copilot 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 introduces a dedicated architecture document for the OTLP exporter subsystem and updates the high-level architecture overview to reference it.

  • Adds docs/design/otlp.md detailing exporter structure, crate layout, runtime flow, configuration, and extension points.
  • Updates docs/design/architecture.md to include a link to the new OTLP design doc.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
docs/design/otlp.md New OTLP architecture guide covering exporters and config.
docs/design/architecture.md Added “OTLP” to the detailed design section.
Comments suppressed due to low confidence (2)

docs/design/architecture.md:29

  • To maintain consistent section numbering, consider changing this heading to ## 4 Detailed Design, aligning it with the preceding numbered sections.
## Detailed Design

docs/design/otlp.md:7

  • The overview section above is unnumbered; for consistency either number it as ## 1 Overview or remove numbering from this heading.
## Overview

@scottgerring scottgerring force-pushed the chore/add-otlp-docs branch from ab1458e to 5964019 Compare July 15, 2025 11:03
@scottgerring
Copy link
Contributor Author

Review comments addressed; as I rewrote the one commit, the comments are a little de-synced from the change; i will add a separate "address changes" commit next time.

@scottgerring scottgerring force-pushed the chore/add-otlp-docs branch from 5964019 to 0a3cbba Compare July 16, 2025 05:39
@scottgerring
Copy link
Contributor Author

Nice document. The only concern I have with those kind of doc is they become stale over time if we don't actively maintain them. Could we add the commit that this is based on? At least so that we have some reference to what's the last update time

Good idea! Added the date ✅ Can always get more granular with git blame.
I am hopeful this is written at the right level to provide 1/ a useful arch overview that is harder to tease out of the code and 2/ not have to change very often.


### 4.1 Transport-specific Client Options

Exporters must function both in binaries that start no async runtime and in services already running Tokio. Consequently the crate provides **blocking** and **non-blocking** HTTP clients behind mutually-exclusive feature-flags. The blocking variant is the default to ensure out-of-the-box operability; applications that already depend on Tokio can opt into an async client to avoid spawning extra threads.
Copy link
Member

Choose a reason for hiding this comment

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

nit: instead of stating "Tokio", could we say "async runtimes like Tokio".

| Reqwest (blocking) **(default)** | `reqwest-blocking-client` | Yes | `reqwest::blocking` | HTTP/1.1 → auto-upgrades to HTTP/2 over TLS | CLI tools, synchronous binaries, or hybrid apps where introducing Tokio is undesirable. A helper thread is spawned once at builder time.
| Reqwest (async) | `reqwest-client` | No (Tokio) | `reqwest` | HTTP/1.1 → auto-upgrades to HTTP/2 over TLS | Services that already run a Tokio runtime and prefer fully non-blocking exports.
| Hyper | `hyper-client` | No (Tokio) | `hyper` | HTTP/1.1 → auto-upgrades to HTTP/2 over TLS | Lean dependency footprint when both Reqwest features are disabled.
| Tonic (gRPC) | `grpc-tonic` | No (Tokio) | `tonic` | HTTP/2 (gRPC) | Environments standardising on gRPC or collectors exposing only port 4317.
Copy link
Member

Choose a reason for hiding this comment

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

The "typical use case" header is not adding value IMHO. If a user want to use gRPC, then they use tonic/gRPC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't it? Aside from "I want to use gRPC so I should use the only gRPC option" I don't think choosing between the others is obvious. It might be something that belongs here alternatively/as-well even!

## 10. Key Architectural Decisions
| Decision | Rationale |
|----------|-----------|
| *Builder pattern with marker types* | Compile-time guarantee that exactly one transport is chosen. |
Copy link
Member

Choose a reason for hiding this comment

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

we do have issues about users accidentally enable multiple feature flags...

@scottgerring scottgerring force-pushed the chore/add-otlp-docs branch 3 times, most recently from b2ef5ac to 41f088d Compare July 18, 2025 10:26
@scottgerring scottgerring force-pushed the chore/add-otlp-docs branch from 41f088d to f83b803 Compare July 22, 2025 08:49
# OpenTelemetry Rust – Architecture Overview

> Status:Development
> Last Updated: 2025-07-16
Copy link
Member

Choose a reason for hiding this comment

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

nit: We have git blame for this!

Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together. We can iterate and improve it as we work on OTLP towards stability!

@cijothomas cijothomas merged commit f00456f into open-telemetry:main Jul 22, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-exporter-otlp priority:p3 Lowest priority issues and bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stabilization - OTLP - Document current OTLP architecture
3 participants