From 0f34885ee077b18cf2360e64300e969b6e8df2bc Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Thu, 12 Feb 2026 14:48:35 +0000 Subject: [PATCH 01/15] Adding a callback to ConfigProvider for runtime instrumentation option changes --- .../api/incubator/config/ConfigProvider.java | 14 ++++++++++++++ .../InstrumentationConfigChangeListener.java | 18 ++++++++++++++++++ .../api/incubator/ConfigProviderTest.java | 4 ++++ 3 files changed, 36 insertions(+) create mode 100644 api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java index dcf6c7bd082..9205e9b1354 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java @@ -65,6 +65,20 @@ default DeclarativeConfigProperties getGeneralInstrumentationConfig() { return getInstrumentationConfig().get("general"); } + /** + * Registers an {@link InstrumentationConfigChangeListener} to receive updates when instrumentation + * configuration changes. + * + *

The default implementation performs no registration and returns a no-op handle. + * + * @param listener the listener to notify when instrumentation configuration changes + * @return an {@link AutoCloseable} handle that can be closed to unregister the listener + */ + default AutoCloseable addInstrumentationConfigChangeListener( + InstrumentationConfigChangeListener listener) { + return () -> {}; + } + /** Returns a no-op {@link ConfigProvider}. */ static ConfigProvider noop() { return DeclarativeConfigProperties::empty; diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java new file mode 100644 index 00000000000..c80cd18cdaf --- /dev/null +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java @@ -0,0 +1,18 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.api.incubator.config; + +/** Listener notified when instrumentation configuration changes. */ +@FunctionalInterface +public interface InstrumentationConfigChangeListener { + + /** + * Called when instrumentation configuration changes. + * + * @param instrumentationConfig the updated instrumentation configuration + */ + void onChange(DeclarativeConfigProperties instrumentationConfig); +} diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java index b5fdca1cdf6..ce7eab7076e 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java @@ -6,6 +6,7 @@ package io.opentelemetry.api.incubator; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import io.opentelemetry.api.incubator.config.ConfigProvider; import org.junit.jupiter.api.Test; @@ -24,5 +25,8 @@ void instrumentationConfigFallback() { assertThat(configProvider.getInstrumentationConfig()).isNotNull(); assertThat(configProvider.getInstrumentationConfig("servlet")).isNotNull(); assertThat(configProvider.getGeneralInstrumentationConfig()).isNotNull(); + AutoCloseable listenerRegistration = + configProvider.addInstrumentationConfigChangeListener(config -> {}); + assertThatCode(listenerRegistration::close).doesNotThrowAnyException(); } } From 0d8e905b81c476624799722aaba545ddfbb80a95 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Thu, 12 Feb 2026 15:06:51 +0000 Subject: [PATCH 02/15] correct listener interface --- .../InstrumentationConfigChangeListener.java | 19 ++++++++++++++++--- .../api/incubator/ConfigProviderTest.java | 2 +- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java index c80cd18cdaf..1e1726722b6 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java @@ -5,14 +5,27 @@ package io.opentelemetry.api.incubator.config; +import javax.annotation.Nullable; + /** Listener notified when instrumentation configuration changes. */ @FunctionalInterface public interface InstrumentationConfigChangeListener { /** - * Called when instrumentation configuration changes. + * Called when the effective config for one top-level instrumentation node changes (for example + * {@code methods}, {@code kafka}, or {@code grpc}). + * + *

Both config arguments are scoped to {@code instrumentationName}. + * + *

{@code newConfig} is never null. If the node is unset or cleared, {@code newConfig} is + * {@link DeclarativeConfigProperties#empty()}. * - * @param instrumentationConfig the updated instrumentation configuration + * @param instrumentationName the top-level instrumentation name that changed + * @param previousConfig the previous effective configuration, or {@code null} if unavailable + * @param newConfig the updated effective configuration for {@code instrumentationName} */ - void onChange(DeclarativeConfigProperties instrumentationConfig); + void onChange( + String instrumentationName, + @Nullable DeclarativeConfigProperties previousConfig, + DeclarativeConfigProperties newConfig); } diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java index ce7eab7076e..de73d93a85c 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java @@ -26,7 +26,7 @@ void instrumentationConfigFallback() { assertThat(configProvider.getInstrumentationConfig("servlet")).isNotNull(); assertThat(configProvider.getGeneralInstrumentationConfig()).isNotNull(); AutoCloseable listenerRegistration = - configProvider.addInstrumentationConfigChangeListener(config -> {}); + configProvider.addInstrumentationConfigChangeListener((name, previous, current) -> {}); assertThatCode(listenerRegistration::close).doesNotThrowAnyException(); } } From ff2fed83748fc55b64f3af0e57e25c5bacd7a2ee Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Thu, 12 Feb 2026 15:24:33 +0000 Subject: [PATCH 03/15] spotless --- .../io/opentelemetry/api/incubator/config/ConfigProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java index 9205e9b1354..0675817ea06 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java @@ -66,8 +66,8 @@ default DeclarativeConfigProperties getGeneralInstrumentationConfig() { } /** - * Registers an {@link InstrumentationConfigChangeListener} to receive updates when instrumentation - * configuration changes. + * Registers an {@link InstrumentationConfigChangeListener} to receive updates when + * instrumentation configuration changes. * *

The default implementation performs no registration and returns a no-op handle. * From afea89d34a87bdad637647eee6dd878405babf92 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Mon, 16 Feb 2026 14:08:30 +0000 Subject: [PATCH 04/15] feedback changes --- .../config/ConfigChangeListener.java | 26 ++++++++++++++++ .../config/ConfigChangeRegistration.java | 18 +++++++++++ .../api/incubator/config/ConfigProvider.java | 20 ++++++++---- .../InstrumentationConfigChangeListener.java | 31 ------------------- .../api/incubator/ConfigProviderTest.java | 6 ++-- 5 files changed, 62 insertions(+), 39 deletions(-) create mode 100644 api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigChangeListener.java create mode 100644 api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigChangeRegistration.java delete mode 100644 api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigChangeListener.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigChangeListener.java new file mode 100644 index 00000000000..44833713f8a --- /dev/null +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigChangeListener.java @@ -0,0 +1,26 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.api.incubator.config; + +/** Listener notified when declarative configuration changes. */ +@FunctionalInterface +public interface ConfigChangeListener { + + /** + * Called when the watched path changes. + * + *

{@code path} is the changed declarative configuration path, for example {@code + * .instrumentation/development.general.http} or {@code + * .instrumentation/development.java.methods}. + * + *

{@code newConfig} is never null. If the watched node is unset or cleared, {@code newConfig} + * is {@link DeclarativeConfigProperties#empty()}. + * + * @param path the declarative configuration path that changed + * @param newConfig the updated configuration for the changed path + */ + void onChange(String path, DeclarativeConfigProperties newConfig); +} diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigChangeRegistration.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigChangeRegistration.java new file mode 100644 index 00000000000..6e499ba11ad --- /dev/null +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigChangeRegistration.java @@ -0,0 +1,18 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.api.incubator.config; + +/** Registration handle returned by {@link ConfigProvider#addConfigChangeListener}. */ +@FunctionalInterface +public interface ConfigChangeRegistration { + + /** + * Unregister the listener associated with this registration. + * + *

Subsequent calls have no effect. + */ + void close(); +} diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java index 0675817ea06..8617ce6f7ca 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java @@ -66,16 +66,24 @@ default DeclarativeConfigProperties getGeneralInstrumentationConfig() { } /** - * Registers an {@link InstrumentationConfigChangeListener} to receive updates when - * instrumentation configuration changes. + * Registers a {@link ConfigChangeListener} for changes to a specific declarative configuration + * path. + * + *

Example paths include {@code .instrumentation/development.general.http} and {@code + * .instrumentation/development.java.methods}. + * + *

When a watched path changes, {@link ConfigChangeListener#onChange(String, + * DeclarativeConfigProperties)} is invoked with the changed path and updated configuration for + * that path. * *

The default implementation performs no registration and returns a no-op handle. * - * @param listener the listener to notify when instrumentation configuration changes - * @return an {@link AutoCloseable} handle that can be closed to unregister the listener + * @param path the declarative configuration path to watch + * @param listener the listener to notify when the watched path changes + * @return a {@link ConfigChangeRegistration} that can be closed to unregister the listener */ - default AutoCloseable addInstrumentationConfigChangeListener( - InstrumentationConfigChangeListener listener) { + default ConfigChangeRegistration addConfigChangeListener( + String path, ConfigChangeListener listener) { return () -> {}; } diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java deleted file mode 100644 index 1e1726722b6..00000000000 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigChangeListener.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.api.incubator.config; - -import javax.annotation.Nullable; - -/** Listener notified when instrumentation configuration changes. */ -@FunctionalInterface -public interface InstrumentationConfigChangeListener { - - /** - * Called when the effective config for one top-level instrumentation node changes (for example - * {@code methods}, {@code kafka}, or {@code grpc}). - * - *

Both config arguments are scoped to {@code instrumentationName}. - * - *

{@code newConfig} is never null. If the node is unset or cleared, {@code newConfig} is - * {@link DeclarativeConfigProperties#empty()}. - * - * @param instrumentationName the top-level instrumentation name that changed - * @param previousConfig the previous effective configuration, or {@code null} if unavailable - * @param newConfig the updated effective configuration for {@code instrumentationName} - */ - void onChange( - String instrumentationName, - @Nullable DeclarativeConfigProperties previousConfig, - DeclarativeConfigProperties newConfig); -} diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java index de73d93a85c..43aa3fd0ba0 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java @@ -8,6 +8,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; +import io.opentelemetry.api.incubator.config.ConfigChangeRegistration; import io.opentelemetry.api.incubator.config.ConfigProvider; import org.junit.jupiter.api.Test; @@ -25,8 +26,9 @@ void instrumentationConfigFallback() { assertThat(configProvider.getInstrumentationConfig()).isNotNull(); assertThat(configProvider.getInstrumentationConfig("servlet")).isNotNull(); assertThat(configProvider.getGeneralInstrumentationConfig()).isNotNull(); - AutoCloseable listenerRegistration = - configProvider.addInstrumentationConfigChangeListener((name, previous, current) -> {}); + ConfigChangeRegistration listenerRegistration = + configProvider.addConfigChangeListener( + ".instrumentation/development.java.servlet", (path, newConfig) -> {}); assertThatCode(listenerRegistration::close).doesNotThrowAnyException(); } } From 6c192014fa488d3ef4e00eed14335eddd220ea06 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Wed, 22 Apr 2026 12:17:50 +0100 Subject: [PATCH 05/15] tentative mutation of declarative config --- .../api/incubator/config/ConfigProvider.java | 32 +++ .../config/DeclarativeConfigProperties.java | 16 ++ .../MapBackedDeclarativeConfigProperties.java | 145 ++++++++++ .../internal/ExtendedOpenTelemetrySdk.java | 25 ++ .../sdk/internal/SdkConfigProvider.java | 256 +++++++++++++++++- 5 files changed, 470 insertions(+), 4 deletions(-) create mode 100644 api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java index 8617ce6f7ca..a944d3b9677 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java @@ -87,6 +87,38 @@ default ConfigChangeRegistration addConfigChangeListener( return () -> {}; } + /** + * Updates the declarative configuration subtree at the given path. + * + *

The path uses {@code .} as a separator (e.g., {@code + * ".instrumentation/development.java.myLib"}). The subtree at that path is replaced with {@code + * newSubtree}, and any registered {@link ConfigChangeListener}s watching affected paths are + * notified. + * + *

The default implementation is a no-op. + * + * @param path the declarative configuration path to update + * @param newSubtree the new configuration subtree to set at the path + */ + default void updateConfig(String path, DeclarativeConfigProperties newSubtree) {} + + /** + * Sets a single scalar configuration property at the given path. + * + *

The path uses {@code .} as a separator (e.g., {@code + * ".instrumentation/development.java.myLib"}). The property identified by {@code key} within that + * path is set to {@code value}, and any registered {@link ConfigChangeListener}s watching affected + * paths are notified. + * + *

The default implementation is a no-op. + * + * @param path the declarative configuration path containing the property + * @param key the property key within the path + * @param value the new value for the property (must be a scalar: String, Boolean, Long, Double, + * Integer, or a List of scalars) + */ + default void setConfigProperty(String path, String key, Object value) {} + /** Returns a no-op {@link ConfigProvider}. */ static ConfigProvider noop() { return DeclarativeConfigProperties::empty; diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java index ac3cef17c99..8ea87135437 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java @@ -43,6 +43,22 @@ static Map toMap(DeclarativeConfigProperties declarativeConfigPr return DeclarativeConfigPropertyUtil.toMap(declarativeConfigProperties); } + /** + * Create a {@link DeclarativeConfigProperties} from a {@code Map}. + * + *

This is the inverse of {@link #toMap(DeclarativeConfigProperties)}. Values in the map are + * expected to follow the same conventions: scalars, lists of scalars, nested maps, and lists of + * maps. + * + * @param map the map to wrap + * @param componentLoader the component loader to use + * @return a {@link DeclarativeConfigProperties} backed by the map + */ + static DeclarativeConfigProperties fromMap( + Map map, ComponentLoader componentLoader) { + return new MapBackedDeclarativeConfigProperties(map, componentLoader); + } + /** * Returns a {@link String} configuration property. * diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java new file mode 100644 index 00000000000..c9fb09513c9 --- /dev/null +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java @@ -0,0 +1,145 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.api.incubator.config; + +import io.opentelemetry.common.ComponentLoader; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import javax.annotation.Nullable; + +/** + * A {@link DeclarativeConfigProperties} implementation backed by a {@code Map}. + * + *

This is the inverse of {@link DeclarativeConfigProperties#toMap(DeclarativeConfigProperties)}. + * Values in the map are expected to follow the same conventions as YAML parsing output: scalars + * (String, Boolean, Long, Double, Integer), lists of scalars, maps (structured children), and lists + * of maps (structured lists). + */ +final class MapBackedDeclarativeConfigProperties implements DeclarativeConfigProperties { + + private final Map values; + private final ComponentLoader componentLoader; + + MapBackedDeclarativeConfigProperties(Map values, ComponentLoader componentLoader) { + this.values = values; + this.componentLoader = componentLoader; + } + + @Nullable + @Override + public String getString(String name) { + Object value = values.get(name); + return value instanceof String ? (String) value : null; + } + + @Nullable + @Override + public Boolean getBoolean(String name) { + Object value = values.get(name); + return value instanceof Boolean ? (Boolean) value : null; + } + + @Nullable + @Override + public Integer getInt(String name) { + Object value = values.get(name); + if (value instanceof Integer) { + return (Integer) value; + } + if (value instanceof Long) { + return ((Long) value).intValue(); + } + return null; + } + + @Nullable + @Override + public Long getLong(String name) { + Object value = values.get(name); + if (value instanceof Long) { + return (Long) value; + } + if (value instanceof Integer) { + return ((Integer) value).longValue(); + } + return null; + } + + @Nullable + @Override + public Double getDouble(String name) { + Object value = values.get(name); + if (value instanceof Double) { + return (Double) value; + } + if (value instanceof Number) { + return ((Number) value).doubleValue(); + } + return null; + } + + @SuppressWarnings("unchecked") + @Nullable + @Override + public List getScalarList(String name, Class scalarType) { + Object value = values.get(name); + if (!(value instanceof List)) { + return null; + } + List raw = (List) value; + List casted = new ArrayList<>(raw.size()); + for (Object element : raw) { + if (!scalarType.isInstance(element)) { + return null; + } + casted.add(scalarType.cast(element)); + } + return casted; + } + + @SuppressWarnings("unchecked") + @Nullable + @Override + public DeclarativeConfigProperties getStructured(String name) { + Object value = values.get(name); + if (!(value instanceof Map)) { + return null; + } + return new MapBackedDeclarativeConfigProperties((Map) value, componentLoader); + } + + @SuppressWarnings("unchecked") + @Nullable + @Override + public List getStructuredList(String name) { + Object value = values.get(name); + if (!(value instanceof List)) { + return null; + } + List raw = (List) value; + List result = new ArrayList<>(raw.size()); + for (Object element : raw) { + if (!(element instanceof Map)) { + return null; + } + result.add( + new MapBackedDeclarativeConfigProperties((Map) element, componentLoader)); + } + return result; + } + + @Override + public Set getPropertyKeys() { + return values.keySet(); + } + + @Override + public ComponentLoader getComponentLoader() { + return componentLoader; + } +} diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java index dc89b116108..3d832913d11 100644 --- a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java @@ -6,9 +6,12 @@ package io.opentelemetry.sdk.internal; import io.opentelemetry.api.incubator.ExtendedOpenTelemetry; +import io.opentelemetry.api.incubator.config.ConfigChangeListener; +import io.opentelemetry.api.incubator.config.ConfigChangeRegistration; import io.opentelemetry.api.incubator.config.ConfigProvider; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.common.CompletableResultCode; import javax.annotation.concurrent.ThreadSafe; /** @@ -50,6 +53,12 @@ public SdkConfigProvider getSdkConfigProvider() { return configProvider.unobfuscate(); } + @Override + public CompletableResultCode shutdown() { + configProvider.unobfuscate().shutdown(); + return super.shutdown(); + } + @Override public String toString() { return "ExtendedOpenTelemetrySdk{" @@ -81,6 +90,22 @@ public DeclarativeConfigProperties getInstrumentationConfig() { return delegate.getInstrumentationConfig(); } + @Override + public ConfigChangeRegistration addConfigChangeListener( + String path, ConfigChangeListener listener) { + return delegate.addConfigChangeListener(path, listener); + } + + @Override + public void updateConfig(String path, DeclarativeConfigProperties newSubtree) { + delegate.updateConfig(path, newSubtree); + } + + @Override + public void setConfigProperty(String path, String key, Object value) { + delegate.setConfigProperty(path, key, value); + } + private SdkConfigProvider unobfuscate() { return delegate; } diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java index a5399ed4a23..a312fa727a9 100644 --- a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java @@ -5,8 +5,22 @@ package io.opentelemetry.sdk.internal; +import static java.util.Objects.requireNonNull; + +import io.opentelemetry.api.incubator.config.ConfigChangeListener; +import io.opentelemetry.api.incubator.config.ConfigChangeRegistration; import io.opentelemetry.api.incubator.config.ConfigProvider; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.logging.Level; +import java.util.logging.Logger; /** * SDK implementation of {@link ConfigProvider}. @@ -16,11 +30,16 @@ * guarantees are made. */ public final class SdkConfigProvider implements ConfigProvider { + private static final Logger logger = Logger.getLogger(SdkConfigProvider.class.getName()); + private static final ConfigChangeRegistration NOOP_CHANGE_REGISTRATION = () -> {}; - private final DeclarativeConfigProperties instrumentationConfig; + private final AtomicReference openTelemetryConfigModel; + private final ConcurrentMap> listenersByPath = + new ConcurrentHashMap<>(); + private final AtomicBoolean disposed = new AtomicBoolean(false); private SdkConfigProvider(DeclarativeConfigProperties openTelemetryConfigModel) { - this.instrumentationConfig = openTelemetryConfigModel.get("instrumentation/development"); + this.openTelemetryConfigModel = new AtomicReference<>(requireNonNull(openTelemetryConfigModel)); } /** @@ -36,11 +55,240 @@ public static SdkConfigProvider create(DeclarativeConfigProperties openTelemetry @Override public DeclarativeConfigProperties getInstrumentationConfig() { - return instrumentationConfig; + return requireNonNull(openTelemetryConfigModel.get()).get("instrumentation/development"); + } + + @Override + public ConfigChangeRegistration addConfigChangeListener( + String path, ConfigChangeListener listener) { + requireNonNull(listener, "listener"); + String watchedPath = normalizeAndValidatePath(path); //fail fast on invalid path + if (disposed.get()) { + return NOOP_CHANGE_REGISTRATION; + } + + ListenerRegistration registration = new ListenerRegistration(watchedPath, listener); + listenersByPath + .computeIfAbsent(watchedPath, unused -> new CopyOnWriteArrayList<>()) + .add(registration); + if (disposed.get()) { + registration.close(); + return NOOP_CHANGE_REGISTRATION; + } + return registration; + } + + @Override + public void updateConfig(String path, DeclarativeConfigProperties newSubtree) { + requireNonNull(newSubtree, "newSubtree"); + String normalizedPath = normalizeAndValidatePath(path); + if (disposed.get()) { + return; + } + Map subtreeMap = DeclarativeConfigProperties.toMap(newSubtree); + while (true) { + DeclarativeConfigProperties current = requireNonNull(openTelemetryConfigModel.get()); + Map rootMap = DeclarativeConfigProperties.toMap(current); + setSubtreeAtPath(rootMap, normalizedPath, subtreeMap); + DeclarativeConfigProperties newRoot = + DeclarativeConfigProperties.fromMap(rootMap, current.getComponentLoader()); + if (openTelemetryConfigModel.compareAndSet(current, newRoot)) { + notifyListeners(current, newRoot); + return; + } + } + } + + @Override + public void setConfigProperty(String path, String key, Object value) { + requireNonNull(key, "key"); + requireNonNull(value, "value"); + String normalizedPath = normalizeAndValidatePath(path); + if (disposed.get()) { + return; + } + while (true) { + DeclarativeConfigProperties current = requireNonNull(openTelemetryConfigModel.get()); + Map rootMap = DeclarativeConfigProperties.toMap(current); + navigateToPath(rootMap, normalizedPath).put(key, value); + DeclarativeConfigProperties newRoot = + DeclarativeConfigProperties.fromMap(rootMap, current.getComponentLoader()); + if (openTelemetryConfigModel.compareAndSet(current, newRoot)) { + notifyListeners(current, newRoot); + return; + } + } + } + + // Visible for testing. + void updateOpenTelemetryConfigModel(DeclarativeConfigProperties updatedOpenTelemetryConfigModel) { + requireNonNull(updatedOpenTelemetryConfigModel, "updatedOpenTelemetryConfigModel"); + DeclarativeConfigProperties previous = + openTelemetryConfigModel.getAndSet(updatedOpenTelemetryConfigModel); + notifyListeners(previous, updatedOpenTelemetryConfigModel); + } + + private void notifyListeners( + DeclarativeConfigProperties previous, DeclarativeConfigProperties updated) { + if (disposed.get()) { + return; + } + + for (Map.Entry> entry : + listenersByPath.entrySet()) { + String watchedPath = entry.getKey(); + DeclarativeConfigProperties previousConfigAtPath = resolvePath(previous, watchedPath); + DeclarativeConfigProperties updatedConfigAtPath = resolvePath(updated, watchedPath); + if (hasSameContents(previousConfigAtPath, updatedConfigAtPath)) { + continue; + } + + for (ListenerRegistration registration : entry.getValue()) { + registration.notifyChange(watchedPath, updatedConfigAtPath); + } + } + } + + void shutdown() { + if (!disposed.compareAndSet(false, true)) { + return; + } + for (List registrations : listenersByPath.values()) { + for (ListenerRegistration registration : registrations) { + registration.close(); + } + } + listenersByPath.clear(); + } + + @SuppressWarnings("unchecked") + private static void setSubtreeAtPath( + Map rootMap, String normalizedPath, Map subtreeMap) { + String relativePath = normalizedPath.substring(1); + if (relativePath.isEmpty()) { + rootMap.clear(); + rootMap.putAll(subtreeMap); + return; + } + String[] segments = relativePath.split("\\."); + Map parent = rootMap; + for (int i = 0; i < segments.length - 1; i++) { + Object child = parent.get(segments[i]); + if (child instanceof Map) { + parent = (Map) child; + } else { + Map newChild = new HashMap<>(); + parent.put(segments[i], newChild); + parent = newChild; + } + } + parent.put(segments[segments.length - 1], subtreeMap); + } + + @SuppressWarnings("unchecked") + private static Map navigateToPath( + Map rootMap, String normalizedPath) { + String relativePath = normalizedPath.substring(1); + if (relativePath.isEmpty()) { + return rootMap; + } + Map current = rootMap; + String[] segments = relativePath.split("\\."); + for (String segment : segments) { + Object child = current.get(segment); + if (child instanceof Map) { + current = (Map) child; + } else { + Map newChild = new HashMap<>(); + current.put(segment, newChild); + current = newChild; + } + } + return current; + } + + private static boolean hasSameContents( + DeclarativeConfigProperties left, DeclarativeConfigProperties right) { + return DeclarativeConfigProperties.toMap(left).equals(DeclarativeConfigProperties.toMap(right)); + } + + private static DeclarativeConfigProperties resolvePath( + DeclarativeConfigProperties root, String watchedPath) { + String relativePath = watchedPath.substring(1); + if (relativePath.isEmpty()) { + return root; + } + + DeclarativeConfigProperties current = root; + String[] segments = relativePath.split("\\."); + for (String segment : segments) { + if (segment.isEmpty()) { + return DeclarativeConfigProperties.empty(); + } + current = current.get(segment); + } + return current; + } + + private static String normalizeAndValidatePath(String path) { + String watchedPath = requireNonNull(path, "path").trim(); + if (!watchedPath.startsWith(".")) { + throw new IllegalArgumentException( + "Config change listener path must be absolute and start with '.': " + path); + } + if (watchedPath.indexOf('*') >= 0) { + throw new IllegalArgumentException( + "Config change listener path does not support wildcards: " + path); + } + if (watchedPath.indexOf('[') >= 0 || watchedPath.indexOf(']') >= 0) { + throw new IllegalArgumentException( + "Config change listener path does not support sequence indexing: " + path); + } + return watchedPath; + } + + private final class ListenerRegistration implements ConfigChangeRegistration { + private final String watchedPath; + private final ConfigChangeListener listener; + private final AtomicBoolean closed = new AtomicBoolean(false); + + private ListenerRegistration(String watchedPath, ConfigChangeListener listener) { + this.watchedPath = watchedPath; + this.listener = listener; + } + + @Override + public void close() { + if (!closed.compareAndSet(false, true)) { + return; + } + CopyOnWriteArrayList registrations = listenersByPath.get(watchedPath); + if (registrations == null) { + return; + } + registrations.remove(this); + if (registrations.isEmpty()) { + listenersByPath.remove(watchedPath, registrations); + } + } + + private void notifyChange(String changedPath, DeclarativeConfigProperties updatedConfigAtPath) { + if (closed.get()) { + return; + } + try { + listener.onChange(changedPath, updatedConfigAtPath); + } catch (Throwable throwable) { + logger.log( + Level.WARNING, + "Config change listener threw while handling path " + changedPath, + throwable); + } + } } @Override public String toString() { - return "SdkConfigProvider{" + "instrumentationConfig=" + instrumentationConfig + '}'; + return "SdkConfigProvider{" + "instrumentationConfig=" + getInstrumentationConfig() + '}'; } } From 00ede1a4c248b7df9de305d65ba814d566b1a75c Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Wed, 22 Apr 2026 12:20:37 +0100 Subject: [PATCH 06/15] spotless --- .../io/opentelemetry/api/incubator/config/ConfigProvider.java | 4 ++-- .../config/MapBackedDeclarativeConfigProperties.java | 3 ++- .../java/io/opentelemetry/sdk/internal/SdkConfigProvider.java | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java index a944d3b9677..715eaa25d2a 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java @@ -107,8 +107,8 @@ default void updateConfig(String path, DeclarativeConfigProperties newSubtree) { * *

The path uses {@code .} as a separator (e.g., {@code * ".instrumentation/development.java.myLib"}). The property identified by {@code key} within that - * path is set to {@code value}, and any registered {@link ConfigChangeListener}s watching affected - * paths are notified. + * path is set to {@code value}, and any registered {@link ConfigChangeListener}s watching + * affected paths are notified. * *

The default implementation is a no-op. * diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java index c9fb09513c9..3effc4f84d3 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java @@ -25,7 +25,8 @@ final class MapBackedDeclarativeConfigProperties implements DeclarativeConfigPro private final Map values; private final ComponentLoader componentLoader; - MapBackedDeclarativeConfigProperties(Map values, ComponentLoader componentLoader) { + MapBackedDeclarativeConfigProperties( + Map values, ComponentLoader componentLoader) { this.values = values; this.componentLoader = componentLoader; } diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java index a312fa727a9..6dc1ca4ba6a 100644 --- a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java @@ -62,7 +62,7 @@ public DeclarativeConfigProperties getInstrumentationConfig() { public ConfigChangeRegistration addConfigChangeListener( String path, ConfigChangeListener listener) { requireNonNull(listener, "listener"); - String watchedPath = normalizeAndValidatePath(path); //fail fast on invalid path + String watchedPath = normalizeAndValidatePath(path); // fail fast on invalid path if (disposed.get()) { return NOOP_CHANGE_REGISTRATION; } From 4fa3872b91c0f40da66db2cadc2564127cd2caaf Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Wed, 22 Apr 2026 12:39:54 +0100 Subject: [PATCH 07/15] fix failure --- .../opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java index 3d832913d11..9a72679103f 100644 --- a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java @@ -11,7 +11,6 @@ import io.opentelemetry.api.incubator.config.ConfigProvider; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.common.CompletableResultCode; import javax.annotation.concurrent.ThreadSafe; /** @@ -54,9 +53,9 @@ public SdkConfigProvider getSdkConfigProvider() { } @Override - public CompletableResultCode shutdown() { + public void close() { configProvider.unobfuscate().shutdown(); - return super.shutdown(); + super.close(); } @Override From e985b033a952c36e49791775e41a0eee6d842225 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Wed, 22 Apr 2026 13:04:21 +0100 Subject: [PATCH 08/15] test coverage --- .../sdk/internal/SdkConfigProviderTest.java | 534 ++++++++++++++++++ 1 file changed, 534 insertions(+) create mode 100644 api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java diff --git a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java new file mode 100644 index 00000000000..2b3eefa90d3 --- /dev/null +++ b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java @@ -0,0 +1,534 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import io.opentelemetry.api.incubator.config.ConfigChangeRegistration; +import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; +import io.opentelemetry.common.ComponentLoader; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import javax.annotation.Nullable; +import org.junit.jupiter.api.Test; + +class SdkConfigProviderTest { + + @Test + void addConfigChangeListener_notifiesOnWatchedPathChange() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + List notifications = new ArrayList<>(); + ConfigChangeRegistration registration = + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); + + provider.updateOpenTelemetryConfigModel( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + + assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); + registration.close(); + } + + @Test + void addConfigChangeListener_ignoresUnchangedAndNonWatchedUpdates() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf( + "general", + mapOf("http", mapOf("enabled", "true")), + "java", + mapOf("servlet", mapOf("enabled", "true")))))); + AtomicInteger callbackCount = new AtomicInteger(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> callbackCount.incrementAndGet()); + + provider.updateOpenTelemetryConfigModel( + config( + mapOf( + "instrumentation/development", + mapOf( + "general", + mapOf("http", mapOf("enabled", "true")), + "java", + mapOf("servlet", mapOf("enabled", "false")))))); + provider.updateOpenTelemetryConfigModel( + config( + mapOf( + "instrumentation/development", + mapOf( + "general", + mapOf("http", mapOf("enabled", "true")), + "java", + mapOf("servlet", mapOf("enabled", "false")))))); + + assertThat(callbackCount).hasValue(0); + } + + @Test + void addConfigChangeListener_returnsEmptyNodeWhenWatchedPathCleared() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + List> propertyKeysSeen = new ArrayList<>(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> propertyKeysSeen.add(newConfig.getPropertyKeys())); + + provider.updateOpenTelemetryConfigModel( + config(mapOf("instrumentation/development", mapOf("general", mapOf())))); + + assertThat(propertyKeysSeen).containsExactly(Collections.emptySet()); + } + + @Test + void addConfigChangeListener_closeAndShutdownStopCallbacks() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + AtomicInteger callbackCount = new AtomicInteger(); + ConfigChangeRegistration registration = + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> callbackCount.incrementAndGet()); + + registration.close(); + registration.close(); + provider.updateOpenTelemetryConfigModel( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + assertThat(callbackCount).hasValue(0); + + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> callbackCount.incrementAndGet()); + provider.shutdown(); + provider.updateOpenTelemetryConfigModel( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + assertThat(callbackCount).hasValue(0); + } + + @Test + void addConfigChangeListener_listenerExceptionIsIsolated() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + AtomicInteger successfulCallbacks = new AtomicInteger(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> { + throw new IllegalStateException("boom"); + }); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> successfulCallbacks.incrementAndGet()); + + provider.updateOpenTelemetryConfigModel( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + + assertThat(successfulCallbacks).hasValue(1); + } + + @Test + void updateConfig_replacesSubtreeAndNotifiesListener() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + List notifications = new ArrayList<>(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); + + provider.updateConfig( + ".instrumentation/development.general.http", config(mapOf("enabled", "true"))); + + assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); + } + + @Test + void updateConfig_doesNotNotifyWhenSubtreeUnchanged() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + AtomicInteger callbackCount = new AtomicInteger(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> callbackCount.incrementAndGet()); + + provider.updateConfig( + ".instrumentation/development.general.http", config(mapOf("enabled", "true"))); + + assertThat(callbackCount).hasValue(0); + } + + @Test + void updateConfig_createsIntermediateNodesIfMissing() { + SdkConfigProvider provider = SdkConfigProvider.create(config(mapOf())); + List notifications = new ArrayList<>(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); + + provider.updateConfig( + ".instrumentation/development.general.http", config(mapOf("enabled", "true"))); + + assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); + } + + @Test + void updateConfig_noopWhenDisposed() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + AtomicInteger callbackCount = new AtomicInteger(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> callbackCount.incrementAndGet()); + provider.shutdown(); + + provider.updateConfig( + ".instrumentation/development.general.http", config(mapOf("enabled", "true"))); + + assertThat(callbackCount).hasValue(0); + } + + @Test + void setConfigProperty_setsScalarAndNotifiesListener() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + List notifications = new ArrayList<>(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); + + provider.setConfigProperty(".instrumentation/development.general.http", "enabled", "true"); + + assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); + } + + @Test + void setConfigProperty_doesNotNotifyWhenValueUnchanged() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + AtomicInteger callbackCount = new AtomicInteger(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> callbackCount.incrementAndGet()); + + provider.setConfigProperty(".instrumentation/development.general.http", "enabled", "true"); + + assertThat(callbackCount).hasValue(0); + } + + @Test + void setConfigProperty_noopWhenDisposed() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + AtomicInteger callbackCount = new AtomicInteger(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> callbackCount.incrementAndGet()); + provider.shutdown(); + + provider.setConfigProperty(".instrumentation/development.general.http", "enabled", "true"); + + assertThat(callbackCount).hasValue(0); + } + + @Test + void concurrentUpdates_allChangesAreApplied() throws Exception { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("count", "0")))))); + List notifications = new CopyOnWriteArrayList<>(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> notifications.add(newConfig.getString("count"))); + + int threadCount = 10; + ExecutorService executor = Executors.newFixedThreadPool(threadCount); + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch doneLatch = new CountDownLatch(threadCount); + List> futures = new ArrayList<>(); + for (int i = 0; i < threadCount; i++) { + int index = i + 1; + futures.add( + executor.submit( + () -> { + try { + startLatch.await(); + provider.setConfigProperty( + ".instrumentation/development.general.http", "count", String.valueOf(index)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } finally { + doneLatch.countDown(); + } + })); + } + startLatch.countDown(); + assertThat(doneLatch.await(5, TimeUnit.SECONDS)).isTrue(); + for (java.util.concurrent.Future future : futures) { + future.get(1, TimeUnit.SECONDS); + } + executor.shutdown(); + + assertThat(notifications).hasSize(threadCount); + DeclarativeConfigProperties finalConfig = + provider.getInstrumentationConfig().get("general").get("http"); + assertThat(finalConfig.getString("count")).isNotNull(); + } + + @Test + void pathValidation_rejectsMissingLeadingDot() { + SdkConfigProvider provider = SdkConfigProvider.create(config(mapOf())); + + assertThatThrownBy( + () -> provider.addConfigChangeListener("instrumentation", (path, newConfig) -> {})) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> provider.updateConfig("instrumentation", config(mapOf()))) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy( + () -> provider.setConfigProperty("instrumentation", "key", "value")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void pathValidation_rejectsWildcards() { + SdkConfigProvider provider = SdkConfigProvider.create(config(mapOf())); + + assertThatThrownBy(() -> provider.addConfigChangeListener(".*", (path, newConfig) -> {})) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> provider.updateConfig(".foo.*", config(mapOf()))) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> provider.setConfigProperty(".foo.*", "key", "value")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void pathValidation_rejectsBrackets() { + SdkConfigProvider provider = SdkConfigProvider.create(config(mapOf())); + + assertThatThrownBy( + () -> provider.addConfigChangeListener(".foo[0]", (path, newConfig) -> {})) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> provider.updateConfig(".foo[0]", config(mapOf()))) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> provider.setConfigProperty(".foo[0]", "key", "value")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void updateConfig_rootPathReplacesEntireConfig() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + List notifications = new ArrayList<>(); + provider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); + + provider.updateConfig( + ".", + config( + mapOf( + "instrumentation/development", + mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + + assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); + } + + private static DeclarativeConfigProperties config(Map root) { + return new MapBackedDeclarativeConfigProperties(root); + } + + private static Map mapOf(Object... entries) { + Map result = new LinkedHashMap<>(); + for (int i = 0; i < entries.length; i += 2) { + result.put((String) entries[i], entries[i + 1]); + } + return result; + } + + private static final class MapBackedDeclarativeConfigProperties + implements DeclarativeConfigProperties { + private static final ComponentLoader COMPONENT_LOADER = + ComponentLoader.forClassLoader(MapBackedDeclarativeConfigProperties.class.getClassLoader()); + + private final Map values; + + private MapBackedDeclarativeConfigProperties(Map values) { + this.values = values; + } + + @Override + public String getString(String name) { + Object value = values.get(name); + return value instanceof String ? (String) value : null; + } + + @Override + public Boolean getBoolean(String name) { + Object value = values.get(name); + return value instanceof Boolean ? (Boolean) value : null; + } + + @Override + public Integer getInt(String name) { + Object value = values.get(name); + return value instanceof Integer ? (Integer) value : null; + } + + @Override + public Long getLong(String name) { + Object value = values.get(name); + if (value instanceof Long) { + return (Long) value; + } + if (value instanceof Integer) { + return ((Integer) value).longValue(); + } + return null; + } + + @Override + public Double getDouble(String name) { + Object value = values.get(name); + if (value instanceof Double) { + return (Double) value; + } + if (value instanceof Number) { + return ((Number) value).doubleValue(); + } + return null; + } + + @SuppressWarnings("unchecked") + @Nullable + @Override + public List getScalarList(String name, Class scalarType) { + Object value = values.get(name); + if (!(value instanceof List)) { + return null; + } + List raw = (List) value; + List casted = new ArrayList<>(raw.size()); + for (Object element : raw) { + if (!scalarType.isInstance(element)) { + return null; + } + casted.add((T) element); + } + return casted; + } + + @SuppressWarnings("unchecked") + @Override + public DeclarativeConfigProperties getStructured(String name) { + Object value = values.get(name); + if (!(value instanceof Map)) { + return null; + } + return new MapBackedDeclarativeConfigProperties((Map) value); + } + + @SuppressWarnings("unchecked") + @Nullable + @Override + public List getStructuredList(String name) { + Object value = values.get(name); + if (!(value instanceof List)) { + return null; + } + List raw = (List) value; + List result = new ArrayList<>(raw.size()); + for (Object element : raw) { + if (!(element instanceof Map)) { + return null; + } + result.add(new MapBackedDeclarativeConfigProperties((Map) element)); + } + return result; + } + + @Override + public Set getPropertyKeys() { + return values.keySet(); + } + + @Override + public ComponentLoader getComponentLoader() { + return COMPONENT_LOADER; + } + } +} From fb61cb90d06e61b7f3c2659f428e7cb6932f1765 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Wed, 22 Apr 2026 13:05:11 +0100 Subject: [PATCH 09/15] spotless --- .../opentelemetry/sdk/internal/SdkConfigProviderTest.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java index 2b3eefa90d3..75a2a3224ee 100644 --- a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java @@ -355,8 +355,7 @@ void pathValidation_rejectsMissingLeadingDot() { .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> provider.updateConfig("instrumentation", config(mapOf()))) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy( - () -> provider.setConfigProperty("instrumentation", "key", "value")) + assertThatThrownBy(() -> provider.setConfigProperty("instrumentation", "key", "value")) .isInstanceOf(IllegalArgumentException.class); } @@ -376,8 +375,7 @@ void pathValidation_rejectsWildcards() { void pathValidation_rejectsBrackets() { SdkConfigProvider provider = SdkConfigProvider.create(config(mapOf())); - assertThatThrownBy( - () -> provider.addConfigChangeListener(".foo[0]", (path, newConfig) -> {})) + assertThatThrownBy(() -> provider.addConfigChangeListener(".foo[0]", (path, newConfig) -> {})) .isInstanceOf(IllegalArgumentException.class); assertThatThrownBy(() -> provider.updateConfig(".foo[0]", config(mapOf()))) .isInstanceOf(IllegalArgumentException.class); From 069ba3828a921e0610b469bda938b4c506409fc4 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Wed, 22 Apr 2026 13:24:54 +0100 Subject: [PATCH 10/15] warning fix --- .../io/opentelemetry/sdk/internal/SdkConfigProviderTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java index 75a2a3224ee..7384eeb5d4d 100644 --- a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java @@ -21,6 +21,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nullable; @@ -316,7 +317,7 @@ void concurrentUpdates_allChangesAreApplied() throws Exception { ExecutorService executor = Executors.newFixedThreadPool(threadCount); CountDownLatch startLatch = new CountDownLatch(1); CountDownLatch doneLatch = new CountDownLatch(threadCount); - List> futures = new ArrayList<>(); + List> futures = new ArrayList<>(); for (int i = 0; i < threadCount; i++) { int index = i + 1; futures.add( @@ -335,7 +336,7 @@ void concurrentUpdates_allChangesAreApplied() throws Exception { } startLatch.countDown(); assertThat(doneLatch.await(5, TimeUnit.SECONDS)).isTrue(); - for (java.util.concurrent.Future future : futures) { + for (Future future : futures) { future.get(1, TimeUnit.SECONDS); } executor.shutdown(); From a4e42bf7bafd3a11693e351276e49d2507d20c4c Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Wed, 22 Apr 2026 14:03:25 +0100 Subject: [PATCH 11/15] test coverage --- .../incubator/ExtendedOpenTelemetryTest.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ExtendedOpenTelemetryTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ExtendedOpenTelemetryTest.java index 017e691946d..fea6659a711 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ExtendedOpenTelemetryTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ExtendedOpenTelemetryTest.java @@ -31,6 +31,7 @@ import java.io.ByteArrayInputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -111,6 +112,32 @@ void instrumentationConfig() { .isEqualTo(Arrays.asList("client-request-header1", "client-request-header2")); } + @Test + void close_shutsDownConfigProvider() { + String configYaml = + "instrumentation/development:\n" + + " general:\n" + + " http:\n" + + " enabled: \"false\""; + SdkConfigProvider configProvider = + SdkConfigProvider.create( + DeclarativeConfiguration.toConfigProperties( + new ByteArrayInputStream(configYaml.getBytes(StandardCharsets.UTF_8)))); + ExtendedOpenTelemetrySdk sdk = + ExtendedOpenTelemetrySdk.create(OpenTelemetrySdk.builder().build(), configProvider); + + AtomicInteger callbackCount = new AtomicInteger(); + configProvider.addConfigChangeListener( + ".instrumentation/development.general.http", + (path, newConfig) -> callbackCount.incrementAndGet()); + + sdk.close(); + + configProvider.setConfigProperty( + ".instrumentation/development.general.http", "enabled", "true"); + assertThat(callbackCount.get()).isEqualTo(0); + } + @Test void instrumentationConfigFallback() { ConfigProvider configProvider = ConfigProvider.noop(); From f90362a3340862de11106feccaaaf2575add4704 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Wed, 22 Apr 2026 15:10:20 +0100 Subject: [PATCH 12/15] test coverage --- .../api/incubator/ConfigProviderTest.java | 15 ++ ...BackedDeclarativeConfigPropertiesTest.java | 165 ++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 api/incubator/src/test/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigPropertiesTest.java diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java index 43aa3fd0ba0..496d2409595 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java @@ -10,6 +10,7 @@ import io.opentelemetry.api.incubator.config.ConfigChangeRegistration; import io.opentelemetry.api.incubator.config.ConfigProvider; +import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; import org.junit.jupiter.api.Test; class ConfigProviderTest { @@ -31,4 +32,18 @@ void instrumentationConfigFallback() { ".instrumentation/development.java.servlet", (path, newConfig) -> {}); assertThatCode(listenerRegistration::close).doesNotThrowAnyException(); } + + @Test + void defaultUpdateConfig_isNoop() { + ConfigProvider configProvider = ConfigProvider.noop(); + assertThatCode(() -> configProvider.updateConfig(".foo", DeclarativeConfigProperties.empty())) + .doesNotThrowAnyException(); + } + + @Test + void defaultSetConfigProperty_isNoop() { + ConfigProvider configProvider = ConfigProvider.noop(); + assertThatCode(() -> configProvider.setConfigProperty(".foo", "key", "value")) + .doesNotThrowAnyException(); + } } diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigPropertiesTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigPropertiesTest.java new file mode 100644 index 00000000000..d966e586a11 --- /dev/null +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigPropertiesTest.java @@ -0,0 +1,165 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.api.incubator.config; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.common.ComponentLoader; +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class MapBackedDeclarativeConfigPropertiesTest { + + private static final ComponentLoader COMPONENT_LOADER = + ComponentLoader.forClassLoader( + MapBackedDeclarativeConfigPropertiesTest.class.getClassLoader()); + + @Test + void getString() { + DeclarativeConfigProperties config = fromMap(mapOf("key", "value", "notString", 42)); + + assertThat(config.getString("key")).isEqualTo("value"); + assertThat(config.getString("missing")).isNull(); + assertThat(config.getString("notString")).isNull(); + } + + @Test + void getBoolean() { + DeclarativeConfigProperties config = fromMap(mapOf("key", true, "notBoolean", "true")); + + assertThat(config.getBoolean("key")).isTrue(); + assertThat(config.getBoolean("missing")).isNull(); + assertThat(config.getBoolean("notBoolean")).isNull(); + } + + @Test + void getInt() { + DeclarativeConfigProperties config = fromMap(mapOf("intVal", 42, "longVal", 100L, "str", "x")); + + assertThat(config.getInt("intVal")).isEqualTo(42); + assertThat(config.getInt("longVal")).isEqualTo(100); + assertThat(config.getInt("missing")).isNull(); + assertThat(config.getInt("str")).isNull(); + } + + @Test + void getLong() { + DeclarativeConfigProperties config = fromMap(mapOf("longVal", 100L, "intVal", 42, "str", "x")); + + assertThat(config.getLong("longVal")).isEqualTo(100L); + assertThat(config.getLong("intVal")).isEqualTo(42L); + assertThat(config.getLong("missing")).isNull(); + assertThat(config.getLong("str")).isNull(); + } + + @Test + void getDouble() { + DeclarativeConfigProperties config = + fromMap(mapOf("doubleVal", 3.14, "intVal", 42, "str", "x")); + + assertThat(config.getDouble("doubleVal")).isEqualTo(3.14); + assertThat(config.getDouble("intVal")).isEqualTo(42.0); + assertThat(config.getDouble("missing")).isNull(); + assertThat(config.getDouble("str")).isNull(); + } + + @Test + void getScalarList() { + DeclarativeConfigProperties config = + fromMap(mapOf("strings", Arrays.asList("a", "b"), "mixed", Arrays.asList("a", 1))); + + assertThat(config.getScalarList("strings", String.class)).containsExactly("a", "b"); + assertThat(config.getScalarList("mixed", String.class)).isNull(); + assertThat(config.getScalarList("missing", String.class)).isNull(); + } + + @Test + void getScalarList_nonListReturnsNull() { + DeclarativeConfigProperties config = fromMap(mapOf("notList", "value")); + + assertThat(config.getScalarList("notList", String.class)).isNull(); + } + + @Test + void getStructured() { + Map child = mapOf("nested", "value"); + DeclarativeConfigProperties config = fromMap(mapOf("child", child, "notMap", "scalar")); + + DeclarativeConfigProperties structured = config.getStructured("child"); + assertThat(structured).isNotNull(); + assertThat(structured.getString("nested")).isEqualTo("value"); + assertThat(config.getStructured("missing")).isNull(); + assertThat(config.getStructured("notMap")).isNull(); + } + + @Test + void getStructuredList() { + List> items = + Arrays.asList(mapOf("name", "first"), mapOf("name", "second")); + DeclarativeConfigProperties config = + fromMap(mapOf("items", items, "badItems", Arrays.asList("notAMap"))); + + List result = config.getStructuredList("items"); + assertThat(result).hasSize(2); + assertThat(result.get(0).getString("name")).isEqualTo("first"); + assertThat(result.get(1).getString("name")).isEqualTo("second"); + assertThat(config.getStructuredList("missing")).isNull(); + assertThat(config.getStructuredList("badItems")).isNull(); + } + + @Test + void getStructuredList_nonListReturnsNull() { + DeclarativeConfigProperties config = fromMap(mapOf("notList", "value")); + + assertThat(config.getStructuredList("notList")).isNull(); + } + + @Test + void getPropertyKeys() { + DeclarativeConfigProperties config = fromMap(mapOf("a", 1, "b", 2)); + + assertThat(config.getPropertyKeys()).containsExactlyInAnyOrder("a", "b"); + } + + @Test + void getPropertyKeys_empty() { + DeclarativeConfigProperties config = fromMap(Collections.emptyMap()); + + assertThat(config.getPropertyKeys()).isEmpty(); + } + + @Test + void getComponentLoader() { + DeclarativeConfigProperties config = fromMap(Collections.emptyMap()); + + assertThat(config.getComponentLoader()).isSameAs(COMPONENT_LOADER); + } + + @Test + void get_defaultMethod() { + Map child = mapOf("nested", "value"); + DeclarativeConfigProperties config = fromMap(mapOf("child", child)); + + assertThat(config.get("child").getString("nested")).isEqualTo("value"); + assertThat(config.get("missing").getPropertyKeys()).isEmpty(); + } + + private static DeclarativeConfigProperties fromMap(Map map) { + return DeclarativeConfigProperties.fromMap(map, COMPONENT_LOADER); + } + + private static Map mapOf(Object... entries) { + Map result = new LinkedHashMap<>(); + for (int i = 0; i < entries.length; i += 2) { + result.put((String) entries[i], entries[i + 1]); + } + return result; + } +} From 0b26dfb3802d4e5d4f2383f4e33552ce3e24b5bf Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Thu, 30 Apr 2026 13:45:16 +0100 Subject: [PATCH 13/15] adapted from feedback --- .../api/incubator/config/ConfigProvider.java | 4 + .../config/DeclarativeConfigProperties.java | 16 -- .../MapBackedDeclarativeConfigProperties.java | 146 ---------------- .../config/InstrumentationConfigUtilTest.java | 2 +- ...BackedDeclarativeConfigPropertiesTest.java | 165 ------------------ .../sdk/internal/SdkConfigProviderTest.java | 37 ++++ .../fileconfig/DeclarativeConfiguration.java | 2 + .../DeclarativeConfigContextTest.java | 2 + .../YamlDeclarativeConfigPropertiesTest.java | 1 + .../sdk/internal/SdkConfigProvider.java | 119 +++++++++---- .../YamlDeclarativeConfigProperties.java | 9 +- 11 files changed, 132 insertions(+), 371 deletions(-) delete mode 100644 api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java delete mode 100644 api/incubator/src/test/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigPropertiesTest.java rename {sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig => sdk/all/src/main/java/io/opentelemetry/sdk/internal}/YamlDeclarativeConfigProperties.java (96%) diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java index 715eaa25d2a..46554b21c93 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java @@ -99,6 +99,8 @@ default ConfigChangeRegistration addConfigChangeListener( * * @param path the declarative configuration path to update * @param newSubtree the new configuration subtree to set at the path + * @throws DeclarativeConfigException if an intermediate segment of the path resolves to a + * non-mapping value (e.g., a scalar or list), indicating a schema conflict */ default void updateConfig(String path, DeclarativeConfigProperties newSubtree) {} @@ -116,6 +118,8 @@ default void updateConfig(String path, DeclarativeConfigProperties newSubtree) { * @param key the property key within the path * @param value the new value for the property (must be a scalar: String, Boolean, Long, Double, * Integer, or a List of scalars) + * @throws DeclarativeConfigException if an intermediate segment of the path resolves to a + * non-mapping value (e.g., a scalar or list), indicating a schema conflict */ default void setConfigProperty(String path, String key, Object value) {} diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java index 8ea87135437..ac3cef17c99 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java @@ -43,22 +43,6 @@ static Map toMap(DeclarativeConfigProperties declarativeConfigPr return DeclarativeConfigPropertyUtil.toMap(declarativeConfigProperties); } - /** - * Create a {@link DeclarativeConfigProperties} from a {@code Map}. - * - *

This is the inverse of {@link #toMap(DeclarativeConfigProperties)}. Values in the map are - * expected to follow the same conventions: scalars, lists of scalars, nested maps, and lists of - * maps. - * - * @param map the map to wrap - * @param componentLoader the component loader to use - * @return a {@link DeclarativeConfigProperties} backed by the map - */ - static DeclarativeConfigProperties fromMap( - Map map, ComponentLoader componentLoader) { - return new MapBackedDeclarativeConfigProperties(map, componentLoader); - } - /** * Returns a {@link String} configuration property. * diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java deleted file mode 100644 index 3effc4f84d3..00000000000 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigProperties.java +++ /dev/null @@ -1,146 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.api.incubator.config; - -import io.opentelemetry.common.ComponentLoader; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Set; -import javax.annotation.Nullable; - -/** - * A {@link DeclarativeConfigProperties} implementation backed by a {@code Map}. - * - *

This is the inverse of {@link DeclarativeConfigProperties#toMap(DeclarativeConfigProperties)}. - * Values in the map are expected to follow the same conventions as YAML parsing output: scalars - * (String, Boolean, Long, Double, Integer), lists of scalars, maps (structured children), and lists - * of maps (structured lists). - */ -final class MapBackedDeclarativeConfigProperties implements DeclarativeConfigProperties { - - private final Map values; - private final ComponentLoader componentLoader; - - MapBackedDeclarativeConfigProperties( - Map values, ComponentLoader componentLoader) { - this.values = values; - this.componentLoader = componentLoader; - } - - @Nullable - @Override - public String getString(String name) { - Object value = values.get(name); - return value instanceof String ? (String) value : null; - } - - @Nullable - @Override - public Boolean getBoolean(String name) { - Object value = values.get(name); - return value instanceof Boolean ? (Boolean) value : null; - } - - @Nullable - @Override - public Integer getInt(String name) { - Object value = values.get(name); - if (value instanceof Integer) { - return (Integer) value; - } - if (value instanceof Long) { - return ((Long) value).intValue(); - } - return null; - } - - @Nullable - @Override - public Long getLong(String name) { - Object value = values.get(name); - if (value instanceof Long) { - return (Long) value; - } - if (value instanceof Integer) { - return ((Integer) value).longValue(); - } - return null; - } - - @Nullable - @Override - public Double getDouble(String name) { - Object value = values.get(name); - if (value instanceof Double) { - return (Double) value; - } - if (value instanceof Number) { - return ((Number) value).doubleValue(); - } - return null; - } - - @SuppressWarnings("unchecked") - @Nullable - @Override - public List getScalarList(String name, Class scalarType) { - Object value = values.get(name); - if (!(value instanceof List)) { - return null; - } - List raw = (List) value; - List casted = new ArrayList<>(raw.size()); - for (Object element : raw) { - if (!scalarType.isInstance(element)) { - return null; - } - casted.add(scalarType.cast(element)); - } - return casted; - } - - @SuppressWarnings("unchecked") - @Nullable - @Override - public DeclarativeConfigProperties getStructured(String name) { - Object value = values.get(name); - if (!(value instanceof Map)) { - return null; - } - return new MapBackedDeclarativeConfigProperties((Map) value, componentLoader); - } - - @SuppressWarnings("unchecked") - @Nullable - @Override - public List getStructuredList(String name) { - Object value = values.get(name); - if (!(value instanceof List)) { - return null; - } - List raw = (List) value; - List result = new ArrayList<>(raw.size()); - for (Object element : raw) { - if (!(element instanceof Map)) { - return null; - } - result.add( - new MapBackedDeclarativeConfigProperties((Map) element, componentLoader)); - } - return result; - } - - @Override - public Set getPropertyKeys() { - return values.keySet(); - } - - @Override - public ComponentLoader getComponentLoader() { - return componentLoader; - } -} diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/InstrumentationConfigUtilTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/InstrumentationConfigUtilTest.java index 9048d744d36..23b9871b491 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/InstrumentationConfigUtilTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/InstrumentationConfigUtilTest.java @@ -9,7 +9,7 @@ import com.google.common.collect.ImmutableMap; import io.opentelemetry.sdk.extension.incubator.fileconfig.DeclarativeConfiguration; -import io.opentelemetry.sdk.extension.incubator.fileconfig.YamlDeclarativeConfigProperties; +import io.opentelemetry.sdk.internal.YamlDeclarativeConfigProperties; import io.opentelemetry.sdk.internal.SdkConfigProvider; import java.io.ByteArrayInputStream; import java.nio.charset.StandardCharsets; diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigPropertiesTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigPropertiesTest.java deleted file mode 100644 index d966e586a11..00000000000 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/MapBackedDeclarativeConfigPropertiesTest.java +++ /dev/null @@ -1,165 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.api.incubator.config; - -import static org.assertj.core.api.Assertions.assertThat; - -import io.opentelemetry.common.ComponentLoader; -import java.util.Arrays; -import java.util.Collections; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import org.junit.jupiter.api.Test; - -class MapBackedDeclarativeConfigPropertiesTest { - - private static final ComponentLoader COMPONENT_LOADER = - ComponentLoader.forClassLoader( - MapBackedDeclarativeConfigPropertiesTest.class.getClassLoader()); - - @Test - void getString() { - DeclarativeConfigProperties config = fromMap(mapOf("key", "value", "notString", 42)); - - assertThat(config.getString("key")).isEqualTo("value"); - assertThat(config.getString("missing")).isNull(); - assertThat(config.getString("notString")).isNull(); - } - - @Test - void getBoolean() { - DeclarativeConfigProperties config = fromMap(mapOf("key", true, "notBoolean", "true")); - - assertThat(config.getBoolean("key")).isTrue(); - assertThat(config.getBoolean("missing")).isNull(); - assertThat(config.getBoolean("notBoolean")).isNull(); - } - - @Test - void getInt() { - DeclarativeConfigProperties config = fromMap(mapOf("intVal", 42, "longVal", 100L, "str", "x")); - - assertThat(config.getInt("intVal")).isEqualTo(42); - assertThat(config.getInt("longVal")).isEqualTo(100); - assertThat(config.getInt("missing")).isNull(); - assertThat(config.getInt("str")).isNull(); - } - - @Test - void getLong() { - DeclarativeConfigProperties config = fromMap(mapOf("longVal", 100L, "intVal", 42, "str", "x")); - - assertThat(config.getLong("longVal")).isEqualTo(100L); - assertThat(config.getLong("intVal")).isEqualTo(42L); - assertThat(config.getLong("missing")).isNull(); - assertThat(config.getLong("str")).isNull(); - } - - @Test - void getDouble() { - DeclarativeConfigProperties config = - fromMap(mapOf("doubleVal", 3.14, "intVal", 42, "str", "x")); - - assertThat(config.getDouble("doubleVal")).isEqualTo(3.14); - assertThat(config.getDouble("intVal")).isEqualTo(42.0); - assertThat(config.getDouble("missing")).isNull(); - assertThat(config.getDouble("str")).isNull(); - } - - @Test - void getScalarList() { - DeclarativeConfigProperties config = - fromMap(mapOf("strings", Arrays.asList("a", "b"), "mixed", Arrays.asList("a", 1))); - - assertThat(config.getScalarList("strings", String.class)).containsExactly("a", "b"); - assertThat(config.getScalarList("mixed", String.class)).isNull(); - assertThat(config.getScalarList("missing", String.class)).isNull(); - } - - @Test - void getScalarList_nonListReturnsNull() { - DeclarativeConfigProperties config = fromMap(mapOf("notList", "value")); - - assertThat(config.getScalarList("notList", String.class)).isNull(); - } - - @Test - void getStructured() { - Map child = mapOf("nested", "value"); - DeclarativeConfigProperties config = fromMap(mapOf("child", child, "notMap", "scalar")); - - DeclarativeConfigProperties structured = config.getStructured("child"); - assertThat(structured).isNotNull(); - assertThat(structured.getString("nested")).isEqualTo("value"); - assertThat(config.getStructured("missing")).isNull(); - assertThat(config.getStructured("notMap")).isNull(); - } - - @Test - void getStructuredList() { - List> items = - Arrays.asList(mapOf("name", "first"), mapOf("name", "second")); - DeclarativeConfigProperties config = - fromMap(mapOf("items", items, "badItems", Arrays.asList("notAMap"))); - - List result = config.getStructuredList("items"); - assertThat(result).hasSize(2); - assertThat(result.get(0).getString("name")).isEqualTo("first"); - assertThat(result.get(1).getString("name")).isEqualTo("second"); - assertThat(config.getStructuredList("missing")).isNull(); - assertThat(config.getStructuredList("badItems")).isNull(); - } - - @Test - void getStructuredList_nonListReturnsNull() { - DeclarativeConfigProperties config = fromMap(mapOf("notList", "value")); - - assertThat(config.getStructuredList("notList")).isNull(); - } - - @Test - void getPropertyKeys() { - DeclarativeConfigProperties config = fromMap(mapOf("a", 1, "b", 2)); - - assertThat(config.getPropertyKeys()).containsExactlyInAnyOrder("a", "b"); - } - - @Test - void getPropertyKeys_empty() { - DeclarativeConfigProperties config = fromMap(Collections.emptyMap()); - - assertThat(config.getPropertyKeys()).isEmpty(); - } - - @Test - void getComponentLoader() { - DeclarativeConfigProperties config = fromMap(Collections.emptyMap()); - - assertThat(config.getComponentLoader()).isSameAs(COMPONENT_LOADER); - } - - @Test - void get_defaultMethod() { - Map child = mapOf("nested", "value"); - DeclarativeConfigProperties config = fromMap(mapOf("child", child)); - - assertThat(config.get("child").getString("nested")).isEqualTo("value"); - assertThat(config.get("missing").getPropertyKeys()).isEmpty(); - } - - private static DeclarativeConfigProperties fromMap(Map map) { - return DeclarativeConfigProperties.fromMap(map, COMPONENT_LOADER); - } - - private static Map mapOf(Object... entries) { - Map result = new LinkedHashMap<>(); - for (int i = 0; i < entries.length; i += 2) { - result.put((String) entries[i], entries[i + 1]); - } - return result; - } -} diff --git a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java index 7384eeb5d4d..ab4bb8d81fd 100644 --- a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java @@ -9,6 +9,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import io.opentelemetry.api.incubator.config.ConfigChangeRegistration; +import io.opentelemetry.api.incubator.config.DeclarativeConfigException; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; import io.opentelemetry.common.ComponentLoader; import java.util.ArrayList; @@ -384,6 +385,42 @@ void pathValidation_rejectsBrackets() { .isInstanceOf(IllegalArgumentException.class); } + @Test + void updateConfig_throwsOnSchemaConflict() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", "scalarValue")))); + + assertThatThrownBy( + () -> + provider.updateConfig( + ".instrumentation/development.general.http", config(mapOf("enabled", "true")))) + .isInstanceOf(DeclarativeConfigException.class) + .hasMessageContaining("general") + .hasMessageContaining("not a mapping"); + } + + @Test + void setConfigProperty_throwsOnSchemaConflict() { + SdkConfigProvider provider = + SdkConfigProvider.create( + config( + mapOf( + "instrumentation/development", + mapOf("general", "scalarValue")))); + + assertThatThrownBy( + () -> + provider.setConfigProperty( + ".instrumentation/development.general.http", "enabled", "true")) + .isInstanceOf(DeclarativeConfigException.class) + .hasMessageContaining("general") + .hasMessageContaining("not a mapping"); + } + @Test void updateConfig_rootPathReplacesEntireConfig() { SdkConfigProvider provider = diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java index 51ba03c8e33..6ad9dffcd96 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java @@ -18,6 +18,8 @@ import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.SamplerModel; +import io.opentelemetry.sdk.internal.ExtendedOpenTelemetrySdk; +import io.opentelemetry.sdk.internal.YamlDeclarativeConfigProperties; import io.opentelemetry.sdk.trace.samplers.Sampler; import java.io.Closeable; import java.io.IOException; diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigContextTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigContextTest.java index 71b9df870df..1580e4cfa9f 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigContextTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigContextTest.java @@ -12,6 +12,8 @@ import io.opentelemetry.api.incubator.config.DeclarativeConfigException; import io.opentelemetry.common.ComponentLoader; +import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper; +import io.opentelemetry.sdk.internal.YamlDeclarativeConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider; import io.opentelemetry.sdk.resources.Resource; import java.util.Collections; diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigPropertiesTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigPropertiesTest.java index e0443cd80b9..69ca700bad6 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigPropertiesTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigPropertiesTest.java @@ -13,6 +13,7 @@ import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel; +import io.opentelemetry.sdk.internal.YamlDeclarativeConfigProperties; import java.io.ByteArrayInputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java index 6dc1ca4ba6a..4a2262115e4 100644 --- a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java @@ -10,6 +10,7 @@ import io.opentelemetry.api.incubator.config.ConfigChangeListener; import io.opentelemetry.api.incubator.config.ConfigChangeRegistration; import io.opentelemetry.api.incubator.config.ConfigProvider; +import io.opentelemetry.api.incubator.config.DeclarativeConfigException; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; import java.util.HashMap; import java.util.List; @@ -36,7 +37,7 @@ public final class SdkConfigProvider implements ConfigProvider { private final AtomicReference openTelemetryConfigModel; private final ConcurrentMap> listenersByPath = new ConcurrentHashMap<>(); - private final AtomicBoolean disposed = new AtomicBoolean(false); + private final AtomicBoolean isShutdown = new AtomicBoolean(false); private SdkConfigProvider(DeclarativeConfigProperties openTelemetryConfigModel) { this.openTelemetryConfigModel = new AtomicReference<>(requireNonNull(openTelemetryConfigModel)); @@ -63,7 +64,7 @@ public ConfigChangeRegistration addConfigChangeListener( String path, ConfigChangeListener listener) { requireNonNull(listener, "listener"); String watchedPath = normalizeAndValidatePath(path); // fail fast on invalid path - if (disposed.get()) { + if (isShutdown.get()) { return NOOP_CHANGE_REGISTRATION; } @@ -71,7 +72,7 @@ public ConfigChangeRegistration addConfigChangeListener( listenersByPath .computeIfAbsent(watchedPath, unused -> new CopyOnWriteArrayList<>()) .add(registration); - if (disposed.get()) { + if (isShutdown.get()) { registration.close(); return NOOP_CHANGE_REGISTRATION; } @@ -82,16 +83,16 @@ public ConfigChangeRegistration addConfigChangeListener( public void updateConfig(String path, DeclarativeConfigProperties newSubtree) { requireNonNull(newSubtree, "newSubtree"); String normalizedPath = normalizeAndValidatePath(path); - if (disposed.get()) { + if (isShutdown.get()) { return; } Map subtreeMap = DeclarativeConfigProperties.toMap(newSubtree); while (true) { DeclarativeConfigProperties current = requireNonNull(openTelemetryConfigModel.get()); Map rootMap = DeclarativeConfigProperties.toMap(current); - setSubtreeAtPath(rootMap, normalizedPath, subtreeMap); + Map newRootMap = withSubtreeAtPath(rootMap, normalizedPath, subtreeMap); DeclarativeConfigProperties newRoot = - DeclarativeConfigProperties.fromMap(rootMap, current.getComponentLoader()); + YamlDeclarativeConfigProperties.create(newRootMap, current.getComponentLoader()); if (openTelemetryConfigModel.compareAndSet(current, newRoot)) { notifyListeners(current, newRoot); return; @@ -104,15 +105,15 @@ public void setConfigProperty(String path, String key, Object value) { requireNonNull(key, "key"); requireNonNull(value, "value"); String normalizedPath = normalizeAndValidatePath(path); - if (disposed.get()) { + if (isShutdown.get()) { return; } while (true) { DeclarativeConfigProperties current = requireNonNull(openTelemetryConfigModel.get()); Map rootMap = DeclarativeConfigProperties.toMap(current); - navigateToPath(rootMap, normalizedPath).put(key, value); + Map newRootMap = withPropertyAtPath(rootMap, normalizedPath, key, value); DeclarativeConfigProperties newRoot = - DeclarativeConfigProperties.fromMap(rootMap, current.getComponentLoader()); + YamlDeclarativeConfigProperties.create(newRootMap, current.getComponentLoader()); if (openTelemetryConfigModel.compareAndSet(current, newRoot)) { notifyListeners(current, newRoot); return; @@ -130,7 +131,7 @@ void updateOpenTelemetryConfigModel(DeclarativeConfigProperties updatedOpenTelem private void notifyListeners( DeclarativeConfigProperties previous, DeclarativeConfigProperties updated) { - if (disposed.get()) { + if (isShutdown.get()) { return; } @@ -150,7 +151,7 @@ private void notifyListeners( } void shutdown() { - if (!disposed.compareAndSet(false, true)) { + if (!isShutdown.compareAndSet(false, true)) { return; } for (List registrations : listenersByPath.values()) { @@ -161,52 +162,98 @@ void shutdown() { listenersByPath.clear(); } - @SuppressWarnings("unchecked") - private static void setSubtreeAtPath( + /** + * Returns a new map with the subtree at {@code normalizedPath} replaced by {@code subtreeMap}. + * Intermediate maps along the path are copied, not mutated. + */ + private static Map withSubtreeAtPath( Map rootMap, String normalizedPath, Map subtreeMap) { String relativePath = normalizedPath.substring(1); if (relativePath.isEmpty()) { - rootMap.clear(); - rootMap.putAll(subtreeMap); - return; + return new HashMap<>(subtreeMap); } String[] segments = relativePath.split("\\."); - Map parent = rootMap; - for (int i = 0; i < segments.length - 1; i++) { - Object child = parent.get(segments[i]); - if (child instanceof Map) { - parent = (Map) child; - } else { - Map newChild = new HashMap<>(); - parent.put(segments[i], newChild); - parent = newChild; - } - } - parent.put(segments[segments.length - 1], subtreeMap); + return copyAndReplace(rootMap, segments, 0, normalizedPath, subtreeMap); } - @SuppressWarnings("unchecked") - private static Map navigateToPath( - Map rootMap, String normalizedPath) { + /** + * Returns a new map with the property {@code key}={@code value} set within the map at {@code + * normalizedPath}. Intermediate maps along the path are copied, not mutated. + */ + private static Map withPropertyAtPath( + Map rootMap, String normalizedPath, String key, Object value) { String relativePath = normalizedPath.substring(1); if (relativePath.isEmpty()) { - return rootMap; + Map copy = new HashMap<>(rootMap); + copy.put(key, value); + return copy; } - Map current = rootMap; String[] segments = relativePath.split("\\."); + Map leafMap = resolveToMap(rootMap, segments, normalizedPath); + Map updatedLeaf = new HashMap<>(leafMap); + updatedLeaf.put(key, value); + return copyAndReplace(rootMap, segments, 0, normalizedPath, updatedLeaf); + } + + @SuppressWarnings("unchecked") + private static Map copyAndReplace( + Map current, + String[] segments, + int depth, + String normalizedPath, + Map replacement) { + Map copy = new HashMap<>(current); + String segment = segments[depth]; + if (depth == segments.length - 1) { + copy.put(segment, replacement); + return copy; + } + Object child = current.get(segment); + Map childMap; + if (child instanceof Map) { + childMap = (Map) child; + } else if (child == null) { + childMap = new HashMap<>(); + } else { + throw schemaConflict(normalizedPath, segment, child); + } + copy.put( + segment, copyAndReplace(childMap, segments, depth + 1, normalizedPath, replacement)); + return copy; + } + + @SuppressWarnings("unchecked") + private static Map resolveToMap( + Map rootMap, String[] segments, String normalizedPath) { + Map current = rootMap; for (String segment : segments) { Object child = current.get(segment); if (child instanceof Map) { current = (Map) child; + } else if (child == null) { + return new HashMap<>(); } else { - Map newChild = new HashMap<>(); - current.put(segment, newChild); - current = newChild; + throw schemaConflict(normalizedPath, segment, child); } } return current; } + private static DeclarativeConfigException schemaConflict( + String normalizedPath, String segment, Object actual) { + return new DeclarativeConfigException( + "Cannot traverse path '" + + normalizedPath + + "': segment '" + + segment + + "' resolves to a " + + actual.getClass().getSimpleName() + + ", not a mapping"); + } + + // TODO: optimize later, this is an expensive operation. + // But note that we only do this on a mutation, and these are expected to be infrquent + // so maybe acceptable private static boolean hasSameContents( DeclarativeConfigProperties left, DeclarativeConfigProperties right) { return DeclarativeConfigProperties.toMap(left).equals(DeclarativeConfigProperties.toMap(right)); diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigProperties.java b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/YamlDeclarativeConfigProperties.java similarity index 96% rename from sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigProperties.java rename to sdk/all/src/main/java/io/opentelemetry/sdk/internal/YamlDeclarativeConfigProperties.java index 99161dc01ba..d1bd4c9fc74 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigProperties.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/YamlDeclarativeConfigProperties.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.sdk.extension.incubator.fileconfig; +package io.opentelemetry.sdk.internal; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; @@ -25,16 +25,13 @@ import javax.annotation.Nullable; /** - * Implementation of {@link DeclarativeConfigProperties} which uses a file configuration model as a - * source. + * Implementation of {@link DeclarativeConfigProperties} backed by a parsed YAML/JSON map. * *

This class is internal and is hence not for public use. Its APIs are unstable and can change * at any time. * * @see #getStructured(String) Accessing nested maps * @see #getStructuredList(String) Accessing lists of maps - * @see DeclarativeConfiguration#toConfigProperties(Object, ComponentLoader) Converting - * configuration model to properties */ public final class YamlDeclarativeConfigProperties implements DeclarativeConfigProperties { @@ -70,8 +67,6 @@ private YamlDeclarativeConfigProperties( *

{@code properties} is expected to be the output of YAML parsing (i.e. with Jackson {@code * com.fasterxml.jackson.databind.ObjectMapper}), and have values which are scalars, lists of * scalars, lists of maps, and maps. - * - * @see DeclarativeConfiguration#toConfigProperties(Object) */ @SuppressWarnings("unchecked") public static YamlDeclarativeConfigProperties create( From f45b2bb135681e322392778f30ab9e72005d2c6b Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Thu, 30 Apr 2026 14:04:50 +0100 Subject: [PATCH 14/15] spotless --- .../config/InstrumentationConfigUtilTest.java | 2 +- .../sdk/internal/SdkConfigProviderTest.java | 10 ++-------- .../incubator/fileconfig/DeclarativeConfiguration.java | 1 - .../fileconfig/DeclarativeConfigContextTest.java | 3 +-- .../opentelemetry/sdk/internal/SdkConfigProvider.java | 3 +-- 5 files changed, 5 insertions(+), 14 deletions(-) diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/InstrumentationConfigUtilTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/InstrumentationConfigUtilTest.java index 23b9871b491..fa1be48872c 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/InstrumentationConfigUtilTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/config/InstrumentationConfigUtilTest.java @@ -9,8 +9,8 @@ import com.google.common.collect.ImmutableMap; import io.opentelemetry.sdk.extension.incubator.fileconfig.DeclarativeConfiguration; -import io.opentelemetry.sdk.internal.YamlDeclarativeConfigProperties; import io.opentelemetry.sdk.internal.SdkConfigProvider; +import io.opentelemetry.sdk.internal.YamlDeclarativeConfigProperties; import java.io.ByteArrayInputStream; import java.nio.charset.StandardCharsets; import java.util.Arrays; diff --git a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java index ab4bb8d81fd..5d38d8a4ea6 100644 --- a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java @@ -389,10 +389,7 @@ void pathValidation_rejectsBrackets() { void updateConfig_throwsOnSchemaConflict() { SdkConfigProvider provider = SdkConfigProvider.create( - config( - mapOf( - "instrumentation/development", - mapOf("general", "scalarValue")))); + config(mapOf("instrumentation/development", mapOf("general", "scalarValue")))); assertThatThrownBy( () -> @@ -407,10 +404,7 @@ void updateConfig_throwsOnSchemaConflict() { void setConfigProperty_throwsOnSchemaConflict() { SdkConfigProvider provider = SdkConfigProvider.create( - config( - mapOf( - "instrumentation/development", - mapOf("general", "scalarValue")))); + config(mapOf("instrumentation/development", mapOf("general", "scalarValue")))); assertThatThrownBy( () -> diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java index 6ad9dffcd96..80088222485 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java @@ -18,7 +18,6 @@ import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.SamplerModel; -import io.opentelemetry.sdk.internal.ExtendedOpenTelemetrySdk; import io.opentelemetry.sdk.internal.YamlDeclarativeConfigProperties; import io.opentelemetry.sdk.trace.samplers.Sampler; import java.io.Closeable; diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigContextTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigContextTest.java index 1580e4cfa9f..73beb10fb28 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigContextTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfigContextTest.java @@ -12,9 +12,8 @@ import io.opentelemetry.api.incubator.config.DeclarativeConfigException; import io.opentelemetry.common.ComponentLoader; -import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper; -import io.opentelemetry.sdk.internal.YamlDeclarativeConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider; +import io.opentelemetry.sdk.internal.YamlDeclarativeConfigProperties; import io.opentelemetry.sdk.resources.Resource; import java.util.Collections; import org.junit.jupiter.api.Test; diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java index 4a2262115e4..3948e785393 100644 --- a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java @@ -217,8 +217,7 @@ private static Map copyAndReplace( } else { throw schemaConflict(normalizedPath, segment, child); } - copy.put( - segment, copyAndReplace(childMap, segments, depth + 1, normalizedPath, replacement)); + copy.put(segment, copyAndReplace(childMap, segments, depth + 1, normalizedPath, replacement)); return copy; } From 8f150388fac8336e1022900ebe47ed6e048f0801 Mon Sep 17 00:00:00 2001 From: Jack Shirazi Date: Wed, 6 May 2026 18:10:41 +0100 Subject: [PATCH 15/15] Applied jack-berg changes --- .../api/incubator/config/ConfigProvider.java | 36 --- .../api/incubator/ConfigProviderTest.java | 15 - .../incubator/ExtendedOpenTelemetryTest.java | 3 +- .../sdk/internal/SdkConfigProviderTest.java | 168 ++++------- .../internal/ExtendedOpenTelemetrySdk.java | 10 - .../sdk/internal/SdkConfigProvider.java | 270 +++++++++++------- 6 files changed, 227 insertions(+), 275 deletions(-) diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java index 46554b21c93..8617ce6f7ca 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java @@ -87,42 +87,6 @@ default ConfigChangeRegistration addConfigChangeListener( return () -> {}; } - /** - * Updates the declarative configuration subtree at the given path. - * - *

The path uses {@code .} as a separator (e.g., {@code - * ".instrumentation/development.java.myLib"}). The subtree at that path is replaced with {@code - * newSubtree}, and any registered {@link ConfigChangeListener}s watching affected paths are - * notified. - * - *

The default implementation is a no-op. - * - * @param path the declarative configuration path to update - * @param newSubtree the new configuration subtree to set at the path - * @throws DeclarativeConfigException if an intermediate segment of the path resolves to a - * non-mapping value (e.g., a scalar or list), indicating a schema conflict - */ - default void updateConfig(String path, DeclarativeConfigProperties newSubtree) {} - - /** - * Sets a single scalar configuration property at the given path. - * - *

The path uses {@code .} as a separator (e.g., {@code - * ".instrumentation/development.java.myLib"}). The property identified by {@code key} within that - * path is set to {@code value}, and any registered {@link ConfigChangeListener}s watching - * affected paths are notified. - * - *

The default implementation is a no-op. - * - * @param path the declarative configuration path containing the property - * @param key the property key within the path - * @param value the new value for the property (must be a scalar: String, Boolean, Long, Double, - * Integer, or a List of scalars) - * @throws DeclarativeConfigException if an intermediate segment of the path resolves to a - * non-mapping value (e.g., a scalar or list), indicating a schema conflict - */ - default void setConfigProperty(String path, String key, Object value) {} - /** Returns a no-op {@link ConfigProvider}. */ static ConfigProvider noop() { return DeclarativeConfigProperties::empty; diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java index 496d2409595..43aa3fd0ba0 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java @@ -10,7 +10,6 @@ import io.opentelemetry.api.incubator.config.ConfigChangeRegistration; import io.opentelemetry.api.incubator.config.ConfigProvider; -import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; import org.junit.jupiter.api.Test; class ConfigProviderTest { @@ -32,18 +31,4 @@ void instrumentationConfigFallback() { ".instrumentation/development.java.servlet", (path, newConfig) -> {}); assertThatCode(listenerRegistration::close).doesNotThrowAnyException(); } - - @Test - void defaultUpdateConfig_isNoop() { - ConfigProvider configProvider = ConfigProvider.noop(); - assertThatCode(() -> configProvider.updateConfig(".foo", DeclarativeConfigProperties.empty())) - .doesNotThrowAnyException(); - } - - @Test - void defaultSetConfigProperty_isNoop() { - ConfigProvider configProvider = ConfigProvider.noop(); - assertThatCode(() -> configProvider.setConfigProperty(".foo", "key", "value")) - .doesNotThrowAnyException(); - } } diff --git a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ExtendedOpenTelemetryTest.java b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ExtendedOpenTelemetryTest.java index 792f10d0e4d..14a4c3c81db 100644 --- a/api/incubator/src/test/java/io/opentelemetry/api/incubator/ExtendedOpenTelemetryTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/api/incubator/ExtendedOpenTelemetryTest.java @@ -133,8 +133,7 @@ void close_shutsDownConfigProvider() { sdk.close(); - configProvider.setConfigProperty( - ".instrumentation/development.general.http", "enabled", "true"); + configProvider.setConfig(".instrumentation/development.general.http.enabled", "true"); assertThat(callbackCount.get()).isEqualTo(0); } diff --git a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java index 5d38d8a4ea6..6410e0cf077 100644 --- a/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java +++ b/api/incubator/src/test/java/io/opentelemetry/sdk/internal/SdkConfigProviderTest.java @@ -44,11 +44,9 @@ void addConfigChangeListener_notifiesOnWatchedPathChange() { ".instrumentation/development.general.http", (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); - provider.updateOpenTelemetryConfigModel( - config( - mapOf( - "instrumentation/development", - mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + provider.setConfig( + ".instrumentation/development", + config(mapOf("general", mapOf("http", mapOf("enabled", "true"))))); assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); registration.close(); @@ -71,24 +69,22 @@ void addConfigChangeListener_ignoresUnchangedAndNonWatchedUpdates() { ".instrumentation/development.general.http", (path, newConfig) -> callbackCount.incrementAndGet()); - provider.updateOpenTelemetryConfigModel( + provider.setConfig( + ".instrumentation/development", config( mapOf( - "instrumentation/development", - mapOf( - "general", - mapOf("http", mapOf("enabled", "true")), - "java", - mapOf("servlet", mapOf("enabled", "false")))))); - provider.updateOpenTelemetryConfigModel( + "general", + mapOf("http", mapOf("enabled", "true")), + "java", + mapOf("servlet", mapOf("enabled", "false"))))); + provider.setConfig( + ".instrumentation/development", config( mapOf( - "instrumentation/development", - mapOf( - "general", - mapOf("http", mapOf("enabled", "true")), - "java", - mapOf("servlet", mapOf("enabled", "false")))))); + "general", + mapOf("http", mapOf("enabled", "true")), + "java", + mapOf("servlet", mapOf("enabled", "false"))))); assertThat(callbackCount).hasValue(0); } @@ -106,8 +102,7 @@ void addConfigChangeListener_returnsEmptyNodeWhenWatchedPathCleared() { ".instrumentation/development.general.http", (path, newConfig) -> propertyKeysSeen.add(newConfig.getPropertyKeys())); - provider.updateOpenTelemetryConfigModel( - config(mapOf("instrumentation/development", mapOf("general", mapOf())))); + provider.setConfig(".instrumentation/development", config(mapOf("general", mapOf()))); assertThat(propertyKeysSeen).containsExactly(Collections.emptySet()); } @@ -128,22 +123,18 @@ void addConfigChangeListener_closeAndShutdownStopCallbacks() { registration.close(); registration.close(); - provider.updateOpenTelemetryConfigModel( - config( - mapOf( - "instrumentation/development", - mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + provider.setConfig( + ".instrumentation/development", + config(mapOf("general", mapOf("http", mapOf("enabled", "true"))))); assertThat(callbackCount).hasValue(0); provider.addConfigChangeListener( ".instrumentation/development.general.http", (path, newConfig) -> callbackCount.incrementAndGet()); provider.shutdown(); - provider.updateOpenTelemetryConfigModel( - config( - mapOf( - "instrumentation/development", - mapOf("general", mapOf("http", mapOf("enabled", "false")))))); + provider.setConfig( + ".instrumentation/development", + config(mapOf("general", mapOf("http", mapOf("enabled", "false"))))); assertThat(callbackCount).hasValue(0); } @@ -165,17 +156,15 @@ void addConfigChangeListener_listenerExceptionIsIsolated() { ".instrumentation/development.general.http", (path, newConfig) -> successfulCallbacks.incrementAndGet()); - provider.updateOpenTelemetryConfigModel( - config( - mapOf( - "instrumentation/development", - mapOf("general", mapOf("http", mapOf("enabled", "true")))))); + provider.setConfig( + ".instrumentation/development", + config(mapOf("general", mapOf("http", mapOf("enabled", "true"))))); assertThat(successfulCallbacks).hasValue(1); } @Test - void updateConfig_replacesSubtreeAndNotifiesListener() { + void setConfig_replacesSubtreeAndNotifiesListener() { SdkConfigProvider provider = SdkConfigProvider.create( config( @@ -187,14 +176,14 @@ void updateConfig_replacesSubtreeAndNotifiesListener() { ".instrumentation/development.general.http", (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); - provider.updateConfig( + provider.setConfig( ".instrumentation/development.general.http", config(mapOf("enabled", "true"))); assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); } @Test - void updateConfig_doesNotNotifyWhenSubtreeUnchanged() { + void setConfig_doesNotNotifyWhenSubtreeUnchanged() { SdkConfigProvider provider = SdkConfigProvider.create( config( @@ -206,28 +195,28 @@ void updateConfig_doesNotNotifyWhenSubtreeUnchanged() { ".instrumentation/development.general.http", (path, newConfig) -> callbackCount.incrementAndGet()); - provider.updateConfig( + provider.setConfig( ".instrumentation/development.general.http", config(mapOf("enabled", "true"))); assertThat(callbackCount).hasValue(0); } @Test - void updateConfig_createsIntermediateNodesIfMissing() { + void setConfig_createsIntermediateNodesIfMissing() { SdkConfigProvider provider = SdkConfigProvider.create(config(mapOf())); List notifications = new ArrayList<>(); provider.addConfigChangeListener( ".instrumentation/development.general.http", (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); - provider.updateConfig( + provider.setConfig( ".instrumentation/development.general.http", config(mapOf("enabled", "true"))); assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); } @Test - void updateConfig_noopWhenDisposed() { + void setConfig_noopWhenDisposed() { SdkConfigProvider provider = SdkConfigProvider.create( config( @@ -240,14 +229,15 @@ void updateConfig_noopWhenDisposed() { (path, newConfig) -> callbackCount.incrementAndGet()); provider.shutdown(); - provider.updateConfig( + provider.setConfig( ".instrumentation/development.general.http", config(mapOf("enabled", "true"))); + provider.setConfig(".instrumentation/development.general.http.enabled", "true"); assertThat(callbackCount).hasValue(0); } @Test - void setConfigProperty_setsScalarAndNotifiesListener() { + void setConfig_setsValueAndNotifiesListener() { SdkConfigProvider provider = SdkConfigProvider.create( config( @@ -259,13 +249,13 @@ void setConfigProperty_setsScalarAndNotifiesListener() { ".instrumentation/development.general.http", (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); - provider.setConfigProperty(".instrumentation/development.general.http", "enabled", "true"); + provider.setConfig(".instrumentation/development.general.http.enabled", "true"); assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); } @Test - void setConfigProperty_doesNotNotifyWhenValueUnchanged() { + void setConfig_doesNotNotifyWhenValueUnchanged() { SdkConfigProvider provider = SdkConfigProvider.create( config( @@ -277,26 +267,7 @@ void setConfigProperty_doesNotNotifyWhenValueUnchanged() { ".instrumentation/development.general.http", (path, newConfig) -> callbackCount.incrementAndGet()); - provider.setConfigProperty(".instrumentation/development.general.http", "enabled", "true"); - - assertThat(callbackCount).hasValue(0); - } - - @Test - void setConfigProperty_noopWhenDisposed() { - SdkConfigProvider provider = - SdkConfigProvider.create( - config( - mapOf( - "instrumentation/development", - mapOf("general", mapOf("http", mapOf("enabled", "false")))))); - AtomicInteger callbackCount = new AtomicInteger(); - provider.addConfigChangeListener( - ".instrumentation/development.general.http", - (path, newConfig) -> callbackCount.incrementAndGet()); - provider.shutdown(); - - provider.setConfigProperty(".instrumentation/development.general.http", "enabled", "true"); + provider.setConfig(".instrumentation/development.general.http.enabled", "true"); assertThat(callbackCount).hasValue(0); } @@ -326,8 +297,8 @@ void concurrentUpdates_allChangesAreApplied() throws Exception { () -> { try { startLatch.await(); - provider.setConfigProperty( - ".instrumentation/development.general.http", "count", String.valueOf(index)); + provider.setConfig( + ".instrumentation/development.general.http.count", String.valueOf(index)); } catch (InterruptedException e) { Thread.currentThread().interrupt(); } finally { @@ -355,9 +326,9 @@ void pathValidation_rejectsMissingLeadingDot() { assertThatThrownBy( () -> provider.addConfigChangeListener("instrumentation", (path, newConfig) -> {})) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> provider.updateConfig("instrumentation", config(mapOf()))) + assertThatThrownBy(() -> provider.setConfig("instrumentation.subtree", config(mapOf()))) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> provider.setConfigProperty("instrumentation", "key", "value")) + assertThatThrownBy(() -> provider.setConfig("instrumentation.key", "value")) .isInstanceOf(IllegalArgumentException.class); } @@ -367,9 +338,9 @@ void pathValidation_rejectsWildcards() { assertThatThrownBy(() -> provider.addConfigChangeListener(".*", (path, newConfig) -> {})) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> provider.updateConfig(".foo.*", config(mapOf()))) + assertThatThrownBy(() -> provider.setConfig(".foo.*.subtree", config(mapOf()))) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> provider.setConfigProperty(".foo.*", "key", "value")) + assertThatThrownBy(() -> provider.setConfig(".foo.*.key", "value")) .isInstanceOf(IllegalArgumentException.class); } @@ -379,65 +350,42 @@ void pathValidation_rejectsBrackets() { assertThatThrownBy(() -> provider.addConfigChangeListener(".foo[0]", (path, newConfig) -> {})) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> provider.updateConfig(".foo[0]", config(mapOf()))) + assertThatThrownBy(() -> provider.setConfig(".foo[0].subtree", config(mapOf()))) .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> provider.setConfigProperty(".foo[0]", "key", "value")) + assertThatThrownBy(() -> provider.setConfig(".foo[0].key", "value")) .isInstanceOf(IllegalArgumentException.class); } @Test - void updateConfig_throwsOnSchemaConflict() { + void pathValidation_rejectsRootOnlyPath() { + SdkConfigProvider provider = SdkConfigProvider.create(config(mapOf())); + + assertThatThrownBy(() -> provider.setConfig(".", config(mapOf()))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("key segment"); + } + + @Test + void setConfig_throwsOnSchemaConflict() { SdkConfigProvider provider = SdkConfigProvider.create( config(mapOf("instrumentation/development", mapOf("general", "scalarValue")))); assertThatThrownBy( () -> - provider.updateConfig( + provider.setConfig( ".instrumentation/development.general.http", config(mapOf("enabled", "true")))) .isInstanceOf(DeclarativeConfigException.class) .hasMessageContaining("general") .hasMessageContaining("not a mapping"); - } - - @Test - void setConfigProperty_throwsOnSchemaConflict() { - SdkConfigProvider provider = - SdkConfigProvider.create( - config(mapOf("instrumentation/development", mapOf("general", "scalarValue")))); assertThatThrownBy( - () -> - provider.setConfigProperty( - ".instrumentation/development.general.http", "enabled", "true")) + () -> provider.setConfig(".instrumentation/development.general.http.enabled", "true")) .isInstanceOf(DeclarativeConfigException.class) .hasMessageContaining("general") .hasMessageContaining("not a mapping"); } - @Test - void updateConfig_rootPathReplacesEntireConfig() { - SdkConfigProvider provider = - SdkConfigProvider.create( - config( - mapOf( - "instrumentation/development", - mapOf("general", mapOf("http", mapOf("enabled", "false")))))); - List notifications = new ArrayList<>(); - provider.addConfigChangeListener( - ".instrumentation/development.general.http", - (path, newConfig) -> notifications.add(path + "=" + newConfig.getString("enabled"))); - - provider.updateConfig( - ".", - config( - mapOf( - "instrumentation/development", - mapOf("general", mapOf("http", mapOf("enabled", "true")))))); - - assertThat(notifications).containsExactly(".instrumentation/development.general.http=true"); - } - private static DeclarativeConfigProperties config(Map root) { return new MapBackedDeclarativeConfigProperties(root); } diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java index 9a72679103f..b05b507ef05 100644 --- a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/ExtendedOpenTelemetrySdk.java @@ -95,16 +95,6 @@ public ConfigChangeRegistration addConfigChangeListener( return delegate.addConfigChangeListener(path, listener); } - @Override - public void updateConfig(String path, DeclarativeConfigProperties newSubtree) { - delegate.updateConfig(path, newSubtree); - } - - @Override - public void setConfigProperty(String path, String key, Object value) { - delegate.setConfigProperty(path, key, value); - } - private SdkConfigProvider unobfuscate() { return delegate; } diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java index 3948e785393..0e0c797092a 100644 --- a/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java +++ b/sdk/all/src/main/java/io/opentelemetry/sdk/internal/SdkConfigProvider.java @@ -12,6 +12,7 @@ import io.opentelemetry.api.incubator.config.ConfigProvider; import io.opentelemetry.api.incubator.config.DeclarativeConfigException; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -19,7 +20,6 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; @@ -34,13 +34,14 @@ public final class SdkConfigProvider implements ConfigProvider { private static final Logger logger = Logger.getLogger(SdkConfigProvider.class.getName()); private static final ConfigChangeRegistration NOOP_CHANGE_REGISTRATION = () -> {}; - private final AtomicReference openTelemetryConfigModel; + private final Object lock = new Object(); + private volatile DeclarativeConfigProperties openTelemetryConfigModel; private final ConcurrentMap> listenersByPath = new ConcurrentHashMap<>(); private final AtomicBoolean isShutdown = new AtomicBoolean(false); private SdkConfigProvider(DeclarativeConfigProperties openTelemetryConfigModel) { - this.openTelemetryConfigModel = new AtomicReference<>(requireNonNull(openTelemetryConfigModel)); + this.openTelemetryConfigModel = requireNonNull(openTelemetryConfigModel); } /** @@ -56,79 +57,76 @@ public static SdkConfigProvider create(DeclarativeConfigProperties openTelemetry @Override public DeclarativeConfigProperties getInstrumentationConfig() { - return requireNonNull(openTelemetryConfigModel.get()).get("instrumentation/development"); + return openTelemetryConfigModel.get("instrumentation/development"); } @Override public ConfigChangeRegistration addConfigChangeListener( String path, ConfigChangeListener listener) { requireNonNull(listener, "listener"); - String watchedPath = normalizeAndValidatePath(path); // fail fast on invalid path + String watchedPath = normalizeAndValidatePath(path); if (isShutdown.get()) { return NOOP_CHANGE_REGISTRATION; } - - ListenerRegistration registration = new ListenerRegistration(watchedPath, listener); - listenersByPath - .computeIfAbsent(watchedPath, unused -> new CopyOnWriteArrayList<>()) - .add(registration); - if (isShutdown.get()) { - registration.close(); - return NOOP_CHANGE_REGISTRATION; - } - return registration; - } - - @Override - public void updateConfig(String path, DeclarativeConfigProperties newSubtree) { - requireNonNull(newSubtree, "newSubtree"); - String normalizedPath = normalizeAndValidatePath(path); - if (isShutdown.get()) { - return; - } - Map subtreeMap = DeclarativeConfigProperties.toMap(newSubtree); - while (true) { - DeclarativeConfigProperties current = requireNonNull(openTelemetryConfigModel.get()); - Map rootMap = DeclarativeConfigProperties.toMap(current); - Map newRootMap = withSubtreeAtPath(rootMap, normalizedPath, subtreeMap); - DeclarativeConfigProperties newRoot = - YamlDeclarativeConfigProperties.create(newRootMap, current.getComponentLoader()); - if (openTelemetryConfigModel.compareAndSet(current, newRoot)) { - notifyListeners(current, newRoot); - return; + synchronized (lock) { + if (isShutdown.get()) { + return NOOP_CHANGE_REGISTRATION; } + ListenerRegistration registration = new ListenerRegistration(watchedPath, listener); + listenersByPath + .computeIfAbsent(watchedPath, unused -> new CopyOnWriteArrayList<>()) + .add(registration); + return registration; } } - @Override - public void setConfigProperty(String path, String key, Object value) { - requireNonNull(key, "key"); + /** + * Sets the configuration value at the given path. + * + *

The path uses {@code .} as a separator. The final segment is the key to set within the + * parent mapping. For example, {@code + * setConfig(".instrumentation/development.java.myLib.enabled", true)} sets the {@code enabled} + * key within the {@code .instrumentation/development.java.myLib} mapping. + * + *

The value may be a String, Boolean, Long, Double, Integer, {@link + * DeclarativeConfigProperties}, or a List whose elements are any of those types. + * + *

If a value already exists at the path, its type must not change. + * + * @param path the full declarative configuration path, including the key to set + * @param value the new value + * @throws IllegalArgumentException if the path does not include a key segment beyond the root, or + * if the value type is not supported + * @throws DeclarativeConfigException if the path traverses a non-mapping value, or if the + * existing value's type would change + */ + public void setConfig(String path, Object value) { requireNonNull(value, "value"); + validateValue(value); + Object normalizedValue = normalizeValue(value); String normalizedPath = normalizeAndValidatePath(path); + int lastDot = normalizedPath.lastIndexOf('.'); + String parentPath = lastDot == 0 ? "." : normalizedPath.substring(0, lastDot); + String key = normalizedPath.substring(lastDot + 1); + if (key.isEmpty()) { + throw new IllegalArgumentException( + "setConfig path must include a key segment beyond the root: " + path); + } if (isShutdown.get()) { return; } - while (true) { - DeclarativeConfigProperties current = requireNonNull(openTelemetryConfigModel.get()); - Map rootMap = DeclarativeConfigProperties.toMap(current); - Map newRootMap = withPropertyAtPath(rootMap, normalizedPath, key, value); - DeclarativeConfigProperties newRoot = + synchronized (lock) { + DeclarativeConfigProperties current = openTelemetryConfigModel; + Map currentRootMap = DeclarativeConfigProperties.toMap(current); + validateTypeUnchanged(currentRootMap, parentPath, key, normalizedValue, normalizedPath); + Map newRootMap = + withValueAtPath(currentRootMap, parentPath, key, normalizedValue, normalizedPath); + openTelemetryConfigModel = YamlDeclarativeConfigProperties.create(newRootMap, current.getComponentLoader()); - if (openTelemetryConfigModel.compareAndSet(current, newRoot)) { - notifyListeners(current, newRoot); - return; - } + notifyListeners(current, openTelemetryConfigModel); } } - // Visible for testing. - void updateOpenTelemetryConfigModel(DeclarativeConfigProperties updatedOpenTelemetryConfigModel) { - requireNonNull(updatedOpenTelemetryConfigModel, "updatedOpenTelemetryConfigModel"); - DeclarativeConfigProperties previous = - openTelemetryConfigModel.getAndSet(updatedOpenTelemetryConfigModel); - notifyListeners(previous, updatedOpenTelemetryConfigModel); - } - private void notifyListeners( DeclarativeConfigProperties previous, DeclarativeConfigProperties updated) { if (isShutdown.get()) { @@ -151,48 +149,32 @@ private void notifyListeners( } void shutdown() { - if (!isShutdown.compareAndSet(false, true)) { - return; - } - for (List registrations : listenersByPath.values()) { - for (ListenerRegistration registration : registrations) { - registration.close(); + synchronized (lock) { + if (!isShutdown.compareAndSet(false, true)) { + return; } + listenersByPath.clear(); } - listenersByPath.clear(); } /** - * Returns a new map with the subtree at {@code normalizedPath} replaced by {@code subtreeMap}. + * Returns a new map with {@code key}={@code value} set within the map at {@code parentPath}. * Intermediate maps along the path are copied, not mutated. */ - private static Map withSubtreeAtPath( - Map rootMap, String normalizedPath, Map subtreeMap) { - String relativePath = normalizedPath.substring(1); - if (relativePath.isEmpty()) { - return new HashMap<>(subtreeMap); - } - String[] segments = relativePath.split("\\."); - return copyAndReplace(rootMap, segments, 0, normalizedPath, subtreeMap); - } - - /** - * Returns a new map with the property {@code key}={@code value} set within the map at {@code - * normalizedPath}. Intermediate maps along the path are copied, not mutated. - */ - private static Map withPropertyAtPath( - Map rootMap, String normalizedPath, String key, Object value) { - String relativePath = normalizedPath.substring(1); + private static Map withValueAtPath( + Map rootMap, String parentPath, String key, Object value, String fullPath) { + String relativePath = parentPath.substring(1); if (relativePath.isEmpty()) { Map copy = new HashMap<>(rootMap); copy.put(key, value); return copy; } + // TODO: this can be done in a single traversal rather than resolving the leaf then replacing String[] segments = relativePath.split("\\."); - Map leafMap = resolveToMap(rootMap, segments, normalizedPath); + Map leafMap = resolveToMap(rootMap, segments, fullPath); Map updatedLeaf = new HashMap<>(leafMap); updatedLeaf.put(key, value); - return copyAndReplace(rootMap, segments, 0, normalizedPath, updatedLeaf); + return copyAndReplace(rootMap, segments, 0, fullPath, updatedLeaf); } @SuppressWarnings("unchecked") @@ -246,7 +228,7 @@ private static DeclarativeConfigException schemaConflict( + "': segment '" + segment + "' resolves to a " - + actual.getClass().getSimpleName() + + typeName(actual) + ", not a mapping"); } @@ -276,19 +258,108 @@ private static DeclarativeConfigProperties resolvePath( return current; } + private static void validateTypeUnchanged( + Map rootMap, + String parentPath, + String key, + Object newValue, + String fullPath) { + String relativePath = parentPath.substring(1); + Map leafMap = + relativePath.isEmpty() + ? rootMap + : resolveToMap(rootMap, relativePath.split("\\."), fullPath); + Object existing = leafMap.get(key); + if (existing == null) { + return; // key doesn't exist yet, any type is allowed + } + boolean typeMismatch; + if (existing instanceof Map) { + typeMismatch = !(newValue instanceof Map); + } else if (existing instanceof List) { + typeMismatch = !(newValue instanceof List); + } else { + typeMismatch = !existing.getClass().equals(newValue.getClass()); + } + if (typeMismatch) { + throw new DeclarativeConfigException( + "Cannot change type at path '" + + fullPath + + "' from " + + typeName(existing) + + " to " + + typeName(newValue)); + } + } + + private static void validateValue(Object value) { + if (value instanceof String + || value instanceof Boolean + || value instanceof Long + || value instanceof Double + || value instanceof Integer + || value instanceof DeclarativeConfigProperties) { + return; + } + if (value instanceof List) { + for (Object element : (List) value) { + if (!(element instanceof String) + && !(element instanceof Boolean) + && !(element instanceof Long) + && !(element instanceof Double) + && !(element instanceof Integer) + && !(element instanceof DeclarativeConfigProperties)) { + throw new IllegalArgumentException( + "setConfig list value elements must be String, Boolean, Long, Double, Integer, or" + + " DeclarativeConfigProperties, got: " + + (element == null ? "null" : element.getClass().getName())); + } + } + return; + } + throw new IllegalArgumentException( + "setConfig value must be a String, Boolean, Long, Double, Integer," + + " DeclarativeConfigProperties, or List thereof, got: " + + value.getClass().getName()); + } + + private static Object normalizeValue(Object value) { + if (value instanceof DeclarativeConfigProperties) { + return DeclarativeConfigProperties.toMap((DeclarativeConfigProperties) value); + } + if (!(value instanceof List)) { + return value; + } + List normalized = new ArrayList<>(); + for (Object element : (List) value) { + normalized.add( + element instanceof DeclarativeConfigProperties + ? DeclarativeConfigProperties.toMap((DeclarativeConfigProperties) element) + : element); + } + return normalized; + } + + private static String typeName(Object value) { + if (value instanceof Map) { + return "mapping"; + } + if (value instanceof List) { + return "list"; + } + return value.getClass().getSimpleName(); + } + private static String normalizeAndValidatePath(String path) { String watchedPath = requireNonNull(path, "path").trim(); if (!watchedPath.startsWith(".")) { - throw new IllegalArgumentException( - "Config change listener path must be absolute and start with '.': " + path); + throw new IllegalArgumentException("Path must be absolute and start with '.': " + path); } if (watchedPath.indexOf('*') >= 0) { - throw new IllegalArgumentException( - "Config change listener path does not support wildcards: " + path); + throw new IllegalArgumentException("Path does not support wildcards: " + path); } if (watchedPath.indexOf('[') >= 0 || watchedPath.indexOf(']') >= 0) { - throw new IllegalArgumentException( - "Config change listener path does not support sequence indexing: " + path); + throw new IllegalArgumentException("Path does not support sequence indexing: " + path); } return watchedPath; } @@ -296,7 +367,6 @@ private static String normalizeAndValidatePath(String path) { private final class ListenerRegistration implements ConfigChangeRegistration { private final String watchedPath; private final ConfigChangeListener listener; - private final AtomicBoolean closed = new AtomicBoolean(false); private ListenerRegistration(String watchedPath, ConfigChangeListener listener) { this.watchedPath = watchedPath; @@ -305,23 +375,19 @@ private ListenerRegistration(String watchedPath, ConfigChangeListener listener) @Override public void close() { - if (!closed.compareAndSet(false, true)) { - return; - } - CopyOnWriteArrayList registrations = listenersByPath.get(watchedPath); - if (registrations == null) { - return; - } - registrations.remove(this); - if (registrations.isEmpty()) { - listenersByPath.remove(watchedPath, registrations); + synchronized (lock) { + CopyOnWriteArrayList registrations = listenersByPath.get(watchedPath); + if (registrations == null) { + return; + } + registrations.remove(this); + if (registrations.isEmpty()) { + listenersByPath.remove(watchedPath, registrations); + } } } private void notifyChange(String changedPath, DeclarativeConfigProperties updatedConfigAtPath) { - if (closed.get()) { - return; - } try { listener.onChange(changedPath, updatedConfigAtPath); } catch (Throwable throwable) {