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

feat: improve batching summary errors #2509

Merged
merged 5 commits into from
Feb 29, 2024

Conversation

igorbernstein2
Copy link
Contributor

To avoid data loss, the Batching api throws an exception when the batcher is closed when at least 1 entry failed. To help debugging, the BatchingException tries to be helpful by giving some details about the errors. Since the Batcher lifetime can extend indefinitely, the Batcher implementation tries to keep a bound on the amount of resources it uses to track the errors. Previously it would only track exception types and counts. The idea being that if the customer has the need for fine grained details, the customer can retrieve the details from the ApiFuture that was returned when an entry was added. However this hasn't panned out and creates confusion.

This PR stores a sample of the error messages. The sample is by default capped to 50 entry and 50 rpc error messages. This can be adjusted by setting system properties

Thank you for opening a Pull Request! For general contributing guidelines, please refer to contributing guide

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

To avoid data loss, the Batching api throws an exception when the batcher is closed when at least 1 entry failed.
To help debugging, the BatchingException tries to be helpful by giving some details about the errors. Since the Batcher lifetime
can extend indefinitely, the Batcher implementation tries to keep a bound on the amount of resources it uses to track the errors.
Previously it would only track exception types and counts. The idea being that if the customer has the need for fine grained details,
the customer can retrieve the details from the ApiFuture that was returned when an entry was added. However this hasn't panned out
and creates confusion.

This PR stores a sample of the error messages. The sample is by default capped to 50 entry and 50 rpc error messages. This can be adjusted by setting system properties
@igorbernstein2 igorbernstein2 requested a review from a team as a code owner February 27, 2024 18:16
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Feb 27, 2024
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Feb 28, 2024
@igorbernstein2 igorbernstein2 enabled auto-merge (squash) February 28, 2024 16:08
@blakeli0 blakeli0 added the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 29, 2024
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Feb 29, 2024
Copy link

sonarcloud bot commented Feb 29, 2024

Quality Gate Failed Quality Gate failed for 'gapic-generator-java-root'

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Feb 29, 2024

Quality Gate Failed Quality Gate failed for 'java_showcase_integration_tests'

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@igorbernstein2 igorbernstein2 merged commit d6964a4 into googleapis:main Feb 29, 2024
36 of 38 checks passed
zhumin8 pushed a commit that referenced this pull request Feb 29, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>2.37.0</summary>

##
[2.37.0](v2.36.0...v2.37.0)
(2024-02-29)


### Features

* improve batching summary errors
([#2509](#2509))
([d6964a4](d6964a4))


### Bug Fixes

* adjust release please config to reflect file location change.
([#2524](#2524))
([1e5af1e](1e5af1e))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
@igorbernstein2 igorbernstein2 deleted the error-msgs branch September 20, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants