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

[client, consumer] Move client features to consumerconnection #9996

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

TylerHelmuth
Copy link
Member

@TylerHelmuth TylerHelmuth commented Apr 18, 2024

Description

This PR moves client features to consumerconnection. This attempts to remove ambiguity from the name client and continues to centralizes consumer as the location for features that pass data between components.

Link to tracking issue

Related to #9818

Testing

Duplicated the unit tests

Documentation

Updated the package comments and added deprecated warnings to the old client package.

@TylerHelmuth TylerHelmuth requested a review from a team as a code owner April 18, 2024 20:46
Copy link

codecov bot commented Apr 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.78%. Comparing base (fcdfdaa) to head (bc7f2f5).
Report is 35 commits behind head on main.

❗ Current head bc7f2f5 differs from pull request most recent head ffdf862. Consider uploading reports for the commit ffdf862 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9996      +/-   ##
==========================================
+ Coverage   91.77%   91.78%   +0.01%     
==========================================
  Files         359      361       +2     
  Lines       16633    16657      +24     
==========================================
+ Hits        15265    15289      +24     
  Misses       1041     1041              
  Partials      327      327              

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

// receivers: [otlp]
// processors: [authprinter]
// exporters: [debug]
package consumerconnection // import "go.opentelemetry.io/collector/consumer/consumerconnection"
Copy link
Member

Choose a reason for hiding this comment

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

consumerconnection go likes shorter names, conn is a good replacement for connection.

Not sure if is not cleaner to have consumer.ConnInfo instead? OR consumerinfo.Conn and consumerinfo.AuthData?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to consumerconn for now. That gives us consumerconn.Info and consumerconn.AuthData which I like since sharing connection data is the overall theme of the package and Info and AuthData are pieces of connection data the package supports sharing.

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.

None yet

2 participants