Skip to content

chore: Refactor OTel concerns in preparation for metric provider#123

Open
morgan-wowk wants to merge 1 commit intomasterfrom
organize-opentelemetry
Open

chore: Refactor OTel concerns in preparation for metric provider#123
morgan-wowk wants to merge 1 commit intomasterfrom
organize-opentelemetry

Conversation

@morgan-wowk
Copy link
Collaborator

@morgan-wowk morgan-wowk commented Feb 25, 2026

Refactor OpenTelemetry instrumentation into modular package

Changes:

  • Restructured OpenTelemetry code into a dedicated package with clear public API
  • Split provider setup and FastAPI instrumentation into separate functions (setup_providers() and instrument_fastapi())
  • Added configuration validation and error handling for OpenTelemetry settings
  • Implemented provider state checking to conditionally enable instrumentation only when providers are configured
  • Created internal modules for shared configuration resolution and provider state management
  • Added comprehensive test coverage for all new modules and functions
  • Updated main application files to use the new simplified API calls

Copy link
Collaborator Author

morgan-wowk commented Feb 25, 2026

@morgan-wowk morgan-wowk self-assigned this Feb 25, 2026
@morgan-wowk morgan-wowk force-pushed the organize-opentelemetry branch 2 times, most recently from 8a5aa83 to 7672232 Compare February 25, 2026 10:13
@morgan-wowk morgan-wowk changed the title chore: Organize opentelemetry concerns in preparation for metric provider chore: Organize OTel concerns in preparation for metric provider Feb 25, 2026
@morgan-wowk morgan-wowk changed the title chore: Organize OTel concerns in preparation for metric provider chore: Refactor OTel concerns in preparation for metric provider Feb 25, 2026
@morgan-wowk morgan-wowk force-pushed the organize-opentelemetry branch from 7672232 to 0d953d1 Compare February 25, 2026 13:01
@morgan-wowk morgan-wowk marked this pull request as ready for review February 25, 2026 19:53
@morgan-wowk morgan-wowk requested a review from Ark-kun as a code owner February 25, 2026 19:53
Copy link
Collaborator

@yuechao-qin yuechao-qin left a comment

Choose a reason for hiding this comment

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

Only

…ider

**Changes:**

* Better organize opentelemetry code to create a better API for callers (various entrypoints) to:
    * Setup providers (tracing, metrics, logging eventually)
    * Setup FastAPI auto instrumentation

Made-with: Cursor
@morgan-wowk morgan-wowk force-pushed the organize-opentelemetry branch from 0d953d1 to f2c09c1 Compare February 26, 2026 00:56
Copy link
Collaborator Author

@yuechao-qin Feedback addressed.

# region: OpenTelemetry initialization
from cloud_pipelines_backend.instrumentation import opentelemetry as otel

otel.setup_providers()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: for local setup, should otel service be run separately? What would happen if no OTEL endpoint is configured (e.g. in case of e2e tests on CI)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great question.

I'll answer in two ways.

For local development, we have this example stack I added and have personally been using to test traces and metrics for OSS: https://github.com/TangleML/tangle/tree/master/examples/observability/otel-jaeger-prometheus

For writing tests, or adding new metrics, here are some important things to know:

  • The OTel Python SDK that we consume, and even our FastAPI auto-instrumentation consumes, is intentionally designed so that every action is a NOOP if no providers are set.
    • It means, no exceptions from emitting metrics or opening traces when OTel is not configured.
  • We only set providers on our end if they have filled out correctly shaped values for the environment variables.
  • We only auto-instrument FastAPI if providers are set.
  • For verfiying a metric, or trace, was used, via testing (whether its unit or e2e) that is possible and I'll follow up with instructions and examples in a separate PR. Consider this is a TODO. We will have to manipulate the type of reader / exporter we use at the start of tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here is an issue: #132

@Ark-kun
Copy link
Contributor

Ark-kun commented Mar 3, 2026

Updated main application files to use the new simplified API calls

Hmm. Are you sure we're simplifying things? It's hard to get more simple than the current 1-liner:

otel_tracing.setup_api_tracing(app)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants