Skip to content

feat(config): wire container resource detector via entry point loading#5004

Open
MikeGoldsmith wants to merge 19 commits intoopen-telemetry:mainfrom
MikeGoldsmith:mike/config-resource-detector-container
Open

feat(config): wire container resource detector via entry point loading#5004
MikeGoldsmith wants to merge 19 commits intoopen-telemetry:mainfrom
MikeGoldsmith:mike/config-resource-detector-container

Conversation

@MikeGoldsmith
Copy link
Copy Markdown
Member

@MikeGoldsmith MikeGoldsmith commented Mar 20, 2026

Description

Implements container resource detector support in create_resource() for the declarative configuration pipeline, as part of the ongoing work tracked in the following PRs:

What's included

  • Wires detection_development.detectors[].container in the config to ContainerResourceDetector from the contrib package opentelemetry-resource-detector-containerid
  • Uses a lazy import rather than a hard dependency: if the contrib package is installed it is used transparently; if absent a warning is logged with an actionable install instruction
  • Respects the detection_development.attributes include/exclude glob filter (shared infrastructure from feat(config): add resource and propagator creation from declarative config #4979)
  • Explicit config attributes always take priority over detector-produced values
  • 6 new tests covering detector opt-in, warning on missing package, contrib package usage (mocked), and explicit attribute override

Design note: why a lazy import rather than a core implementation?

Unlike the process, host, and service detectors, container detection (container.id via /proc/self/cgroup) is not implemented in the Python core SDK — it lives in the opentelemetry-resource-detector-containerid contrib package. This is consistent with other SDKs:

SDK Container detector
Java opentelemetry-java-instrumentation (contrib)
JS No implementation — silently skipped
PHP Contrib package
Python Contrib package (opentelemetry-resource-detector-containerid)
C++ Core (recent addition under a preview flag)

Rather than adding a hard install_requires dependency on a contrib package from the core SDK, or silently skipping (JS approach), this PR uses Python's idiomatic lazy import pattern — the same technique used for optional extras elsewhere in the SDK (e.g. gRPC in OTLP exporters).

Open question for reviewers: Given that C++ has moved container detection into core, should Python do the same? That would remove the contrib dependency entirely and make the behaviour consistent with process/host/service. Happy to implement that in a follow-up if the community prefers it.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • 6 unit tests in tests/_configuration/test_resource.py
  • Includes tests for missing package (warning), contrib package usage (mocked), and explicit override

Does This PR Require a Contrib Repo Change?

  • Yes.
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Assisted-by: Claude Sonnet 4.6

Implements create_resource() and create_propagator()/configure_propagator()
for the declarative file configuration. Resource creation does not read
OTEL_RESOURCE_ATTRIBUTES or run any detectors (matches Java/JS SDK behavior).
Propagator configuration always calls set_global_textmap to override Python's
default tracecontext+baggage, setting a noop CompositePropagator when no
propagator is configured.

Assisted-by: Claude Sonnet 4.6
Assisted-by: Claude Sonnet 4.6
- _resource.py: refactor _coerce_attribute_value to dispatch table to
  avoid too-many-return-statements; fix short variable names k/v ->
  attr_key/attr_val; fix return type of _sdk_default_attributes to
  dict[str, str] to satisfy pyright
- _propagator.py: rename short variable names e -> exc, p -> propagator
- test_resource.py: move imports to top level; split TestCreateResource
  (25 methods) into three focused classes to satisfy too-many-public-methods
- test_propagator.py: add pylint disable for protected-access

Assisted-by: Claude Sonnet 4.6
- replace _sdk_default_attributes() with _DEFAULT_RESOURCE from resources module
- move _coerce_bool into dispatch tables for both scalar and array bool types,
  fixing a bug where bool_array with string values like "false" would coerce
  incorrectly via plain bool() (non-empty string -> True)
- add test for bool_array with string values to cover the bug

Assisted-by: Claude Sonnet 4.6
…erge

- collapse _SCALAR_COERCIONS and _ARRAY_COERCIONS into a single _COERCIONS
  dict using an _array() factory, reducing _coerce_attribute_value to two lines
- process attributes_list before attributes so explicit attributes naturally
  overwrite list entries without needing an explicit guard

Assisted-by: Claude Sonnet 4.6
Assisted-by: Claude Sonnet 4.6
Adds _run_detectors() stub and _filter_attributes() to create_resource(),
providing the shared scaffolding for detector PRs to build on. Detectors
are opt-in: nothing runs unless explicitly listed under
detection_development.detectors in the config. The include/exclude
attribute filter mirrors other SDK behaviour.

Assisted-by: Claude Sonnet 4.6
Merges service.name=unknown_service into base before running detectors,
so detectors (e.g. service) can override it. Previously it was added to
config_attrs and merged last, which would have silently overridden any
detector-provided service.name.

Assisted-by: Claude Sonnet 4.6
MikeGoldsmith added a commit to MikeGoldsmith/opentelemetry-python that referenced this pull request Mar 20, 2026
When `resource.detection_development.detectors[].container` is set in
the config file, attempt to use `ContainerResourceDetector` from the
`opentelemetry-resource-detector-containerid` contrib package. Unlike
the process, host, and service detectors which are implemented in the
core SDK, container detection is not available in core. Rather than
adding a hard dependency on the contrib package, we use a lazy import:
if the package is installed it is used transparently; if absent a
warning is logged with an actionable install instruction.

This mirrors the approach taken by other SDKs: JS explicitly skips
container detection when no implementation is available, and PHP also
defers container detection to a contrib package.

See: open-telemetry/opentelemetry-configuration#570

Assisted-by: Claude Sonnet 4.6
@MikeGoldsmith MikeGoldsmith force-pushed the mike/config-resource-detector-container branch from be1fb24 to 327e168 Compare March 20, 2026 19:46
@MikeGoldsmith MikeGoldsmith marked this pull request as ready for review March 29, 2026 16:36
@MikeGoldsmith MikeGoldsmith requested a review from a team as a code owner March 29, 2026 16:36
"""
if detector_config.container is not None:
# The container detector is not part of the core SDK. It is provided
# by the opentelemetry-resource-detector-containerid contrib package.
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.

I wondered about this when looking at the other PRs. I think the warning approach is a reasonable way about this.

Assisted-by: Claude Sonnet 4.6
Matches the env-var config counterpart (OTEL_EXPERIMENTAL_RESOURCE_DETECTORS)
and avoids a hard import dependency on the contrib package. Tests updated
to mock entry_points instead of sys.modules.

Assisted-by: Claude Sonnet 4.6
@MikeGoldsmith MikeGoldsmith changed the title feat(config): wire container resource detector via lazy contrib import feat(config): wire container resource detector via entry point loading Apr 1, 2026
@MikeGoldsmith MikeGoldsmith moved this to Approved PRs in Python PR digest Apr 1, 2026
"installed; install it to enable container detection"
)
else:
detected_attrs.update(ep.load()().detect().attributes)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like the env var version uses get_aggregated_resources(). Maybe this should too for consistency -- wdyt?

ep = next(
iter(
entry_points(
group="opentelemetry_resource_detector", name="container"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are the resource detector names standardized for declarative config? I feel like this block of code could be used to generically load any of the detector_config.* detectors. It would also work for out-of-source detectors generically.

Although maybe it's better as a follow up, wdyt?

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

Labels

None yet

Projects

Status: Approved PRs

Development

Successfully merging this pull request may close these issues.

5 participants