Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for client-initiated connection settings request #192

Merged
merged 1 commit into from
Nov 6, 2023
Merged

Add support for client-initiated connection settings request #192

merged 1 commit into from
Nov 6, 2023

Conversation

tigrannajaryan
Copy link
Member

@tigrannajaryan tigrannajaryan commented Jul 25, 2023 •
edited
Loading

@tigrannajaryan tigrannajaryan mentioned this pull request Jul 21, 2023
Merged
@codecov
Copy link

codecov bot commented Jul 25, 2023 •
edited
Loading

Codecov Report

Attention: 129 lines in your changes are missing coverage. Please review.

Files Coverage Δ
client/httpclient.go 95.31% <100.00%> (+0.57%) ⬆️
client/internal/clientstate.go 90.47% <100.00%> (ø)
client/internal/receivedprocessor.go 84.93% <100.00%> (+0.64%) ⬆️
client/types/callbacks.go 66.66% <ø> (ø)
client/wsclient.go 91.56% <100.00%> (+6.81%) ⬆️
server/callbacks.go 68.18% <100.00%> (-4.55%) ⬇️
server/wsconnection.go 100.00% <100.00%> (ø)
client/internal/clientcommon.go 80.47% <90.90%> (+0.67%) ⬆️
client/internal/packagessyncer.go 59.25% <0.00%> (ø)
server/serverimpl.go 58.92% <95.00%> (+0.37%) ⬆️
... and 4 more

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/csrflow branch 4 times, most recently from 1399b2c to 4515106 Compare October 18, 2023 17:02
@tigrannajaryan tigrannajaryan changed the title Add client-initiated CSR flow example Add support for client-initiated connection settings request Oct 18, 2023
@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/csrflow branch 3 times, most recently from 4566e39 to bd6f23e Compare October 18, 2023 17:09
@tigrannajaryan
Add support for client-initiated connection settings request
f521153 
- Implemented spec change open-telemetry/opamp-spec#162
- Added ability for the client to request connection settings.
- Added CSR flow example to demonstrate the new capability.
@tigrannajaryan tigrannajaryan marked this pull request as ready for review October 18, 2023 17:22
@tigrannajaryan tigrannajaryan requested a review from a team October 18, 2023 17:22
@srikanthccv
Copy link
Member

Does anyone know why the codecov BASE commit is several months old? It says Coverage data is based on head f521153 compared to base efddaa2

@tigrannajaryan
Copy link
Member Author

Does anyone know why the codecov BASE commit is several months old? It says Coverage data is based on head f521153 compared to base efddaa2

Probably because the PR was created a few months ago?

Also I think the overall coverage percentages are incorrect:
image

I don't think the diff is at 44.15%, it should be higher based on line-by-line report I checked.

@srikanthccv
Copy link
Member

I think there is some bug with codecov. Even the most recent PR merged to the main also has a report that says codecov/project 71.58% (-4.54%) compared to efddaa2, which is from last year Nov.

@srikanthccv srikanthccv mentioned this pull request Oct 24, 2023
Closed
@andykellr andykellr mentioned this pull request Oct 25, 2023
Closed
@tigrannajaryan
Copy link
Member Author

@open-telemetry/opamp-go-approvers @open-telemetry/opamp-go-maintainers this is ready for review.

andykellr
andykellr approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@andykellr andykellr left a comment

Choose a reason for hiding this comment

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

Very nice.

@tigrannajaryan tigrannajaryan merged commit f0e220d into open-telemetry:main Nov 6, 2023
@tigrannajaryan tigrannajaryan deleted the feature/tigran/csrflow branch November 6, 2023 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evan-bradley evan-bradley evan-bradley left review comments

@srikanthccv srikanthccv srikanthccv approved these changes

@andykellr andykellr andykellr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tigrannajaryan @srikanthccv @andykellr @evan-bradley