Skip to content

[OpAMP] Add remote config settings and ensure correct capabilities#3618

Merged
Kielek merged 4 commits into
open-telemetry:mainfrom
stevejgordon:opamp-remote-config-settings
Dec 19, 2025
Merged

[OpAMP] Add remote config settings and ensure correct capabilities#3618
Kielek merged 4 commits into
open-telemetry:mainfrom
stevejgordon:opamp-remote-config-settings

Conversation

@stevejgordon
Copy link
Copy Markdown
Contributor

Fixes #3617

Changes

  • Introduce RemoteConfigSettings to allow configuring remote configuration support in the OpAMP client.
  • Update capability advertisement logic to include AcceptsRemoteConfig when enabled.
  • Add and update tests to verify capability flags and heartbeat behavior.
  • Update unshipped public API.

Design notes

For this initial PR, this exposes a single property on RemoteConfigSettings to identify that the client can handle remote configuration. The intent is that this can be later expanded once we expose APIs for sending the effictive config and remote config status.

I've also updated the advertised capabilities to not set the health or heartbeat flags when heartbeat is disabled based on an assumption of how that should work. I believe we should only advertise features we know the client implements (and are enabled) or consumers implement by confguring the client and registering subscribers.

I also wonder if we should avoid dispatching the RemoteConfigMessage to listeners if this feature is disabled. In theory, we should never receive it from the server unless we advertise the capability, but we could take a precaution to drop the message should it somehow arrive. This only really matters if for some reason a listener is registered for RemoteConfigMessage but the feature is disabled in the settings.

Alternative design to consider

We may want to consider avoiding this extra RemoteConfigSettings type entirely and instead, we could potentially infer the capability based on registered subscribers. I've not prototyped that to know if it's definititely possible and it would also only work if all subscribed IOpAmpListerners are added registered before StartAsync is called. In one respect, I like the idea that all a consumer would need to do is register the listener and our implementation works out the rest, but it might be a little too much magic. We may want to support binding configuration from other sources at some point so having a settings class we can bind to is useful there. If we don't include the settings type, it would also prevent a consumer from easily disabling the feature through code (temporily) but leaving their listener registered.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

- Introduce `RemoteConfigSettings` to allow configuring remote configuration support in the OpAMP client.
- Update capability advertisement logic to include `AcceptsRemoteConfig` when enabled.
- Add and update tests to verify capability flags and heartbeat behavior.
- Update unshipped public API.
@stevejgordon stevejgordon requested a review from a team as a code owner December 16, 2025 10:09
@github-actions github-actions Bot added the comp:opamp.client Things related to OpenTelemetry.OpAmp.Client label Dec 16, 2025
@github-actions github-actions Bot requested a review from RassK December 16, 2025 10:09
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.52%. Comparing base (c7edda1) to head (50342a6).
⚠️ Report is 15 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3618      +/-   ##
==========================================
+ Coverage   71.49%   71.52%   +0.02%     
==========================================
  Files         443      444       +1     
  Lines       17546    17551       +5     
==========================================
+ Hits        12545    12553       +8     
+ Misses       5001     4998       -3     
Flag Coverage Δ
unittests-OpAmp.Client 80.16% <100.00%> (+0.56%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...penTelemetry.OpAmp.Client/Internal/FrameBuilder.cs 66.21% <100.00%> (+1.42%) ⬆️
...metry.OpAmp.Client/Settings/OpAmpClientSettings.cs 78.94% <100.00%> (+1.16%) ⬆️
...etry.OpAmp.Client/Settings/RemoteConfigSettings.cs 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread test/OpenTelemetry.OpAmp.Client.Tests/PlainHttpTransportTests.cs Outdated
@stevejgordon
Copy link
Copy Markdown
Contributor Author

@Kielek If this looks okay, I'm wondering if we can get a new alpha release with the changes so far? I'm on leave for two weeks starting Monday so I'll pick up the other message types etc. when I'm back to work.

@Kielek Kielek added this pull request to the merge queue Dec 19, 2025
@Kielek
Copy link
Copy Markdown
Member

Kielek commented Dec 19, 2025

This PR should be merged shortly. When it will be done, please create issues based on https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/new?template=release_request.yml

Merged via the queue into open-telemetry:main with commit 3a05ef1 Dec 19, 2025
67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:opamp.client Things related to OpenTelemetry.OpAmp.Client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[OpAMP] Provide settings to enable remote configuration messages

4 participants