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

[exporter] [chore] Initialize batchSender and queueSender after configuration - #2 #11184

Closed

Conversation

sfc-gh-sili
Copy link
Contributor

@sfc-gh-sili sfc-gh-sili commented Sep 16, 2024

This PR follows #11041.

The previous PR changed the initialization of batchSender and queueSender to AFTER configuration, because that enables us to access queueConfig and batcherConfig in the same place.

I noticed since then that there is another API for queue configuration, and this PR takes care of that other API

Link to tracking issue

#10368

Testing

Ran opentelemetry-collector$ make to make sure all tests still pass.

@sfc-gh-sili sfc-gh-sili changed the title Delay queue sender initialization [exporter] [chore] Initialize batchSender and queueSender after configuration - #2 Sep 16, 2024
@sfc-gh-sili sfc-gh-sili marked this pull request as ready for review September 16, 2024 22:49
@sfc-gh-sili sfc-gh-sili requested review from a team and dmitryax September 16, 2024 22:49
for _, op := range options {
err = multierr.Append(err, op(be))
}
}

// Set up batch sender if batching is enabled
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving batch sender initialization down because in the future we will only initialize batch sender if queue sender is not initialized.

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.29%. Comparing base (2c0941f) to head (0b491c6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #11184   +/-   ##
=======================================
  Coverage   92.28%   92.29%           
=======================================
  Files         413      413           
  Lines       19766    19768    +2     
=======================================
+ Hits        18242    18244    +2     
  Misses       1152     1152           
  Partials      372      372           

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

@sfc-gh-sili
Copy link
Contributor Author

sfc-gh-sili commented Sep 20, 2024

Closing Pull request because there has been some changes to common.go. ref: df3c9e3

@sfc-gh-sili
Copy link
Contributor Author

Second trial here: #11240

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