Skip to content

Use declarative config throughout dynamic-control - #3013

Open
zeitlinger wants to merge 11 commits into
open-telemetry:mainfrom
zeitlinger:agent/dynamic-control-declarative-config
Open

zeitlinger wants to merge 11 commits into
open-telemetry:mainfrom
zeitlinger:agent/dynamic-control-declarative-config

Conversation

@zeitlinger

@zeitlinger zeitlinger commented Jul 23, 2026

Copy link
Copy Markdown
Member

Blocked by open-telemetry/opentelemetry-java#7832

Summary

  • use DeclarativeConfigProperties as the configuration interface for dynamic-control policy sources
  • remove the declarative-to-ConfigProperties adapter from the sampler component
  • bridge legacy auto-configuration to declarative properties only at the compatibility boundary
  • preserve the existing flat OpAMP properties until OpAMP has a declarative configuration schema

Rationale

This follows the design discussion in #2989, while keeping ConfigProperties out of the dynamic-control internals as the common configuration interface.

We deliberately do not use the deprecated DeclarativeConfigPropertiesBridgeBuilder. Its removal is slated for the upcoming 3.0 release, so this is more than a cleanup concern: using it would add a dependency on an API that will soon disappear and would keep ConfigProperties as the unifying interface.

Instead, the declarative component path passes DeclarativeConfigProperties directly through dynamic-control:

  • TelemetryPolicySamplerComponentProvider receives declarative properties.
  • PolicyInit and PolicyInitConfig consume declarative properties directly.
  • SourceKind and OpampPolicyProvider use declarative properties internally.
  • No ConfigProperties adapter is created for the declarative component path.

For the legacy auto-configuration path, we still need to support the existing file-based configuration:

otel.java.experimental.telemetry.policy.init.yaml
otel.java.experimental.telemetry.policy.init.json

Those properties point to legacy YAML/JSON files, so readFromConfigProperties remains an intentional compatibility boundary. It is not used as the internal configuration abstraction, and it cannot be replaced by the declarative bridge without changing the legacy property names and file-loading behavior.

At the point where legacy auto-configuration activates policy sources, we use the new DeclarativeConfigBridge with an empty component prefix to expose the general flat properties as DeclarativeConfigProperties. This is important for OpAMP: OpAMP does not yet have a declarative schema, and its endpoint, service identity, resource attributes, and headers must continue to come from properties. The small boundary compatibility view retains the map-shaped legacy resource and header properties that the scalar bridge cannot represent. The dynamic-control internals still receive only declarative properties.

This also avoids the deprecated ConfigPropertiesBackedConfigProvider, whose instrumentation-scoped view would not expose the general OpAMP properties.

This supports the three intended execution paths:

  1. No declarative configuration: retain the existing auto-configuration fallback and its OpAMP property support.
  2. Declarative configuration is active: read the policy configuration purely from declarative properties.
  3. Legacy auto-configuration with a config provider present: use the general-property compatibility bridge at the boundary, then continue internally with declarative properties.

Optional structured lookups use the nullable getStructured form where the configuration block may be absent. get is used only for properties whose presence is guaranteed or where the implementation supplies the required empty default.

Testing

  • ./gradlew --no-daemon --no-parallel :dynamic-control:spotlessApply :dynamic-control:check
  • mise run lint

Fixes #2989

@zeitlinger zeitlinger changed the title Keep dynamic-control declarative config isolated Use declarative config throughout dynamic-control Jul 23, 2026
@zeitlinger
zeitlinger marked this pull request as ready for review July 23, 2026 11:54
@zeitlinger
zeitlinger requested a review from a team as a code owner July 23, 2026 11:54
Copilot AI review requested due to automatic review settings July 23, 2026 11:54

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

This PR migrates dynamic-control internals to use DeclarativeConfigProperties as the common configuration interface, keeping ConfigProperties limited to the legacy auto-configuration compatibility boundary.

Changes:

  • Switched policy source/provider wiring (PolicyInit, SourceKind, OpampPolicyProvider) to consume DeclarativeConfigProperties directly.
  • Removed the declarative-to-ConfigProperties adapter/fallback layer from the declarative sampler component path.
  • Bridged legacy auto-configuration to declarative properties at activation time via ConfigPropertiesBackedConfigProvider, and updated tests accordingly.

Reviewed changes

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

Show a summary per file
File Description
dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/TelemetryPolicySamplerComponentProvider.java Removes ConfigProperties bridging/fallback from the declarative sampler component path.
dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/OpampPolicyProvider.java Converts OpAMP provider configuration input to DeclarativeConfigProperties and updates header/resource attribute resolution.
dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/registry/PolicyInit.java Bridges legacy auto-config properties to declarative properties when activating sources.
dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/registry/PolicyInitConfig.java Refactors declarative parsing reuse and simplifies legacy YAML/JSON path selection/reading.
dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/source/SourceKind.java Updates provider creation signature to accept DeclarativeConfigProperties.
dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/OpampPolicyProviderTest.java Updates unit tests to reflect declarative properties for OpAMP config/header/resource attributes.
dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/registry/PolicyInitTest.java Updates tests for the new initFromDeclarativeConfig signature.
dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/source/SourceKindTest.java Updates source kind/provider tests to use declarative properties.
Comments suppressed due to low confidence (1)

dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/OpampPolicyProvider.java:360

  • getServiceEnvironment assumes otel.resource.attributes is present. If properties.get(RESOURCE_ATTRIBUTES) returns null, this will throw; previous behavior returned null when resource attributes were missing. Default to DeclarativeConfigProperties.empty() to preserve that behavior.
    DeclarativeConfigProperties resourceAttributes = properties.get(RESOURCE_ATTRIBUTES);
    String semconvEnvironment = resourceAttributes.getString(DEPLOYMENT_ENVIRONMENT_NAME);

@jackshirazi

Copy link
Copy Markdown
Contributor

I don't think this correctly handles the opamp provider. There is (as yet) no declarative config for opamp, so it needs to continue to be supported via properties. I'll have time to look at this next week

@zeitlinger
zeitlinger force-pushed the agent/dynamic-control-declarative-config branch from 3bf4c64 to 337dc08 Compare July 23, 2026 13:40
@zeitlinger

Copy link
Copy Markdown
Member Author

Thanks, I updated this to keep OpAMP on the legacy properties path until it has a declarative schema.

The legacy auto-configuration path still exposes the general flat configuration through DeclarativeConfigBridge.createComponentProperties(config, ""), so the OpAMP endpoint and service identity continue to be read from ConfigProperties. Resource attributes retain the small compatibility view needed by the provider.

Headers need a separate treatment: the declarative bridge documents that getPropertyKeys() is always empty for system-property-backed configuration. Enumerating headers.getPropertyKeys() would therefore silently drop configured legacy OpAMP headers. The legacy boundary now captures config.getMap("otel.experimental.opamp.headers") and passes that immutable map into OpampPolicyProvider; the provider no longer enumerates declarative property keys. The declarative component path passes an empty header map because OpAMP has no declarative configuration yet.

This keeps ConfigProperties confined to the legacy compatibility boundary, avoids the deprecated bridge, and preserves the existing legacy OpAMP header behavior. The change is in 53afb3f5, with the stacked diff shown in the PR comparison.

@zeitlinger
zeitlinger marked this pull request as draft July 23, 2026 15:48
@zeitlinger
zeitlinger marked this pull request as ready for review July 23, 2026 16:44
@zeitlinger

Copy link
Copy Markdown
Member Author

the declarative bridge documents that getPropertyKeys() is always empty for system-property-backed configuration

we could extend in different ways to bridge this gap - but this hasn't been needed in the Javaagent for which the bridge has been developed

I think I'll try it out - just to see what it would look like (as a separate PR)

@zeitlinger

zeitlinger commented Jul 24, 2026

Copy link
Copy Markdown
Member Author

I think I'll try it out - just to see what it would look like (as a separate PR)

To do that I'd have to know where in the declarative config schema "otel.experimental.opamp.headers" and "otel.resource.attributes" should be located. If they are not there at all, then the bridge would be more confusing than helpful in making it look like it could be found in declarative config.

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger
zeitlinger force-pushed the agent/dynamic-control-declarative-config branch from ed1aaa1 to 304ce21 Compare July 24, 2026 16:12
@zeitlinger

Copy link
Copy Markdown
Member Author

This PR targets main now.

@jackshirazi

Copy link
Copy Markdown
Contributor

@zeitlinger okay for me to commit changes to the branch or would you prefer to see a diff and apply that yourself?

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger

Copy link
Copy Markdown
Member Author

@zeitlinger okay for me to commit changes to the branch or would you prefer to see a diff and apply that yourself?

the PR already targets main - what do you mean?

@zeitlinger

Copy link
Copy Markdown
Member Author

@zeitlinger okay for me to commit changes to the branch or would you prefer to see a diff and apply that yourself?

the PR already targets main - what do you mean?

now I get it - yes, feel free to do any changes or create a new PR - whatever works best for you

@jackshirazi

Copy link
Copy Markdown
Contributor

I couldn't push to your branch because of perms, so I created zeitlinger#1

@jackshirazi

Copy link
Copy Markdown
Contributor

@zeitlinger is there a problem with the suggested changes?

@zeitlinger

Copy link
Copy Markdown
Member Author

@zeitlinger is there a problem with the suggested changes?

I missed this - sorry - will look at it soon

@zeitlinger

Copy link
Copy Markdown
Member Author

@zeitlinger is there a problem with the suggested changes?

I missed this - sorry - will look at it soon

answered

@zeitlinger

Copy link
Copy Markdown
Member Author

Blocked by open-telemetry/opentelemetry-java#7832 (or an equivalent resource-aware SDK callback).

For declarative configuration, the OpAMP client should identify itself using the SDK's resolved Resource, including service.name and deployment environment. Reading those independently from system properties/environment variables can disagree with the configured SDK; copying YAML attributes also misses resource-detector results.

The intended flow is to prepare the sampler/policy state during configuration, then start OpAMP from the existing post-autoconfiguration callback once the SDK is built. That callback currently receives the SDK but cannot access its Resource. I've reopened #7832 to discuss exposing it with the SDK maintainers: open-telemetry/opentelemetry-java#7832 (comment).

OpAMP endpoint and authentication-header settings can retain their temporary sysprop/env configuration while the OpAMP declarative schema is unsettled. The blocker here is obtaining the resolved service identity without an agent-specific workaround. I'll hold off on landing the OpAMP compatibility changes until we settle that API direction.

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@zeitlinger

zeitlinger commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

@jackshirazi here's my update - it's the best I can come up with - but we can discuss more in the SIG meeting

User impact: configure the service name twice

With this implementation, users need to configure service.name in two places: in the SDK’s declarative configuration and separately for OpAMP through system properties or environment variables. Deployment-environment attributes also need to be kept consistent.

This is a bad user experience: it duplicates configuration and risks OpAMP reporting a different identity from the telemetry it manages. The PR now implements and documents this workaround, so the migration no longer depends on the proposed SDK resource getter.

Why the duplication is necessary

OpAMP needs the SDK’s resolved service identity. Simply copying service.name from YAML is not sufficient because resource detectors and resource merging can contribute attributes that are not present in the YAML.

The extension’s post-configuration callback receives the built SDK, but cannot access its resolved resource. Injecting system properties into the SDK’s declarative configuration would avoid some duplication, but would violate DC semantics rather than solve the resource-access problem.

The implementation therefore keeps the two paths separate: the SDK and dynamic-control policies use declarative configuration, while OpAMP reads its endpoint, headers, and identity independently from properties or environment variables.

Path to a better solution

As Jack explained, he is not opposed to exposing this information; it needs a specification basis. The proposed entities ResourceProvider may provide a path if it lands.

I’d encourage the folks working on OpAMP to bring this concrete usability problem into the specification process and help advance the OpAMP/SDK integration. That would let us replace duplicate configuration with a consistent, supported solution.

Status

The workaround is implemented, tested, and documented in the README. It lets this migration proceed without making SDK resource access a prerequisite for agent 3.0.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discuss migrating dynamic-control from ConfigProperties to DeclarativeConfigProperties

3 participants