Skip to content

New - Support distribution.solarwinds config node with per-key merging - #549

Merged
cleverchuk merged 10 commits into
mainfrom
cc/NH-143594
Jul 23, 2026
Merged

New - Support distribution.solarwinds config node with per-key merging#549
cleverchuk merged 10 commits into
mainfrom
cc/NH-143594

Conversation

@cleverchuk

Copy link
Copy Markdown
Contributor

Summary

The agent can now read its configuration from the top-level distribution.solarwinds node in
sdk-config.yaml, making it usable in multi-language deployments where shared settings live outside
the Java-specific instrumentation/development.java.solarwinds node. When both nodes are present,
their keys are merged: distribution wins for any key it defines, and the instrumentation node
fills in the rest. The resolution logic is centralized in a new SolarwindsConfigResolver class
consumed by both DeclarativeLoader and SharedConfigCustomizerProvider. A flaky JDBC
instrumentation test is also fixed in this branch.


Changes

DeclarativeLoader — new integration point

DeclarativeLoader previously implemented BeforeAgentListener and obtained its config via
ExtendedOpenTelemetry.getInstrumentationConfig("solarwinds"), which only ever resolved the
instrumentation/development.java.solarwinds node. It now implements
DeclarativeConfigurationCustomizerProvider and registers a model customizer, which gives it direct
access to the full config tree and lets it resolve both nodes before the SDK is fully initialized.

SolarwindsConfigResolver — centralized resolution with merging

Both DeclarativeLoader and SharedConfigCustomizerProvider previously duplicated the node
traversal logic (getStructured("distribution")…). The new SolarwindsConfigResolver class in
libs/shared is the single place that knows how to locate and combine the two nodes.

Resolution rules:

  • Only distribution.solarwinds present → use it directly.
  • Only instrumentation/development.java.solarwinds present → use it directly.
  • Both present → return a MergedConfigProperties view that resolves each key from distribution
    first and falls back to instrumentation for keys not defined there.
  • Neither present → return null (callers wrap this in Objects.requireNonNull).

MergedConfigProperties is a private inner class that implements DeclarativeConfigProperties by
delegating every getter to the primary node and falling back to the secondary node when the primary
returns null. getPropertyKeys() returns the union of both nodes' key sets. An empty node
(present but no keys) is honored — it defers every key to the other node rather than blocking it.

sdk-config.yaml updates

Shared settings (agent.serviceKey, agent.collector, agent.logging) are moved from
instrumentation/development.java.solarwinds into the new distribution.solarwinds block in both
the root and smoke-test config files, reflecting the intended multi-language sharing model. Empty
stubs for tracer_provider.processors, meter_provider.readers, and logger_provider.processors
are added for discoverability.

Thread name in APM log lines

The internal logger format now includes [thread-name] between the log level and the message,
making it easier to correlate agent-internal log output with application thread activity.

Flaky JDBC test fix

JdbcInstrumentationTest was flaky because:

  1. The MySQL container was recreated per test, causing nondeterministic container-startup spans to
    leak into subsequent assertions.
  2. Assertions used waitAndAssertTraces, which pinned the exact number and grouping of traces,
    while the JDBC driver emits a variable number of setup statements on connection open.

The fix makes the container a static field (started once for the class) and calls
testing.clearData() after the connection is opened to discard driver-setup spans before the code
under test runs. Assertions are rewritten to poll testing.spans() with Awaitility and check only
that a root INTERNAL span carrying sw.query_tag was exported, without constraining the rest of
the trace structure.

Test services data

  1. e-1712644058766987264
  2. e-1712643928659124224
  3. e-1742334541200846848
  4. e-1777406072376840192

Copilot AI review requested due to automatic review settings July 22, 2026 20:21
@cleverchuk
cleverchuk requested review from a team as code owners July 22, 2026 20:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for reading SolarWinds agent configuration from a new top-level distribution.solarwinds node in sdk-config.yaml, with per-key merging against the existing instrumentation/development.java.solarwinds node (distribution takes precedence per key). The resolution logic is centralized in a new SolarwindsConfigResolver, and both the agent bootstrap/customization path and shared config customizer are updated to use it; additionally, logging output is enhanced to include thread name and a flaky JDBC test is stabilized.

Changes:

  • Introduce SolarwindsConfigResolver to resolve and merge distribution.solarwinds + instrumentation/development.java.solarwinds.
  • Update DeclarativeLoader and SharedConfigCustomizerProvider to use the centralized resolver.
  • Update example/smoke YAML configs, enhance internal log formatting, and stabilize JDBC instrumentation tests.

Reviewed changes

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

Show a summary per file
File Description
smoke-tests/sdk-config.yaml Moves shared SolarWinds settings into distribution.solarwinds for smoke tests.
sdk-config.yaml Adds distribution.solarwinds to the root example config and adds placeholder sections.
libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProviderTest.java Adds coverage for reading/precedence/merge behavior across distribution + instrumentation nodes.
libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolver.java New centralized resolver implementing per-key merge semantics.
libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java Switches to centralized resolver instead of hard-coded instrumentation-node traversal.
libs/logging/src/main/java/com/solarwinds/joboe/logging/Logger.java Updates log label formatting to include thread name.
libs/core/src/main/java/com/solarwinds/joboe/core/rpc/RpcClient.java Minor refactor to a method reference for async init submission.
instrumentation/jdbc/javaagent/src/test/java/com/solarwinds/opentelemetry/instrumentation/JdbcInstrumentationTest.java Stabilizes test by reusing container and asserting via span polling instead of exact trace structure.
custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java Updates tests to match new declarative customization integration and distribution-node support.
custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java Switches to DeclarativeConfigurationCustomizerProvider and resolves SolarWinds config via the new resolver.

Comment thread sdk-config.yaml Outdated
Comment thread smoke-tests/sdk-config.yaml Outdated
Comment thread sdk-config.yaml
Comment thread sdk-config.yaml
Comment thread sdk-config.yaml
cleverchuk and others added 4 commits July 22, 2026 16:31
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@jerrytfleung jerrytfleung left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. Just left a question about the merge logic.


@Override
public ComponentLoader getComponentLoader() {
return primary.getComponentLoader();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it true that primary and fallback have the same ComponentLoader such that we don't need to consider the fallback one while merging?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. They belong to the same class loader.

@cleverchuk
cleverchuk merged commit 7f7590a into main Jul 23, 2026
17 checks passed
@cleverchuk
cleverchuk deleted the cc/NH-143594 branch July 23, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants