Skip to content

#6454 log warning and adjust maxExportBatchSize when exceeds maxQueueSize.#7045

Merged
jack-berg merged 5 commits intoopen-telemetry:mainfrom
chukunx:help/queue-batch-size
Feb 25, 2025
Merged

#6454 log warning and adjust maxExportBatchSize when exceeds maxQueueSize.#7045
jack-berg merged 5 commits intoopen-telemetry:mainfrom
chukunx:help/queue-batch-size

Conversation

@chukunx
Copy link
Contributor

@chukunx chukunx commented Jan 26, 2025

Fix #6454

@chukunx chukunx force-pushed the help/queue-batch-size branch from 0744fa5 to 0f383c2 Compare February 10, 2025 04:36
@chukunx chukunx changed the title #6454 log warning message for misconfigured batch and queue size. #6454 log warning and adjust maxExportBatchSize when exceeds maxQueueSize. Feb 10, 2025
@codecov
Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.87%. Comparing base (92b089a) to head (3fa9c51).
Report is 36 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7045      +/-   ##
============================================
+ Coverage     89.84%   89.87%   +0.03%     
- Complexity     6612     6624      +12     
============================================
  Files           740      740              
  Lines         19991    20008      +17     
  Branches       1966     1968       +2     
============================================
+ Hits          17960    17983      +23     
+ Misses         1439     1437       -2     
+ Partials        592      588       -4     

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

@jack-berg jack-berg merged commit 31f484f into open-telemetry:main Feb 25, 2025
27 checks passed
Comment on lines +134 to +136
if (maxExportBatchSize > maxQueueSize) {
logger.log(Level.WARNING, "maxExportBatchSize should not exceed maxQueueSize.");
}
Copy link
Member

Choose a reason for hiding this comment

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

Can this generate a false positive depending on the order the setters are called?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about change the log level in build() to warning, remove these or use fine level here?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. I think removing them from here would be ok in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made the change in #7148

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.

Can the api give a clear warning message if miss config for BatchLogRecordProcessorBuilder's maxQueueSize and maxExportBatchSize?

4 participants