Skip to content

feat(dataconnect): add "X-Client-Version" header for cloud monitoring - #3222

Merged
dconeybe merged 4 commits into
mainfrom
dconeybe/dataconnect/CloudMonitoringHeaders
Jul 30, 2026
Merged

feat(dataconnect): add "X-Client-Version" header for cloud monitoring#3222
dconeybe merged 4 commits into
mainfrom
dconeybe/dataconnect/CloudMonitoringHeaders

Conversation

@dconeybe

@dconeybe dconeybe commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

This PR adds the X-Client-Version request header (formatted as node/<version>) to DataConnectApiClient to enable metrics collection in Cloud Monitoring.

Highlights

  • Cloud Monitoring Metrics Header: Added the X-Client-Version header (set to node/$SDK_VERSION) to outgoing requests in DataConnectApiClient to enable Cloud Monitoring telemetry.
  • Test Updates: Updated unit test suites to verify that X-Client-Version is correctly attached to API client requests.
Changelog
  • data-connect-api-client-internal.ts
    • Updated getHeaders to include the X-Client-Version header formatted as node/${utils.getSdkVersion()} in default request headers.
  • data-connect-api-client-internal.spec.ts
    • Updated unit test header constants to verify X-Client-Version is included in request headers.

…lient-Version" headers

Googlers see go/dataconnect:sdk:headers for details
@dconeybe dconeybe self-assigned this Jul 28, 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 adds 'x-client-platform' and 'x-client-version' headers to the Data Connect API client and updates the corresponding unit tests. The reviewer suggested capitalizing these headers to 'X-Client-Platform' and 'X-Client-Version' in both the implementation and test files to maintain consistency with existing headers.

Comment thread src/data-connect/data-connect-api-client-internal.ts Outdated
Comment thread test/unit/data-connect/data-connect-api-client-internal.spec.ts Outdated
Comment thread test/unit/data-connect/data-connect-api-client-internal.spec.ts Outdated
Comment thread test/unit/data-connect/data-connect-api-client-internal.spec.ts Outdated
…ther headers

The new headers `'x-client-platform'` and `'x-client-version'` are defined in
lowercase, which is inconsistent with the Pascal-Kebab-Case used for other
headers in this object (e.g., `'X-Firebase-Client'`, `'X-Goog-Api-Client'`) and
the PR title (`X-Client-Platform` and `X-Client-Version`). For consistency and
to match the PR description, these should be capitalized.

See #3222 (comment)
@dconeybe
dconeybe marked this pull request as ready for review July 28, 2026 19:46
@dconeybe
dconeybe requested a review from stephenarosaj July 28, 2026 19:46
dconeybe added 2 commits July 30, 2026 12:02
This change was made because the "X-Client-Platform" header causes the CORS
OPTIONS preflight request to fail with a 403 "Permission Denied" error due to
that header being absent from Google's list of whitelisted CORS headers. In
contrast, the "X-Client-Version" header *is* whitelisted and, therefore,
successfully passes the CORS preflight. This problem only affects the JS
client SDK, but it affected how all platforms, including
firebase-admin-node, send their headers.

See firebase/firebase-js-sdk#10217 for details.
@dconeybe dconeybe changed the title feat(dataconnect): add "X-Client-Platform" and "X-Client-Version" headers feat(dataconnect): add "X-Client-Version" header for cloud monitoring Jul 30, 2026

@stephenarosaj stephenarosaj 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.

LGTM

@dconeybe
dconeybe merged commit 470bde2 into main Jul 30, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants