Skip to content

feat(ocw_studio,ovs): export OTel traces from two more Django apps - #5498

Open
blarghmatey wants to merge 1 commit into
mainfrom
tmacey/otel-ocw-studio-ovs-stacks
Open

feat(ocw_studio,ovs): export OTel traces from two more Django apps#5498
blarghmatey wants to merge 1 commit into
mainfrom
tmacey/otel-ocw-studio-ovs-stacks

Conversation

@blarghmatey

Copy link
Copy Markdown
Member

What are the relevant tickets?

Part of the OpenTelemetry APM coverage epic (tk-extend-otel-tracing-to-the-uninstrumented-apps-m-41729c). Pairs with:

⚠️ Draft: land this after both app images deploy

The env vars here are inert without the app-side changes — ocw-studio has no observability app installed at all today, and OVS has one but no instrumentors. Landing this first buys config that reads as working and does nothing.

Description (What does it do?)

Sets the OTLP endpoint and resource attributes for ocw_studio and odl_video_service in QA and Production, so the tracing the two app PRs turn on has somewhere to go.

What each variable is for, and what is deliberately absent

The six existing OTel stacks carry nine variables each. Three of those nine are read by nothing, so the new adopters get five rather than inheriting them:

Variable Who reads it
OPENTELEMETRY_ENDPOINT a Django setting the app reads; handed to OTLPSpanExporter verbatim, hence the full /v1/traces path rather than a base URL
OTEL_SERVICE_NAME read straight from os.environ by the library
OTEL_RESOURCE_ATTRIBUTES the SDK's OTELResourceDetector
OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT the SDK, when it builds SpanLimits
OTEL_TRACES_SAMPLER TracerProvider's default sampler

Not set, and why:

  • OTEL_PROPAGATORS — the library calls set_global_textmap() with an explicit CompositePropagator, which overrides it.
  • OTEL_EXPORTER_OTLP_PROTOCOL — consumed by the opentelemetry-instrument agent, not by the http exporter the library constructs directly.
  • OTEL_METRIC_EXPORT_INTERVAL — there is no MeterProvider until feat(observability): add a MeterProvider so RED metrics stop inheriting sampling bias ol-django#553 ships, and no metrics endpoint until the task that owns OTEL_EXPORTER_OTLP_ENDPOINT runs. Both belong to that change, together.
  • deployment.environment — the library passes it to Resource.create() from settings.ENVIRONMENT, and Resource.create merges explicit attributes over env-detected ones, so an OTEL_RESOURCE_ATTRIBUTES value for that key never reaches a span. Verified in QA Tempo: learn-webapp, learn-ai-webapp and mitxonline-webapp all report deployment.environment="rc" (their app-level ENVIRONMENT) despite every one of those stacks setting "qa". Filed separately rather than copied forward.

parentbased_always_on, not the 0.25 ratio the older stacks use

Alloy already tail-samples — keep errors, keep >5000ms, 15% of the rest (substructure/aws/eks/grafana.py). Head sampling in front of that multiplies with it instead of composing: a trace is dropped by the coin flip before any policy can look at whether it errored. Same reasoning as #5487 (witan) and #5471 (mit-learn-nextjs); the house pattern is stated in components/services/apisix.py, where the gateway runs always_on.

Narrower in practice than it looks: APISIX propagates a sampled traceparent and parentbased_ honours it, so anything arriving through the gateway was already at 100%. The ratio would only ever have applied to traces these apps root themselves — celery tasks and cron — which is exactly where full capture is worth most.

CI is deliberately dark

setup_grafana returns early for ci, so no Alloy runs there and grafana-k8s-monitoring-alloy-receiver does not resolve. Setting the endpoint uniformly is not free: it buys a connection failure per span batch, forever, in the environment nobody watches.

ocw_studio gets the block only in Pulumi.{QA,Production}.yaml. OVS has no vars: config block — every non-secret env var is literal in its program — so it gets an explicit env_suffix guard there instead.

How can this be tested?

pulumi preview:

  • odl_video_service QA: 3 to update (webapp, celery worker, celery beat), 133 unchanged. Diff shows the five vars added and OTEL_RESOURCE_ATTRIBUTES extended with the existing ol.mit.edu/* labels.
  • odl_video_service CI: no OTel env in the diff, confirming the guard. (One unrelated pre-existing liveness-probe drift.)
  • ocw_studio QA: 6 to update, 1 replace. The replace is the pre-deploy Job, which is immutable, so any env change replaces it.

After deploy, confirm ocw-studio-webapp and ovs-webapp appear under resource.service.name in Tempo.

Additional Context

micromasters and xpro are the other two apps in this epic. Their stack configs are deliberately not included: both repos currently have an unsatisfiable uv.lock on master, so neither can add the instrumentors this depends on. Tracked separately.

Both apps have been running in applications-{qa,production} with no tracing at
all. Once mitodl/ocw-studio#TBD and mitodl/odl-video-service#TBD ship, the only
thing still missing is the endpoint, which is what this sets.

★ ORDERING: LAND THIS AFTER THOSE IMAGES ARE DEPLOYED. ★
The env vars are inert without the app-side change (ocw-studio has no
observability app installed at all; OVS has one but no instrumentors), so
landing this first buys config that reads as working and does nothing.

── WHAT EACH VARIABLE IS FOR, AND WHAT IS DELIBERATELY ABSENT ──
The six existing OTel stacks carry nine variables each. Three of those nine are
read by nothing, so the new adopters get five rather than inheriting them:

  OPENTELEMETRY_ENDPOINT           a Django setting the app reads; the library
                                   hands it to OTLPSpanExporter verbatim, hence
                                   the full /v1/traces path rather than a base URL
  OTEL_SERVICE_NAME                read straight from os.environ by the library
  OTEL_RESOURCE_ATTRIBUTES         read by the SDK's OTELResourceDetector
  OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT  read by the SDK when it builds SpanLimits
  OTEL_TRACES_SAMPLER              read by TracerProvider's default sampler

  NOT SET: OTEL_PROPAGATORS -- the library calls set_global_textmap() with an
  explicit CompositePropagator, which overrides it.
  NOT SET: OTEL_EXPORTER_OTLP_PROTOCOL -- consumed by the opentelemetry-instrument
  agent, not by the http exporter the library constructs directly.
  NOT SET: OTEL_METRIC_EXPORT_INTERVAL -- there is no MeterProvider until
  mitodl/ol-django#553 ships, and no metrics endpoint until the task that owns
  OTEL_EXPORTER_OTLP_ENDPOINT runs. Both belong to that change, together.

Also NOT SET: deployment.environment. The library passes it to Resource.create()
from settings.ENVIRONMENT, and Resource.create merges explicit attributes over
env-detected ones, so an OTEL_RESOURCE_ATTRIBUTES value for that key never
reaches a span. Verified in QA Tempo: learn-webapp, learn-ai-webapp and
mitxonline-webapp all report deployment.environment="rc" (their app-level
ENVIRONMENT) despite every one of those stacks setting "qa". Filed separately
rather than copied forward.

── parentbased_always_on, NOT the 0.25 ratio the older stacks use ──
Alloy already tail-samples (keep errors, keep >5000ms, 15% of the rest --
substructure/aws/eks/grafana.py). Head sampling in front of that multiplies with
it instead of composing: a trace is dropped by the coin flip before any policy
can look at whether it errored. This is the same reasoning as #5487 (witan) and
#5471 (mit-learn-nextjs); the house pattern is stated in
components/services/apisix.py, where the gateway runs always_on.

Narrower in practice than it looks: APISIX propagates a sampled traceparent and
`parentbased_` honours it, so anything arriving through the gateway was already
at 100%. The ratio would only ever have applied to traces these apps root
themselves -- celery tasks and cron -- which is exactly where full capture is
worth most.

── CI IS DELIBERATELY DARK ──
setup_grafana returns early for `ci`, so no Alloy runs there and
grafana-k8s-monitoring-alloy-receiver does not resolve. Setting the endpoint
uniformly is not free: it buys a connection failure per span batch, forever, in
the environment nobody watches. ocw_studio gets the block only in
Pulumi.{QA,Production}.yaml; OVS, which has no `vars:` config block and builds
every env var in its program, gets an explicit env_suffix guard instead.

── PREVIEW ──
odl_video_service QA: 3 to update (webapp, celery worker, celery beat), 133
unchanged. CI: no OTel env in the diff, confirming the guard.
ocw_studio QA: 6 to update, 1 replace -- the replace is the pre-deploy Job,
which is immutable, so any env change replaces it.

micromasters and xpro are the other two apps in this epic. Their stack configs
are deliberately not included: both repos currently have an unsatisfiable
uv.lock on master, so neither can add the instrumentors this depends on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E5y3R8zEkr2A26X3Y43aBi
@blarghmatey
blarghmatey marked this pull request as ready for review August 18, 2026 19:39
Copilot AI balanced review requested due to automatic review settings August 18, 2026 19:39

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

Configures QA and production trace export for OCW Studio and ODL Video Service, pending deployment of their app-side instrumentation.

Changes:

  • Adds OTLP endpoint, service identity, resource attributes, limits, and sampling configuration.
  • Excludes CI and appends Kubernetes metadata.

Reviewed changes

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

File Description
src/ol_infrastructure/applications/odl_video_service/__main__.py Adds environment-gated OTel configuration.
src/ol_infrastructure/applications/ocw_studio/Pulumi.QA.yaml Enables tracing configuration in QA.
src/ol_infrastructure/applications/ocw_studio/Pulumi.Production.yaml Enables tracing configuration in production.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

OCW_STUDIO_LOG_LEVEL: INFO
OCW_STUDIO_SUPPORT_EMAIL: 'ocw-studio-support@mit.edu'
OPENTELEMETRY_ENDPOINT: "http://grafana-k8s-monitoring-alloy-receiver.grafana.svc.cluster.local:4318/v1/traces"
OTEL_RESOURCE_ATTRIBUTES: "service.namespace=ocw-studio,service.instance.id=$(KUBERNETES_POD_NAME)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The ocw_studio configuration is missing the standard deployment.environment OpenTelemetry attribute, which will cause its traces to be excluded from standard observability dashboards.
Severity: MEDIUM

Suggested Fix

Add the deployment.environment attribute to the OTEL_RESOURCE_ATTRIBUTES configuration in the ocw_studio Pulumi YAML files (Pulumi.Production.yaml, Pulumi.QA.yaml) to align with other applications and standard OpenTelemetry conventions. For example: OTEL_RESOURCE_ATTRIBUTES: "service.namespace=ocw-studio,deployment.environment=production,service.instance.id=$(KUBERNETES_POD_NAME)".

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/ol_infrastructure/applications/ocw_studio/Pulumi.Production.yaml#L44

Potential issue: The `ocw_studio` application's OpenTelemetry configuration lacks the
standard `deployment.environment` resource attribute. While a helper function,
`merge_otel_resource_attributes`, adds a Kubernetes label-derived attribute named
`ol.mit.edu/environment`, this does not conform to the OTel semantic convention. As a
result, observability dashboards and queries that filter or group by the standard
`deployment.environment` attribute will fail to capture traces from `ocw_studio`. This
creates an observability gap, making it difficult to monitor the application by
environment alongside other services that correctly use the standard attribute.

Also affects:

  • src/ol_infrastructure/applications/ocw_studio/Pulumi.QA.yaml:48~48

Did we get this right? 👍 / 👎 to inform future reviews.

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