Skip to content

[SDK] File configuration: declarative resource detection types#4148

Merged
marcalff merged 2 commits into
open-telemetry:mainfrom
thc1006:feat/declarative-resource-detectors-3916
Jun 13, 2026
Merged

[SDK] File configuration: declarative resource detection types#4148
marcalff merged 2 commits into
open-telemetry:mainfrom
thc1006:feat/declarative-resource-detectors-3916

Conversation

@thc1006

@thc1006 thc1006 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #3916.

Adds the declarative configuration model classes for resource detection per the
file configuration spec, so that the detection/development block under
resource parses into typed configs the SDK can dispatch on. SDK wiring
to actual ResourceDetector implementations is out of scope here and stays
behind the existing resource detectors not supported warning (#3548).

What changed

New header files under sdk/include/opentelemetry/sdk/configuration/:

  • resource_detector_configuration.h — abstract base with
    Accept(visitor) const = 0
  • resource_detector_configuration_visitor.h — visitor with one
    Visit* per leaf type (Container/Host/Process/Service/Extension)
  • container_resource_detector_configuration.h
  • host_resource_detector_configuration.h
  • process_resource_detector_configuration.h
  • service_resource_detector_configuration.h
  • extension_resource_detector_configuration.h — fallback for
    non-standard detector names, follows the existing
    ExtensionSamplerConfiguration pattern (stores name, raw DocumentNode,
    depth)
  • resource_detection_configuration.h — wraps optional
    IncludeExcludeConfiguration attributes plus a vector of detector configs

ResourceConfiguration: field renamed from detectors (old shape:
IncludeExclude of detector name strings) to detection
(std::unique_ptr<ResourceDetectionConfiguration>) so the model
matches the spec.

ConfigurationParser: parser methods for each new type. The detector
dispatcher reads the single map key per list entry (e.g. - container:) and
falls through to the Extension parser for any other name. The top-level
parser now looks up detection/development instead of detectors.

SdkBuilder: updated reference to the renamed field; existing warning
preserved.

Tests

sdk/test/configuration/yaml_resource_test.cc rewritten for the new shape
(17 cases total, all green). Type discrimination uses a TestDetectorVisitor
that implements the visitor interface, so the suite compiles under
-fno-rtti (Bazel nortti CI).

  • empty resource / empty attributes / attributes list / both
  • empty detection/development: block
  • detection with attributes (included + excluded)
  • each of container / host / process / service alone
  • multiple detectors in one list
  • extension detector with an arbitrary name
  • combined attributes + detectors

The 5 old tests parsing the deprecated detectors.included/excluded shape
were removed (no longer matches the spec and the field no longer exists).

Verification

Built with WITH_CONFIGURATION=ON (Release, Ninja). All three YAML test
binaries pass with no regressions:

  • yaml_test: 50/50
  • yaml_resource_test: 17/17
  • yaml_propagator_test: 9/9

Also built with -DCMAKE_CXX_FLAGS=-fno-rtti (reproducing the Bazel
nortti CI environment): yaml_resource_test 17/17.

End-to-end load of examples/configuration/kitchen-sink.yaml via
example_yaml --yaml ... --test reports MODEL PARSED, confirming the
full spec-shaped block (detection/development with nested attributes
and all 4 standard detectors) parses without error.

@thc1006 thc1006 requested a review from a team as a code owner June 11, 2026 02:16
@thc1006 thc1006 force-pushed the feat/declarative-resource-detectors-3916 branch 2 times, most recently from c31b9b6 to 7f06fd3 Compare June 11, 2026 02:17
@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.83%. Comparing base (e988943) to head (a8cecf9).

Files with missing lines Patch % Lines
sdk/src/configuration/configuration_parser.cc 91.08% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4148      +/-   ##
==========================================
+ Coverage   82.80%   82.83%   +0.04%     
==========================================
  Files         399      406       +7     
  Lines       16845    16913      +68     
==========================================
+ Hits        13946    14009      +63     
- Misses       2899     2904       +5     
Files with missing lines Coverage Δ
...ration/container_resource_detector_configuration.h 100.00% <100.00%> (ø)
...ration/extension_resource_detector_configuration.h 100.00% <100.00%> (ø)
...nfiguration/host_resource_detector_configuration.h 100.00% <100.00%> (ø)
...guration/process_resource_detector_configuration.h 100.00% <100.00%> (ø)
...dk/configuration/resource_detector_configuration.h 100.00% <100.00%> (ø)
...guration/resource_detector_configuration_visitor.h 100.00% <100.00%> (ø)
...guration/service_resource_detector_configuration.h 100.00% <100.00%> (ø)
sdk/src/configuration/configuration_parser.cc 77.57% <91.08%> (+0.54%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thc1006 thc1006 force-pushed the feat/declarative-resource-detectors-3916 branch from 7f06fd3 to 04ea4c7 Compare June 11, 2026 03:22

@marcalff marcalff left a comment

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.

Thanks for the PR.

There are multiple competing PRs to implement this, so seeing duplicate efforts is a bit sad.

Still, this PR is the most convincing, and the most complete.

Please see renaming comments, and make sure CI is clean.

It should be ready to merge after the cleanup.

@marcalff marcalff self-assigned this Jun 12, 2026
Add declarative configuration model for resource detection per file
configuration spec:

* ResourceDetectorConfiguration (abstract base + visitor)
* ContainerResourceDetectorConfiguration
* HostResourceDetectorConfiguration
* ProcessResourceDetectorConfiguration
* ServiceResourceDetectorConfiguration
* ExtensionResourceDetectorConfiguration (for non-standard detector
  names, follows ExtensionSamplerConfiguration pattern)
* ResourceDetectionConfiguration (wraps attributes
  IncludeExcludeConfiguration + vector of detector configs)

ResourceConfiguration field renamed from "detectors" (old shape:
include/exclude lists of detector names) to "detection" (new shape:
typed detector list under detection/development YAML key).

The parser dispatches on the single map key of each detector entry
(container/host/process/service map to typed configs; any other name
falls through to Extension*). Tests use the Accept(visitor) pattern
so the suite compiles under -fno-rtti (Bazel nortti CI).

The sdk_builder Resource path still emits the existing
"resource detectors not supported" warning (open-telemetry#3548) since wiring
detectors to actual ResourceDetector implementations is out of scope
for this PR.

Fixes open-telemetry#3916

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006 thc1006 force-pushed the feat/declarative-resource-detectors-3916 branch from 04ea4c7 to 1ced912 Compare June 13, 2026 09:16

@marcalff marcalff left a comment

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.

LGTM, thanks for the contribution.

@marcalff marcalff merged commit c14bf11 into open-telemetry:main Jun 13, 2026
71 checks passed
@thc1006 thc1006 deleted the feat/declarative-resource-detectors-3916 branch June 13, 2026 16:17
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.

[CONFIGURATION] File configuration - resource detectors

2 participants