Skip to content

feat: OpenTelemetry (OTel) observability for the community API#9419

Open
sriramveeraghanta wants to merge 6 commits into
previewfrom
feat/otel-api-observability
Open

feat: OpenTelemetry (OTel) observability for the community API#9419
sriramveeraghanta wants to merge 6 commits into
previewfrom
feat/otel-api-observability

Conversation

@sriramveeraghanta

@sriramveeraghanta sriramveeraghanta commented Jul 13, 2026

Copy link
Copy Markdown
Member

Description

Ports the API slice of the enterprise OpenTelemetry PR (makeplane/plane-ee#7643) into the community edition. Self-hosters can now get OTLP-compatible APM for the Django API — a server span per request with DB / Redis / HTTP-client child spans, a span per Celery task (traceparent propagated through the queue), HTTP server metrics, and trace-correlated JSON logs — by setting two environment variables and pointing at any OTLP-compatible backend (Jaeger, Tempo, Datadog Agent, Honeycomb, Grafana Cloud, …).

Everything is off by default (OTEL_ENABLED=0) and gated behind OTEL_ENABLED + OTEL_EXPORTER_OTLP_ENDPOINT, so existing deployments are unaffected until they opt in. The off path is byte-for-byte unchanged (no OTLP connections, log schema untouched).

Scope is the community API only. pi / silo (which don't exist in the community repo), packages/observability, all node/browser/SSR instrumentation, and the FRONTEND_OTEL_* browser-telemetry plumbing from the EE PR are intentionally excluded.

Highlights:

  • New apps/api/plane/observability/ package — configure_otel() (idempotent, gated) + flush_otel() + TraceContextFilter. Sets up a TracerProvider (parent-based 10% head sampler) and MeterProvider, selects grpc/http exporters by OTEL_EXPORTER_OTLP_PROTOCOL, instruments Django / Celery / Psycopg / Redis / Requests / HTTPX, and pins noisy opentelemetry.* loggers to WARNING.
  • configure_otel() is called at the top of wsgi.py, asgi.py, manage.py, and celery.py before Django wires up, so DjangoInstrumentor patches the WSGI/ASGI handler with zero view/serializer changes.
  • Trace-correlated JSON logs are wired via Django's LOGGING dict (both settings files). Because the community Celery workers attach their own JSON handlers (they don't route through dictConfig), a small CE-specific gated filter in celery.py gives worker logs the same trace_id / span_id / service_name fields — off-path handler setup is unchanged.
  • Celery worker_process_shutdown flushes buffered spans/metrics (prefork children exit via os._exit and skip atexit).
  • Adds the 7 new opentelemetry-instrumentation-* / OTLP-HTTP pins to requirements/base.txt (the 5 base OTel packages were already present).
  • Wires the OTel env into api / worker / beat-worker in the cli/community compose (not migrator), and documents the vars in the cli/community and aio/community variables.env files.
  • Adds a self-hoster guide and a reference collector config under docs/otel-api-observability/.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Improvement (change that would cause existing functionality to not work as expected)
  • Code refactoring
  • Performance improvements
  • Documentation update

Screenshots and Media (if applicable)

Test Scenarios

Off by default (no regression):

  • Boot the API/worker with OTEL_ENABLED unset → normal startup, no OTLP connection attempts, and JSON logs keep their existing schema (no trace_id/span_id). Verified: manage.py check is clean, and the full test suite collects 494 tests with no import errors.

Unit tests (plane/tests/unit/observability/, 32 tests, all green):

  • Gating (off by default, explicit 0, enabled-without-endpoint warns and returns, truthy tokens 1/true/yes/on), provider + instrumentation wiring, PsycopgInstrumentor(enable_commenter=False), default application + operator overrides, quiet loggers, idempotency, and protocol-aware exporter selection (grpc default, http/protobuf → http). Plus TraceContextFilter behavior.

Enabled (OTEL_ENABLED=1 + collector reachable):

  • Hit any REST endpoint → a server span with http.route/http.status_code plus child Postgres/Redis/HTTP-client spans; http.server.duration metrics arrive.
  • Trigger an action that enqueues a Celery task → the request span and the task execution span share one trace.
  • Confirm stdout JSON logs (API request path and Celery workers) carry trace_id/span_id/service_name.
  • Set OTEL_ENABLED=1 with OTEL_EXPORTER_OTLP_ENDPOINT unset → exactly one WARNING at boot; the app continues without instrumentation.

Deployment:

  • docker compose -f deployments/cli/community/docker-compose.yml config renders the OTEL_* vars on api/worker/beat-worker only (not migrator).

References

  • Ports the API slice of makeplane/plane-ee#7643
  • Self-hoster guide & sample collector config: docs/otel-api-observability/README.md, docs/otel-api-observability/otel-collector.yaml

🤖 PR created with Claude Code

Summary by CodeRabbit

  • New Features

    • Added optional OpenTelemetry observability for API requests, background tasks, database and external service calls.
    • Added trace and span identifiers to logs when observability is enabled.
    • Added support for exporting traces and metrics through OTLP.
  • Documentation

    • Added setup guidance, supported configuration options, troubleshooting information, and a sample OpenTelemetry Collector configuration.
  • Chores

    • OpenTelemetry is disabled by default and can be enabled through environment settings.

Copilot AI review requested due to automatic review settings July 13, 2026 20:07
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds environment-gated OpenTelemetry tracing, metrics, instrumentation, trace-correlated logging, startup bootstrapping, deployment settings, collector documentation, and unit tests across the API and worker stack.

Changes

OpenTelemetry API observability

Layer / File(s) Summary
Telemetry bootstrap and exporters
apps/api/plane/observability/*, apps/api/requirements/base.txt
Configures OTLP tracing and metrics, resource metadata, protocol-specific exporters, Django/Celery/client instrumentation, logger levels, idempotency, and provider flushing.
Application and worker startup wiring
apps/api/manage.py, apps/api/plane/asgi.py, apps/api/plane/wsgi.py, apps/api/plane/celery.py
Initializes OpenTelemetry before Django or worker wiring and flushes telemetry on Celery worker process shutdown.
Trace-correlated logging
apps/api/plane/observability/logging.py, apps/api/plane/settings/local.py, apps/api/plane/settings/production.py, apps/api/plane/celery.py
Adds trace identifiers, flags, and service metadata to log records and conditionally extends JSON logging.
Runtime environment configuration
apps/api/.env.example, deployments/aio/community/variables.env, deployments/cli/community/variables.env, deployments/cli/community/docker-compose.yml
Adds disabled-by-default OTEL settings and shares them with API, worker, and beat-worker containers.
Collector documentation
docs/otel-api-observability/README.md, docs/otel-api-observability/otel-collector.yaml
Documents instrumentation, environment variables, startup behavior, troubleshooting, and a reference collector with traces, metrics, and log pipelines.
Observability tests
apps/api/plane/tests/unit/observability/*
Tests trace-context enrichment, configuration gating, defaults, exporter protocol selection, instrumentation, logger levels, idempotency, and isolated OTEL state.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant APIStartup
  participant configure_otel
  participant DjangoCelery
  participant OTLPCollector
  APIStartup->>configure_otel: initialize when OTEL is enabled
  configure_otel->>DjangoCelery: instrument requests and tasks
  DjangoCelery->>OTLPCollector: export traces and metrics
  DjangoCelery->>OTLPCollector: emit correlated logs
Loading

Suggested reviewers: dheeru0198, mguptahub, pablohashescobar, copilot

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.32% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and clearly summarizes the main change: adding OpenTelemetry observability to the community API.
Description check ✅ Passed The PR description follows the template well and covers the change, type, testing, and references sections.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/otel-api-observability

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

_instrument_libraries()
_quiet_otel_loggers()

_CONFIGURED = True

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds OpenTelemetry (OTel) observability to the community Django API, including request/task tracing, metrics export, and trace-correlated JSON logs, gated by environment variables for opt-in self-hosting.

Changes:

  • Introduces plane.observability bootstrap (configure_otel/flush_otel) and log correlation filter (TraceContextFilter), wired into WSGI/ASGI/Celery/manage entrypoints.
  • Extends Django and Celery JSON logging to optionally include trace_id/span_id/service_name fields.
  • Adds OTel instrumentation/exporter dependencies plus self-hoster docs and deployment env wiring.

Reviewed changes

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

Show a summary per file
File Description
docs/otel-api-observability/README.md Self-hoster guide for enabling OTLP traces/metrics/log correlation.
docs/otel-api-observability/otel-collector.yaml Reference collector config (OTLP receivers + filelog pipeline).
deployments/cli/community/variables.env Documents new OTEL_* env vars (off by default).
deployments/cli/community/docker-compose.yml Wires OTEL_* env into api/worker/beat-worker services.
deployments/aio/community/variables.env Documents new OTEL_* env vars (off by default).
apps/api/requirements/base.txt Adds required OTel instrumentation/exporter packages.
apps/api/plane/wsgi.py Calls configure_otel() before Django WSGI initialization.
apps/api/plane/asgi.py Calls configure_otel() before Django ASGI initialization.
apps/api/manage.py Calls configure_otel() before executing Django management commands.
apps/api/plane/celery.py Boots OTel for Celery, adds flush hook, and conditionally enriches worker logs.
apps/api/plane/settings/production.py Conditionally extends JSON log formatter + installs TraceContextFilter.
apps/api/plane/settings/local.py Conditionally extends JSON log formatter + installs TraceContextFilter.
apps/api/plane/observability/setup.py New OTel bootstrap (providers, exporters, instrumentation, flush).
apps/api/plane/observability/logging.py New TraceContextFilter for trace-context log enrichment.
apps/api/plane/observability/init.py Initializes the new observability package.
apps/api/plane/tests/unit/observability/conftest.py Fixtures to isolate OTEL env/global state across tests.
apps/api/plane/tests/unit/observability/test_setup.py Unit tests for gating, wiring, idempotency, exporter selection.
apps/api/plane/tests/unit/observability/test_logging.py Unit tests for TraceContextFilter behavior.
apps/api/plane/tests/unit/observability/init.py Initializes the new unit test package.
apps/api/.env.example Documents OTEL_* env vars for local/self-host configuration.

# the handler level (rather than the root logger) is required because
# most plane.* loggers have propagate=False; runtime mutation also wouldn't
# survive Django's dictConfig. Off path leaves the log schema unchanged.
if os.environ.get("OTEL_ENABLED", "0").lower() in ("1", "true", "yes"):
# the handler level (rather than the root logger) is required because
# most plane.* loggers have propagate=False; runtime mutation also wouldn't
# survive Django's dictConfig. Off path leaves the log schema unchanged.
if os.environ.get("OTEL_ENABLED", "0").lower() in ("1", "true", "yes"):
Comment thread apps/api/plane/celery.py
Comment on lines +33 to +36
# Whether to trace-correlate worker logs. Matches the Django LOGGING gate in
# plane/settings/{local,production}.py; the bootstrap in configure_otel() uses
# its own (superset) token check.
_OTEL_LOG_ENABLED = os.environ.get("OTEL_ENABLED", "0").strip().lower() in ("1", "true", "yes")

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (3)
apps/api/plane/settings/local.py (1)

96-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

OTEL logging block is duplicated verbatim across local.py and production.py. Both files contain an identical 10-line block that gates on OTEL_ENABLED and mutates the LOGGING dict. The on omission above shows how duplication invites drift. Extract this into a shared helper (e.g., plane.observability.logging.extend_logging_config(LOGGING)) and call it from both settings modules.

  • apps/api/plane/settings/local.py#L96-L105: Replace inline block with a call to the shared helper.
  • apps/api/plane/settings/production.py#L106-L115: Same — replace inline block with a call to the shared helper.
🤖 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 `@apps/api/plane/settings/local.py` around lines 96 - 105, Extract the
duplicated OTEL logging mutation into a shared helper such as
extend_logging_config in plane.observability.logging, preserving the
OTEL_ENABLED gate and existing formatter, filter, and handler behavior. Replace
the inline blocks with helper calls in apps/api/plane/settings/local.py lines
96-105 and apps/api/plane/settings/production.py lines 106-115.
apps/api/plane/tests/unit/observability/conftest.py (1)

23-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reset _TRACER_PROVIDER and _METER_PROVIDER in the fixture.

The fixture resets _CONFIGURED but not the provider references. After a test that calls configure_otel(), _TRACER_PROVIDER and _METER_PROVIDER are set to real provider objects and persist into subsequent tests. No current test checks these, but adding monkeypatch.setattr for them prevents latent test pollution.

♻️ Proposed fix
 `@pytest.fixture`(autouse=True)
 def isolate_otel_state(monkeypatch):
     from plane.observability import setup as otel_setup

     monkeypatch.setattr(otel_setup, "_CONFIGURED", False, raising=False)
+    monkeypatch.setattr(otel_setup, "_TRACER_PROVIDER", None, raising=False)
+    monkeypatch.setattr(otel_setup, "_METER_PROVIDER", None, raising=False)
🤖 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 `@apps/api/plane/tests/unit/observability/conftest.py` around lines 23 - 37,
Update the isolate_otel_state fixture to also reset observability setup’s
_TRACER_PROVIDER and _METER_PROVIDER globals to their unconfigured state via
monkeypatch, alongside _CONFIGURED, so provider objects cannot persist between
tests.
apps/api/plane/observability/setup.py (1)

134-149: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Unhandled instrumentor exceptions can prevent application startup.

If any instrumentor raises (e.g., version mismatch, missing optional dependency), the exception propagates through configure_otel() to the caller — manage.py, asgi.py, wsgi.py, and celery.py all call it at startup. An observability failure should degrade gracefully rather than take down the application.

♻️ Proposed fix: wrap each instrumentor in try/except
 def _instrument_libraries() -> None:
-    """Patch Django + Celery + downstream client libraries.
-
-    DjangoInstrumentor emits HTTP server spans + http.server.* metrics.
-    CeleryInstrumentor emits a span per task with celery.action, task_name,
-    task_id, state, and propagates traceparent across the queue so a
-    request that enqueues a task is linked to that task's execution span.
-    The others add child spans so latency can be decomposed (SQL query,
-    Redis op, outbound HTTP).
-    """
-    DjangoInstrumentor().instrument()
-    CeleryInstrumentor().instrument()
-    PsycopgInstrumentor().instrument(enable_commenter=False)
-    RedisInstrumentor().instrument()
-    RequestsInstrumentor().instrument()
-    HTTPXClientInstrumentor().instrument()
+    """Patch Django + Celery + downstream client libraries.
+
+    Each instrumentor is wrapped independently so a single failure
+    (version mismatch, missing optional dependency) doesn't prevent
+    the rest from loading or crash the application.
+    """
+    _instrumentors = (
+        (DjangoInstrumentor, {}),
+        (CeleryInstrumentor, {}),
+        (PsycopgInstrumentor, {"enable_commenter": False}),
+        (RedisInstrumentor, {}),
+        (RequestsInstrumentor, {}),
+        (HTTPXClientInstrumentor, {}),
+    )
+    for cls, kwargs in _instrumentors:
+        try:
+            cls().instrument(**kwargs)
+        except Exception as exc:
+            logger.warning("Failed to instrument %s: %s", cls.__name__, exc)
🤖 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 `@apps/api/plane/observability/setup.py` around lines 134 - 149, Update
_instrument_libraries to isolate each instrumentor call with its own try/except
so failures from optional dependencies or incompatible versions are caught and
do not propagate through configure_otel or block application startup. Continue
attempting the remaining instrumentors after an individual failure, and log each
exception using the module’s existing observability logging mechanism.
🤖 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 `@apps/api/plane/celery.py`:
- Line 36: Align the _OTEL_LOG_ENABLED check in celery.py with
configure_otel()’s _is_enabled() by accepting “on” as a truthy value, and apply
the same update to the Django LOGGING gates in local.py and production.py.
Prefer reusing a shared truthiness helper from plane.observability.setup if
available or introduce one to prevent these checks from diverging.

In `@apps/api/plane/observability/setup.py`:
- Around line 36-38: Centralize OTEL flag parsing in an exported
is_otel_enabled() function in setup.py, using _TRUTHY_VALUES so “on” is accepted
consistently. Replace the duplicated environment checks in celery.py’s Celery
log-correlation gate and the Django settings OTEL gating with this function,
preserving existing behavior for other values.

In `@apps/api/plane/settings/local.py`:
- Line 96: Accept “on” as a truthy OTEL_ENABLED value in the settings checks at
apps/api/plane/settings/local.py:96 and
apps/api/plane/settings/production.py:106, preserving the existing accepted
values. Keep docs/otel-api-observability/README.md:27 unchanged because “on”
should remain documented as supported.

In `@deployments/cli/community/docker-compose.yml`:
- Around line 64-73: Update the x-otel-env anchor to pass through
OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG, and OTEL_RESOURCE_ATTRIBUTES using
the corresponding environment-variable defaults, matching the documented
.env.example configuration.

---

Nitpick comments:
In `@apps/api/plane/observability/setup.py`:
- Around line 134-149: Update _instrument_libraries to isolate each instrumentor
call with its own try/except so failures from optional dependencies or
incompatible versions are caught and do not propagate through configure_otel or
block application startup. Continue attempting the remaining instrumentors after
an individual failure, and log each exception using the module’s existing
observability logging mechanism.

In `@apps/api/plane/settings/local.py`:
- Around line 96-105: Extract the duplicated OTEL logging mutation into a shared
helper such as extend_logging_config in plane.observability.logging, preserving
the OTEL_ENABLED gate and existing formatter, filter, and handler behavior.
Replace the inline blocks with helper calls in apps/api/plane/settings/local.py
lines 96-105 and apps/api/plane/settings/production.py lines 106-115.

In `@apps/api/plane/tests/unit/observability/conftest.py`:
- Around line 23-37: Update the isolate_otel_state fixture to also reset
observability setup’s _TRACER_PROVIDER and _METER_PROVIDER globals to their
unconfigured state via monkeypatch, alongside _CONFIGURED, so provider objects
cannot persist between tests.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: fafaa717-4b00-41a8-8a1d-ac68f6e903a7

📥 Commits

Reviewing files that changed from the base of the PR and between d3d3de4 and 38306e3.

📒 Files selected for processing (20)
  • apps/api/.env.example
  • apps/api/manage.py
  • apps/api/plane/asgi.py
  • apps/api/plane/celery.py
  • apps/api/plane/observability/__init__.py
  • apps/api/plane/observability/logging.py
  • apps/api/plane/observability/setup.py
  • apps/api/plane/settings/local.py
  • apps/api/plane/settings/production.py
  • apps/api/plane/tests/unit/observability/__init__.py
  • apps/api/plane/tests/unit/observability/conftest.py
  • apps/api/plane/tests/unit/observability/test_logging.py
  • apps/api/plane/tests/unit/observability/test_setup.py
  • apps/api/plane/wsgi.py
  • apps/api/requirements/base.txt
  • deployments/aio/community/variables.env
  • deployments/cli/community/docker-compose.yml
  • deployments/cli/community/variables.env
  • docs/otel-api-observability/README.md
  • docs/otel-api-observability/otel-collector.yaml

Comment thread apps/api/plane/celery.py
# Whether to trace-correlate worker logs. Matches the Django LOGGING gate in
# plane/settings/{local,production}.py; the bootstrap in configure_otel() uses
# its own (superset) token check.
_OTEL_LOG_ENABLED = os.environ.get("OTEL_ENABLED", "0").strip().lower() in ("1", "true", "yes")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

_OTEL_LOG_ENABLED truthy check is inconsistent with configure_otel()'s _is_enabled().

The celery.py gate accepts ("1", "true", "yes") after .strip().lower(), but configure_otel() treats "on" and "ON" as enabled (per test_truthy_values_for_otel_enabled). Setting OTEL_ENABLED=on would enable tracing and instrumentation but silently skip trace-correlated logging, defeating log-trace correlation. The comment at line 35 acknowledges the mismatch but doesn't resolve it.

🔧 Proposed fix: add `"on"` to the truthy tuple
-_OTEL_LOG_ENABLED = os.environ.get("OTEL_ENABLED", "0").strip().lower() in ("1", "true", "yes")
+_OTEL_LOG_ENABLED = os.environ.get("OTEL_ENABLED", "0").strip().lower() in ("1", "true", "yes", "on")

The same fix should be applied to the Django LOGGING gate in plane/settings/local.py and plane/settings/production.py, which the comment says uses the same check. Consider extracting the truthy check into a shared helper in plane.observability.setup to prevent future drift.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_OTEL_LOG_ENABLED = os.environ.get("OTEL_ENABLED", "0").strip().lower() in ("1", "true", "yes")
_OTEL_LOG_ENABLED = os.environ.get("OTEL_ENABLED", "0").strip().lower() in ("1", "true", "yes", "on")
🤖 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 `@apps/api/plane/celery.py` at line 36, Align the _OTEL_LOG_ENABLED check in
celery.py with configure_otel()’s _is_enabled() by accepting “on” as a truthy
value, and apply the same update to the Django LOGGING gates in local.py and
production.py. Prefer reusing a shared truthiness helper from
plane.observability.setup if available or introduce one to prevent these checks
from diverging.

Comment on lines +36 to +38
# Accepted "on" tokens — kept in sync with the pi/node services so OTEL_ENABLED
# behaves identically across every runtime.
_TRUTHY_VALUES = ("1", "true", "yes", "on")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

_TRUTHY_VALUES includes "on" but the Celery log-correlation gate does not.

_TRUTHY_VALUES accepts "on", but the Celery log-correlation check in apps/api/plane/celery.py uses ("1", "true", "yes") — missing "on". Setting OTEL_ENABLED=on enables telemetry but silently disables trace-correlated worker logs. The celery.py comment acknowledges configure_otel() uses a "superset" token check, but the practical effect is confusing for operators.

Consider exporting a single is_otel_enabled() function from this module and reusing it in celery.py and Django settings to guarantee identical gating everywhere.

🤖 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 `@apps/api/plane/observability/setup.py` around lines 36 - 38, Centralize OTEL
flag parsing in an exported is_otel_enabled() function in setup.py, using
_TRUTHY_VALUES so “on” is accepted consistently. Replace the duplicated
environment checks in celery.py’s Celery log-correlation gate and the Django
settings OTEL gating with this function, preserving existing behavior for other
values.

# the handler level (rather than the root logger) is required because
# most plane.* loggers have propagate=False; runtime mutation also wouldn't
# survive Django's dictConfig. Off path leaves the log schema unchanged.
if os.environ.get("OTEL_ENABLED", "0").lower() in ("1", "true", "yes"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

OTEL_ENABLED=on is documented but not accepted by the code. The README documents on as a valid value, but both settings files omit it from their truthy check — a user setting OTEL_ENABLED=on gets no instrumentation silently.

  • apps/api/plane/settings/local.py#L96: Add "on" to the truthy tuple: ("1", "true", "yes", "on").
  • apps/api/plane/settings/production.py#L106: Same fix — add "on" to the truthy tuple.
  • docs/otel-api-observability/README.md#L27: If on is intentionally not supported, remove it from the docs instead.
📍 Affects 3 files
  • apps/api/plane/settings/local.py#L96-L96 (this comment)
  • apps/api/plane/settings/production.py#L106-L106
  • docs/otel-api-observability/README.md#L27-L27
🤖 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 `@apps/api/plane/settings/local.py` at line 96, Accept “on” as a truthy
OTEL_ENABLED value in the settings checks at apps/api/plane/settings/local.py:96
and apps/api/plane/settings/production.py:106, preserving the existing accepted
values. Keep docs/otel-api-observability/README.md:27 unchanged because “on”
should remain documented as supported.

Comment on lines +64 to +73
# OpenTelemetry APM for the Django API (api, worker, beat-worker).
# Read at container RUNTIME; off by default. See docs/otel-api-observability/README.md.
x-otel-env: &otel-env
OTEL_ENABLED: ${OTEL_ENABLED:-0}
OTEL_EXPORTER_OTLP_ENDPOINT: ${OTEL_EXPORTER_OTLP_ENDPOINT:-}
OTEL_EXPORTER_OTLP_PROTOCOL: ${OTEL_EXPORTER_OTLP_PROTOCOL:-grpc}
OTEL_EXPORTER_OTLP_HEADERS: ${OTEL_EXPORTER_OTLP_HEADERS:-}
OTEL_SERVICE_NAME: ${OTEL_SERVICE_NAME:-plane-api}
OTEL_ENVIRONMENT: ${OTEL_ENVIRONMENT:-}

Copy link
Copy Markdown
Contributor

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

x-otel-env anchor omits sampling and resource-attributes variables.

The .env.example documents OTEL_TRACES_SAMPLER, OTEL_TRACES_SAMPLER_ARG, and OTEL_RESOURCE_ATTRIBUTES, but the x-otel-env anchor doesn't pass them through. Users who set these in their .env file (per the example) will find they have no effect in docker-compose deployments because the vars never reach the container.

♻️ Proposed addition to the anchor
 x-otel-env: &otel-env
   OTEL_ENABLED: ${OTEL_ENABLED:-0}
   OTEL_EXPORTER_OTLP_ENDPOINT: ${OTEL_EXPORTER_OTLP_ENDPOINT:-}
   OTEL_EXPORTER_OTLP_PROTOCOL: ${OTEL_EXPORTER_OTLP_PROTOCOL:-grpc}
   OTEL_EXPORTER_OTLP_HEADERS: ${OTEL_EXPORTER_OTLP_HEADERS:-}
   OTEL_SERVICE_NAME: ${OTEL_SERVICE_NAME:-plane-api}
   OTEL_ENVIRONMENT: ${OTEL_ENVIRONMENT:-}
+  OTEL_TRACES_SAMPLER: ${OTEL_TRACES_SAMPLER:-}
+  OTEL_TRACES_SAMPLER_ARG: ${OTEL_TRACES_SAMPLER_ARG:-}
+  OTEL_RESOURCE_ATTRIBUTES: ${OTEL_RESOURCE_ATTRIBUTES:-}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# OpenTelemetry APM for the Django API (api, worker, beat-worker).
# Read at container RUNTIME; off by default. See docs/otel-api-observability/README.md.
x-otel-env: &otel-env
OTEL_ENABLED: ${OTEL_ENABLED:-0}
OTEL_EXPORTER_OTLP_ENDPOINT: ${OTEL_EXPORTER_OTLP_ENDPOINT:-}
OTEL_EXPORTER_OTLP_PROTOCOL: ${OTEL_EXPORTER_OTLP_PROTOCOL:-grpc}
OTEL_EXPORTER_OTLP_HEADERS: ${OTEL_EXPORTER_OTLP_HEADERS:-}
OTEL_SERVICE_NAME: ${OTEL_SERVICE_NAME:-plane-api}
OTEL_ENVIRONMENT: ${OTEL_ENVIRONMENT:-}
# OpenTelemetry APM for the Django API (api, worker, beat-worker).
# Read at container RUNTIME; off by default. See docs/otel-api-observability/README.md.
x-otel-env: &otel-env
OTEL_ENABLED: ${OTEL_ENABLED:-0}
OTEL_EXPORTER_OTLP_ENDPOINT: ${OTEL_EXPORTER_OTLP_ENDPOINT:-}
OTEL_EXPORTER_OTLP_PROTOCOL: ${OTEL_EXPORTER_OTLP_PROTOCOL:-grpc}
OTEL_EXPORTER_OTLP_HEADERS: ${OTEL_EXPORTER_OTLP_HEADERS:-}
OTEL_SERVICE_NAME: ${OTEL_SERVICE_NAME:-plane-api}
OTEL_ENVIRONMENT: ${OTEL_ENVIRONMENT:-}
OTEL_TRACES_SAMPLER: ${OTEL_TRACES_SAMPLER:-}
OTEL_TRACES_SAMPLER_ARG: ${OTEL_TRACES_SAMPLER_ARG:-}
OTEL_RESOURCE_ATTRIBUTES: ${OTEL_RESOURCE_ATTRIBUTES:-}
🤖 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 `@deployments/cli/community/docker-compose.yml` around lines 64 - 73, Update
the x-otel-env anchor to pass through OTEL_TRACES_SAMPLER,
OTEL_TRACES_SAMPLER_ARG, and OTEL_RESOURCE_ATTRIBUTES using the corresponding
environment-variable defaults, matching the documented .env.example
configuration.

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.

2 participants