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.

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

Add client context propagation #248

Merged

Conversation

jaronoff97
Copy link
Contributor

@jaronoff97 jaronoff97 commented Jan 26, 2024

This is a follow up to #237 and #247, adding context propagation for client methods.

This involves a breaking change for the client interfaces

@jaronoff97 jaronoff97 requested a review from a team January 26, 2024 15:36
}
} else {
r.logger.Debugf(ctx, "Ignoring Opamp, agent does not have AcceptsOpAMPConnectionSettings capability")
}
}

func (r *receivedProcessor) processErrorResponse(ctx context.Context, body *protobufs.ServerErrorResponse) {
// TODO: implement this.
r.logger.Errorf(ctx, "received an error from server: %s", body.ErrorMessage)
if body != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This never called the OnError method when it should have.

Copy link

codecov bot commented Jan 26, 2024

Codecov Report

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

Comparison is base (4d07a6a) 72.86% compared to head (5e080dd) 72.92%.

Files Patch % Lines
client/internal/receivedprocessor.go 57.14% 3 Missing ⚠️
client/types/callbacks.go 70.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #248      +/-   ##
==========================================
+ Coverage   72.86%   72.92%   +0.06%     
==========================================
  Files          25       25              
  Lines        2038     2039       +1     
==========================================
+ Hits         1485     1487       +2     
+ Misses        444      443       -1     
  Partials      109      109              

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

@tigrannajaryan tigrannajaryan merged commit fab35cf into open-telemetry:main Jan 26, 2024
5 of 6 checks passed
@jaronoff97 jaronoff97 deleted the client-context-propagation branch January 26, 2024 20:22
Copy link
Contributor

@gdfast gdfast left a comment

Choose a reason for hiding this comment

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

Great change!

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.

3 participants