[XTA-15079] Add SPOG ?o= routing support and fix final-flush auth on close#391
[XTA-15079] Add SPOG ?o= routing support and fix final-flush auth on close#391samikshya-db wants to merge 3 commits into
Conversation
…flag SPOG (Single Panel of Glass) replaces workspace-specific hostnames with account-level vanity URLs. When httpPath carries `?o=<workspaceId>`, endpoints that don't include the workspace in their URL path (telemetry, feature flags) need the workspace conveyed via the `x-databricks-org-id` header instead. Changes: - Parse `?o=<digits>` out of httpPath in DBSQLClient.connect() and stash the org-id as `x-databricks-org-id` on a new `ClientConfig.customHeaders` field. A user-supplied `customHeaders` entry (case-insensitive) takes precedence. - DatabricksTelemetryExporter spreads `config.customHeaders` into the outgoing POST headers. Auth headers still win on collision. - FeatureFlagCache does the same for the feature-flag GET. Not applicable to this driver (vs JDBC port in databricks/databricks-jdbc#1316): - httpPath property parser fix — Node.js passes `options.path` through unmodified. - Warehouse ID regex fix for SEA — driver uses Thrift only. - DBFS Volume header injection — driver exposes no Volume API. OAuth/OIDC token requests deliberately do NOT receive customHeaders. Co-authored-by: Isaac
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
…lush `TelemetryClientProvider.releaseClient` was calling `unregisterContext` before `client.close()`. On the last refcount, that emptied the FIFO of `(context, authProvider)` pairs before the final flush ran, so the exporter's `getAuthProvider()` walk returned undefined and the batch was dropped with "Telemetry: Authorization header missing — metrics will be dropped". Defer `unregisterContext` until after `close()` on the last refcount; multi-refcount path is unchanged (immediate unregister so surviving consumers don't reach into a closing context). Verified end-to-end against a real SPOG host: the final flush now exports its 3 metrics (connection.open, statement.start, statement.complete) instead of warning and dropping them. Co-authored-by: Isaac
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
| const msg = error instanceof Error ? error.message : String(error); | ||
| logger.log(LogLevel.debug, `Error releasing TelemetryClient: ${msg}`); | ||
| } | ||
| // Remove from map BEFORE awaiting close so a concurrent |
There was a problem hiding this comment.
Have we tested this for auth and telemetry?
There was a problem hiding this comment.
yes, added test details in description
Tighten the previous commit to the minimal diff: guard the existing unregisterContext call instead of restructuring the function. Restores the unchanged comment on TelemetryClient.unregisterContext. Co-authored-by: Isaac
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Summary
Ports the SPOG (Single Panel of Glass) routing fix from
databricks/databricks-jdbc#1316
to the Node.js driver, and fixes a related close-ordering bug uncovered
while validating the port against a real SPOG host.
Change 1: SPOG header injection
SPOG replaces workspace-specific hostnames with account-level vanity URLs.
When
httpPathcarries?o=<workspaceId>, endpoints that don't includethe workspace in their URL path need the workspace conveyed via the
x-databricks-org-idheader instead.?o=<digits>out ofhttpPathinDBSQLClient.connect()andstash the org-id as
x-databricks-org-idon a newClientConfig.customHeadersfield. A user-suppliedcustomHeadersentry (case-insensitively keyed) takes precedence.
DatabricksTelemetryExporterspreadsconfig.customHeadersinto theoutgoing POST headers; auth headers still win on collision.
FeatureFlagCachedoes the same for the feature-flag GET.ConnectionOptions.customHeadersexposed so callers can also injecttheir own out-of-band headers.
OAuth / OIDC token requests use
openid-client's private HTTP transportand never see
customHeaders, matching JDBC's exemption.Not needed vs JDBC: this driver passes
options.paththroughunmodified (no split-on-
=parser), uses Thrift only (no SEA JSON bodywarehouse-ID extraction), and exposes no Volume API surface.
Change 2: Close-ordering fix
While verifying Change 1 against a real SPOG warehouse, the final
client.close()flush emitted:Root cause:
TelemetryClientProvider.releaseClientcalledunregisterContextbeforeclient.close(). On the last refcount, thatemptied the FIFO of
(context, authProvider)pairs before the finalflush, so the exporter's
getAuthProvider()walk returned undefined andthe batch was dropped.
Fix: skip the
unregisteron the last release so the FIFO snapshotsurvives through
close()'s final flush. Multi-refcount path isunchanged. No leak — the TelemetryClient is unreachable as soon as
releaseClientreturns, and the retained(context, authProvider)pair dies with it.
Live verification
Ran against
dogfood-spog.staging.azuredatabricks.netwarehousead6f9b54e6593746?o=7064161269814046:Test plan
Unit tests added (all passing locally):
buildCustomHeadersparses?o=intox-databricks-org-id?o=is absent and no user-suppliedcustomHeaderscustomHeadersis preserved alongside parsed org-idx-databricks-org-id(any case) wins over?o=o=<value>does not inject a headerDBSQLClient.connect()populatesconfig.customHeaders['x-databricks-org-id']from the pathDatabricksTelemetryExporterattachesconfig.customHeaderstothe POST headers
customHeaderson key collisionconfig.customHeadersis emptyFeatureFlagCache.fetchFeatureFlagattachescustomHeaderstothe GET headers
releaseClientkeeps the context registered throughclose()onlast refcount
releaseClientdrops the context immediately when otherrefcounts remain
This pull request and its description were written by Isaac.