Skip to content

[dynamic control] wire up first policy#2833

Open
jackshirazi wants to merge 4 commits into
open-telemetry:mainfrom
jackshirazi:policy26
Open

[dynamic control] wire up first policy#2833
jackshirazi wants to merge 4 commits into
open-telemetry:mainfrom
jackshirazi:policy26

Conversation

@jackshirazi
Copy link
Copy Markdown
Contributor

@jackshirazi jackshirazi commented May 13, 2026

Description:

Wire up TraceSamplingRatePolicy so it can be used in config.

Existing Issue(s):

#2546

Testing:

Added

Documentation:

Included

Outstanding items:

Wiring up the policy pipeline:

  • Generic: message -> provider -> policy -> policy handler -> implementer
  • eg "change sampling rate" message -> OpampPolicyProvider -> TraceSamplingRatePolicy -> PolicyStore -> TraceSamplingRatePolicyImplementer

Steps needed for the wiring:

  • configuring the pipeline
    • DONE: PolicyInitConfig
  • providers reading policies, eg OpampPolicyProvider, FilePolicyProvider, etc
    • DONE: OpampPolicyProvider
  • implementers applying policies, eg TraceSamplingRatePolicyImplementer
    • DONE: TraceSamplingRatePolicyImplementer
  • policy structures (eg TraceSamplingRatePolicy) that the provider converts messages into
    • DONE: TraceSamplingRatePolicy
  • registering config to policy structures (eg "trace_sampling_rate_policy" registered to TraceSamplingRatePolicy)
    • This PR
  • initializing policy classes (eg TraceSamplingRatePolicy needs to install a custom sampler)
    • DONE: TraceSamplingRatePolicy initialization
  • activate configured providers (eg start OpampPolicyprovider reading from it's source)
    • TBD
  • register implementers for policies (eg a new TraceSamplingRatePolicy is applied by a TraceSamplingRatePolicyImplementer)
    • TBD
  • link the provider to processing policies and applying implementers
    • DONE PolicyInit

Copilot AI review requested due to automatic review settings May 13, 2026 08:50
@jackshirazi jackshirazi requested a review from a team as a code owner May 13, 2026 08:50
@github-actions github-actions Bot requested a review from LikeTheSalad May 13, 2026 08:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 aims to make the first dynamic-control policy (TraceSamplingRatePolicy) selectable via policy-init configuration by registering the policy type string to its policy class/initializer, and ensuring the policy initializer produces the corresponding implementer.

Changes:

  • Added registration hook so TraceSamplingRatePolicy can be registered with PolicyInit.registerPolicyType(...).
  • Updated TraceSamplingRatePolicy.initialize(...) to return a PolicyImplementer (created with the installed DelegatingSampler).
  • Added a PolicyInitTest covering the “no init-config properties present” (non-initialization) path.

Reviewed changes

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

File Description
dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/registry/PolicyInitTest.java Adds tests around PolicyInit.init(...) behavior when init-config properties are absent.
dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/tracesampling/TraceSamplingRatePolicy.java Returns an implementer from initialization and adds a policy-type registration method.
dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/registry/PolicyInit.java Registers TraceSamplingRatePolicy in the static bootstrap.
Comments suppressed due to low confidence (2)

dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/registry/PolicyInitTest.java:57

  • These two tests are currently identical (both only verify that YAML/JSON init config properties are missing) but the second test name refers to missing top-level declarative telemetry policy config. Consider removing the duplicate or rewriting one test to exercise PolicyInit.initFromDeclarativeConfig(...) so the test intent matches what’s actually being validated.
  @Test
  void doesNotInitializePolicyWhenTopLevelTelemetryPolicyDeclarativeConfigMissing() {
    AutoConfigurationCustomizer customizer = mock(AutoConfigurationCustomizer.class);
    PolicyInit.init(customizer);
    Function<ConfigProperties, Map<String, String>> propertiesCustomizer =
        capturePropertiesCustomizer(customizer);

    ConfigProperties config = mock(ConfigProperties.class);
    when(config.getString(PolicyInitConfig.POLICY_INIT_CONFIG_PROPERTY_YAML)).thenReturn(null);
    when(config.getString(PolicyInitConfig.POLICY_INIT_CONFIG_PROPERTY_JSON)).thenReturn(null);
    Map<String, String> ignored = propertiesCustomizer.apply(config);

dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/registry/PolicyInitTest.java:76

  • invokeStaticNoArg() uses reflective access (setAccessible(true)) to call TraceSamplingRatePolicy.resetForTest. This is brittle (can be restricted by JVM access rules) and makes the test harder to maintain. Prefer placing the test in the same package as the method, or exposing a test-only reset hook without reflection (e.g., package-private in the test package or an explicit @VisibleForTesting-style method).
  private static void invokeStaticNoArg(Class<?> targetClass, String methodName) throws Exception {
    Method method = targetClass.getDeclaredMethod(methodName);
    method.setAccessible(true);
    method.invoke(null);
  }

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.

4 participants