Skip to content

Conversation

@cyrille-leclerc
Copy link
Member

@cyrille-leclerc cyrille-leclerc commented Oct 3, 2025

Changes

Ensure the docker container can start in the absence of OTEL_* config variables injected in containers by Docker Compose or K8s.

Rationale:

  • OTEL SDKs don't fail if no config is passed to them thanks to sensible defaults, e.g. no env var like OTEL_EXPORTER_OTLP_ENDPOINT is passed to them
  • It will help for injection of OTEL_* config through the Otel Operator if the container supports absence of those ev vars.

Merge Requirements

For new features contributions, please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs
  • Appropriate Helm chart updates in the helm-charts

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

@cyrille-leclerc cyrille-leclerc marked this pull request as ready for review October 3, 2025 19:33
@cyrille-leclerc cyrille-leclerc requested a review from a team as a code owner October 3, 2025 19:33
@cyrille-leclerc cyrille-leclerc changed the title Support absence of OTEL_* config variables injected by Docker Compose or K8s Support absence of OTEL_* config variables injected in containers Oct 3, 2025
Comment on lines +134 to +135
tracer = trace.get_tracer_provider().get_tracer("recommendation-server")
meter = metrics.get_meter_provider().get_meter("recommendation-server")
Copy link
Member Author

Choose a reason for hiding this comment

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

this parameter is the instrumentation scope nam, not the service name.

@cyrille-leclerc
Copy link
Member Author

@puckpuck FYI a first step heading toward OTLP-protocol agnostic instrumentation.

@julianocosta89
Copy link
Member

julianocosta89 commented Oct 13, 2025

@cyrille-leclerc I don't understand the need of this PR.
Could you elaborate a bit?

If the user forgets to set an env var, this will just mask the error.

I'd rather have the service fail to start with a message saying Env Var OTEL_XYZ is required, than the service starting and not working as expected.

@cyrille-leclerc
Copy link
Member Author

cyrille-leclerc commented Oct 13, 2025

Initially, I wanted this behaviour because the when combining the otel-kube-stack with the otel-demo components within the same chart, the demo components first starts without being injected the OTEL_* env variables and I didn't want some of the components to report failure to start when some other did start with sensible defaults.

Then I did somecresearch and found that most apm agent don't fail the process when misconfigured which made a lot of sense to me and I came to the conclusion that the demo should exhibit the same behaviour: if monitoring config is missing, don't fail the service, just report the failure in a non blocking manner.

Does it clarify?

I'd rather have the service fail to start with a message saying Env Var OTEL_XYZ is required, than the service starting and not working as expected.

My understanding is that OTel SDKs gracefully degrade if not provided configuration, like setting the export to the OTLP endpoint on http://localhost:4318 service name to unknown_service

@julianocosta89
Copy link
Member

the demo should exhibit the same behaviour: if monitoring config is missing, don't fail the service, just report the failure in a non blocking manner.

I'd love to have more opinions on this. (@rogercoll, @puckpuck and @mviitane, wdyt?!)

To me, the main goal of the OTel Demo is to demonstrate OTel. If a service starts successfully, but without OTel configured, then the service is not showcasing what was supposed to.

@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants