Repository navigation
test: e2e verification of built-in metrics and fail-open behavior; panic guard on the record path - #1186
Merged
Conversation
Base automatically changed from
metrics-quiet-export
to
feature/client-side-metrics
October 21, 2026 16:30
…nic guard on the record path Replaces the manual prototype (cmd/metric_prototype) with automated coverage that runs in the existing integration-test job: - TestMetricsE2EExport: dial the e2e instance with metrics enabled and a unique application_name, Close (flush), then poll Cloud Monitoring for this process's client_uid and assert all five metrics arrive with the documented label values. Skips with the exact gcloud commands when the e2e identity lacks roles/monitoring.metricWriter or monitoring.viewer. - TestMetricsE2EFailOpenUnreachableEndpoint: metrics pointed at an unroutable endpoint (real DeadlineExceeded/Unavailable). Dials succeed, dial p50 matches a metrics-off dialer, Close returns within the shutdown timeout, nothing on the standard logger. - TestMetricsE2EFailOpenBadCredentials: an invalid token yields a real Unauthenticated. Dials succeed, exactly one standard-logger warning with remediation, export suspended (1-3 attempts in the window). All three pass against a real instance and the real Monitoring API; they skip under -short like the other e2e tests. Also add a deferred recover() to every MetricRecorder.Record* method so a panic inside the metrics SDK can never reach the dial path, with a test that fails when the guard is removed. Test hooks move from internal/tel/export_test.go to testhooks.go so the external e2e package can use them; export_test.go in the root package exposes the few unexported accessors the e2e tests need.
panavenue
force-pushed
the
metrics-e2e
branch
from
October 21, 2026 16:33
6ef474d to
78af000
Compare
kgala2
approved these changes
Oct 21, 2026
kgala2
reviewed
Oct 21, 2026
…2e test Review feedback on #1186. A bare recover() would make a bug in the metrics pipeline invisible. recoverPanic now counts recovered panics in tel.RecoveredPanics, debug-logs once per recorder, and writes one line per process to the standard logger with a link to file an issue. The dial path is still never affected. The e2e bad-credentials test uses atomic.Int32 instead of a mutex for its call counter.
kgala2
approved these changes
Oct 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Automates what we have been verifying by hand with the
cmd/metric_prototypebranch (#1139), and adds a last-line-of-defence guard so metrics can never break connectivity.E2E tests (
e2e_metrics_test.go, run by the existing integration-test job)TestMetricsE2EExportClose(), all five metrics arrive in Cloud Monitoring for this process'sclient_uid, with the documented labels (application_name,database_engine_type=postgresql,connector_type=go,region,resource_id, resolvedinstance_ip_type, validcompute_platform,status=success).TestMetricsE2EFailOpenUnreachableEndpointDeadlineExceeded/Unavailable): dials succeed, dial p50 matches a metrics-off dialer,Close()returns within the shutdown timeout, nothing on the standard logger (transient tier).TestMetricsE2EFailOpenBadCredentialsUnauthenticated): dials succeed, exactly one standard-logger warning with role/project/opt-out hint, export suspended (1–3 attempts in the window).All three pass locally against
jinphou-playground:us-west1:pg-dev(export verified in ~13 s). They skip under-shortlike the other e2e tests and only needPOSTGRES_CONNECTION_NAME.One-time CI setup needed
The e2e identities on
cloud-sql-connector-testing(cloud-build-pool@…and the compute default SA) currently havecloudsql.clientbut no Monitoring roles, soTestMetricsE2EExportwill skip in CI (with the exactgcloudcommands in the skip message) until these are granted:The two fail-open tests need no IAM and run everywhere.
Panic guard
Every
MetricRecorder.Record*method now defersrecover(). The OTel SDK does not panic in practice, but with metrics on by default in every connector and proxy process, "unlikely" should be "impossible" on the dial path.TestRecordNeverPanicsuses a recorder with nil instruments and fails if the guard is removed (verified).Plumbing
internal/tel/export_test.go→internal/tel/testhooks.go(non-_test, so the external e2e package can use the hooks;internal/is not importable outside the module).export_test.goexposingWithMetricClientOptionsForTest,DialerIDForTest,DetectPrincipalForTest,MetricShutdownTimeoutForTesttocloudsqlconn_test.Follow-ups
connectors/toolsprober (G2) can reuseTestMetricsE2EExport's body against the released connector/proxy, plus an absence alert onopen_connection_countfor the prober'sclient_uid.