Skip to content

Allow connection settings to be applied asyncronously#544

Open
michel-laterman wants to merge 2 commits intoopen-telemetry:mainfrom
michel-laterman:fix/allow-async-connection-settings
Open

Allow connection settings to be applied asyncronously#544
michel-laterman wants to merge 2 commits intoopen-telemetry:mainfrom
michel-laterman:fix/allow-async-connection-settings

Conversation

@michel-laterman
Copy link
Copy Markdown
Contributor

Expose SetConnectionsSettingsStatus method in client interface to allow implementations that use async approaches to connection management to succeed instead of automatically setting APPLIED.

This change technically breaks an assumption we had in the initial implementation and passes the responsibility for setting the connectionsettingsstatus to the callback implementation.

The client/internal/receivedprocessor.go still sets the status to APPLYING if any connection settings are sent (and the reports connections settings status capability is set),

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.10%. Comparing base (03800d0) to head (4d5f22b).

Files with missing lines Patch % Lines
client/internal/clientcommon.go 88.23% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #544      +/-   ##
==========================================
+ Coverage   82.38%   85.10%   +2.72%     
==========================================
  Files          28       28              
  Lines        2208     2189      -19     
==========================================
+ Hits         1819     1863      +44     
+ Misses        264      203      -61     
+ Partials      125      123       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Expose SetConnectionsSettingsStatus method in client interface to allow
implementations that use async approaches to connection management to
succeed instead of automatically setting APPLIED.
@michel-laterman michel-laterman force-pushed the fix/allow-async-connection-settings branch from b4aa108 to db8b353 Compare April 2, 2026 14:56
@michel-laterman michel-laterman marked this pull request as ready for review April 2, 2026 15:15
@michel-laterman michel-laterman requested a review from a team as a code owner April 2, 2026 15:15
@michel-laterman
Copy link
Copy Markdown
Contributor Author

@tigrannajaryan @evan-bradley, I'm addressing the changes needed in opamp-go for open-telemetry/opentelemetry-collector-contrib#46157

This is a pretty fundamental change as it changes what sets the status from an internal detail (that assumes the callback is synchronous), to something that the connection settings callbacks needs to explicitly set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant