Skip to content

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

Open
cleverchuk wants to merge 7 commits into
mainfrom
cc/NH-143594
Open

New - Support distribution.solarwinds config node with per-key merging#549
cleverchuk wants to merge 7 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.

* across languages in a multi-language deployment) and/or under the language-specific {@code
* instrumentation/development.java.solarwinds} node. When both are present they are merged per key:
* the distribution node takes precedence for any key it defines and the instrumentation node fills
* in the rest. A present-but-empty node is treated as absent.
Comment thread sdk-config.yaml
- name: service.name
value: swi-test-app

# Provide shared config for multi-langauge deployment
- name: test.app
value: java-apm-smoke-test-v2

# Provide shared config for multi-langauge deployment
Comment thread sdk-config.yaml
Comment on lines 34 to 36
tracer_provider:
processors:

Comment thread sdk-config.yaml
Comment on lines 38 to 40
meter_provider:
readers:

Comment thread sdk-config.yaml
Comment on lines 42 to 44
logger_provider:
processors:

Comment on lines +139 to +140
// distribution.solarwinds is present but has no properties, so it is skipped; configuration is
// still loaded from the instrumentation node.
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.

2 participants