Skip to content

fix(bigtable): test resource leaks, timestamp clamping, and metrics exporter - #9059

Merged
bshaffer merged 6 commits into
mainfrom
bigtable-ci-fixes
Aug 4, 2026
Merged

fix(bigtable): test resource leaks, timestamp clamping, and metrics exporter#9059
bshaffer merged 6 commits into
mainfrom
bigtable-ci-fixes

Conversation

@bshaffer

@bshaffer bshaffer commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Separated from #9058 to isolate Bigtable CI fixes:

  • Fixes resource leaks in Bigtable system tests by preventing reapInstances from deleting active test instances
  • Fixes OpenTelemetry timestamp clamping in exporter
  • Overrides OpenTelemetry metrics periodic exporter interval to 1s in tests to prevent timeouts

@bshaffer
bshaffer requested a review from a team as a code owner August 3, 2026 19:11
@product-auto-label product-auto-label Bot added the api: bigtable Issues related to the Bigtable API. label Aug 3, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the client-side metrics exporter to handle cases where the start time is greater than or equal to the end time, and stubs the OpenTelemetry metric reader in tests to use a faster export interval. Additionally, in the system tests, the instance reaping age is increased to 2 hours, and instances are now tagged with a creation timestamp. However, the reviewer correctly points out that changing the reaping logic to ignore instances without a 'time_created' label could lead to resource leaks if other tests create instances without this label.

Comment thread handwritten/bigtable/system-test/common.ts Outdated

@westarle westarle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's a bit hard to review (e.g. the change in exporter.ts without knowing what the failure mode was.

Comment thread handwritten/bigtable/src/client-side-metrics/exporter.ts Outdated
Comment thread handwritten/bigtable/system-test/common.ts
Comment thread handwritten/bigtable/src/client-side-metrics/exporter.ts
@bshaffer

bshaffer commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

it's a bit hard to review (e.g. the change in exporter.ts without knowing what the failure mode was.

When OpenTelemetry records very fast operations (or during system tests), it can emit metric data points where startTime >= endTime. Google Cloud Monitoring's CreateTimeSeries API was rejecting these with INVALID_ARGUMENT: startTime must be before endTime. So this change just ensures that endTime is at least 1 nanosecond after startTime to prevent those API rejections.

@bshaffer
bshaffer merged commit 0712c05 into main Aug 4, 2026
40 of 42 checks passed
@bshaffer
bshaffer deleted the bigtable-ci-fixes branch August 4, 2026 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants