Skip to content

Commit dbeff48

Browse files
committed
Merge telemetry-3-client-management: sync TelemetryClient.close(), fix auth
- TelemetryClient.close() is now synchronous with try/catch/finally - TelemetryClientProvider.releaseClient() is synchronous; map entry deleted before close() to prevent stale-client async race - Provider and client log at debug level with @internal test helpers - Fix getAuthHeaders() compile error: add getAuthProvider() to IClientContext and update DatabricksTelemetryExporter / FeatureFlagCache - Remove undefined TelemetryTerminalError re-export from index.ts - types.ts DEFAULT_TELEMETRY_CONFIG now Readonly+Object.freeze Co-authored-by: Isaac Signed-off-by: samikshya-chand_data <samikshya.chand@databricks.com>
2 parents 91ab633 + 2749751 commit dbeff48

30 files changed

Lines changed: 1048 additions & 249 deletions
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
name: Setup JFrog OIDC
2+
description: Obtain a JFrog access token via GitHub OIDC and configure npm to use JFrog registry proxy
3+
4+
runs:
5+
using: composite
6+
steps:
7+
- name: Get JFrog OIDC token
8+
shell: bash
9+
run: |
10+
set -euo pipefail
11+
ID_TOKEN=$(curl -sLS \
12+
-H "User-Agent: actions/oidc-client" \
13+
-H "Authorization: Bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \
14+
"${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=jfrog-github" | jq .value | tr -d '"')
15+
echo "::add-mask::${ID_TOKEN}"
16+
ACCESS_TOKEN=$(curl -sLS -XPOST -H "Content-Type: application/json" \
17+
"https://databricks.jfrog.io/access/api/v1/oidc/token" \
18+
-d "{\"grant_type\": \"urn:ietf:params:oauth:grant-type:token-exchange\", \"subject_token_type\":\"urn:ietf:params:oauth:token-type:id_token\", \"subject_token\": \"${ID_TOKEN}\", \"provider_name\": \"github-actions\"}" | jq .access_token | tr -d '"')
19+
echo "::add-mask::${ACCESS_TOKEN}"
20+
if [ -z "$ACCESS_TOKEN" ] || [ "$ACCESS_TOKEN" = "null" ]; then
21+
echo "FAIL: Could not extract JFrog access token"
22+
exit 1
23+
fi
24+
echo "JFROG_ACCESS_TOKEN=${ACCESS_TOKEN}" >> "$GITHUB_ENV"
25+
echo "JFrog OIDC token obtained successfully"
26+
27+
- name: Configure npm for JFrog
28+
shell: bash
29+
run: |
30+
set -euo pipefail
31+
cat > ~/.npmrc << EOF
32+
registry=https://databricks.jfrog.io/artifactory/api/npm/db-npm/
33+
//databricks.jfrog.io/artifactory/api/npm/db-npm/:_authToken=${JFROG_ACCESS_TOKEN}
34+
always-auth=true
35+
EOF
36+
echo "npm configured to use JFrog registry"

.github/dependabot.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
version: 2
2+
updates:
3+
- package-ecosystem: npm
4+
directory: /
5+
schedule:
6+
interval: weekly
7+
- package-ecosystem: github-actions
8+
directory: /
9+
schedule:
10+
interval: weekly

.github/workflows/dco-check.yml

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,36 @@ name: DCO Check
22

33
on: [pull_request]
44

5+
permissions:
6+
contents: read
7+
pull-requests: write
8+
59
jobs:
610
check:
7-
runs-on: ubuntu-latest
11+
runs-on:
12+
group: databricks-protected-runner-group
13+
labels: linux-ubuntu-latest
814
steps:
9-
- name: Check for DCO
15+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
16+
with:
17+
fetch-depth: 0
18+
- name: Check for DCO sign-off
1019
id: dco-check
11-
uses: tisonkun/actions-dco@v1.1
20+
run: |
21+
base_sha="${{ github.event.pull_request.base.sha }}"
22+
head_sha="${{ github.event.pull_request.head.sha }}"
23+
failed=0
24+
for sha in $(git rev-list "$base_sha".."$head_sha"); do
25+
if ! git log -1 --format='%B' "$sha" | grep -qiE '^Signed-off-by: .+ <.+>'; then
26+
echo "::error::Commit $sha is missing a DCO sign-off"
27+
failed=1
28+
fi
29+
done
30+
if [ "$failed" -eq 1 ]; then
31+
exit 1
32+
fi
1233
- name: Comment about DCO status
13-
uses: actions/github-script@v6
34+
uses: actions/github-script@d7906e4ad0b1822421a7e6a35d5ca353c962f410 # v6
1435
if: ${{ failure() }}
1536
with:
1637
script: |

.github/workflows/main.yml

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,23 @@ on:
88
branches:
99
- main
1010

11+
permissions:
12+
contents: read
13+
id-token: write
14+
1115
jobs:
1216
lint:
13-
runs-on: ubuntu-latest
17+
runs-on:
18+
group: databricks-protected-runner-group
19+
labels: linux-ubuntu-latest
1420
steps:
15-
- uses: actions/checkout@v3
21+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
22+
- uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
23+
with:
24+
node-version: 20
25+
- uses: ./.github/actions/setup-jfrog
1626
- name: Cache node modules
17-
uses: actions/cache@v3
27+
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
1828
env:
1929
cache-name: cache-node-modules
2030
with:
@@ -31,7 +41,9 @@ jobs:
3141
npm run lint
3242
3343
unit-test:
34-
runs-on: ubuntu-latest
44+
runs-on:
45+
group: databricks-protected-runner-group
46+
labels: linux-ubuntu-latest
3547
strategy:
3648
matrix:
3749
# only LTS versions starting from the lowest we support
@@ -41,17 +53,18 @@ jobs:
4153
NYC_REPORT_DIR: coverage_unit_node${{ matrix.node-version }}
4254

4355
steps:
44-
- uses: actions/setup-node@v4
56+
- uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
4557
with:
4658
node-version: ${{ matrix.node-version }}
47-
- uses: actions/checkout@v3
59+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
4860
- name: Set up Python 3.10 for Node 14
4961
if: ${{ matrix.node-version == '14' }}
50-
uses: actions/setup-python@v4
62+
uses: actions/setup-python@7f4fc3e22c37d6ff65e88745f38bd3157c663f7c # v4
5163
with:
5264
python-version: '3.10'
65+
- uses: ./.github/actions/setup-jfrog
5366
- name: Cache node modules
54-
uses: actions/cache@v3
67+
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
5568
with:
5669
path: ~/.npm
5770
key: ${{ runner.os }}-${{ matrix.node-version }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
@@ -65,14 +78,16 @@ jobs:
6578
npm run test
6679
- run: tar -cvf ${{ env.NYC_REPORT_DIR }}.tar ${{ env.NYC_REPORT_DIR }}
6780
- name: Store coverage report
68-
uses: actions/upload-artifact@v4
81+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
6982
with:
7083
name: ${{ env.NYC_REPORT_DIR }}
7184
path: ${{ env.NYC_REPORT_DIR }}.tar
7285
retention-days: 1
7386

7487
e2e-test:
75-
runs-on: ubuntu-latest
88+
runs-on:
89+
group: databricks-protected-runner-group
90+
labels: linux-ubuntu-latest
7691
environment: azure-prod
7792
env:
7893
E2E_HOST: ${{ secrets.DATABRICKS_HOST }}
@@ -86,9 +101,13 @@ jobs:
86101
NYC_REPORT_DIR: coverage_e2e
87102

88103
steps:
89-
- uses: actions/checkout@v3
104+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
105+
- uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
106+
with:
107+
node-version: 20
108+
- uses: ./.github/actions/setup-jfrog
90109
- name: Cache node modules
91-
uses: actions/cache@v3
110+
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
92111
with:
93112
path: ~/.npm
94113
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
@@ -102,30 +121,32 @@ jobs:
102121
NODE_OPTIONS="--max-old-space-size=4096" npm run e2e
103122
- run: tar -cvf ${{ env.NYC_REPORT_DIR }}.tar ${{ env.NYC_REPORT_DIR }}
104123
- name: Store coverage report
105-
uses: actions/upload-artifact@v4
124+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
106125
with:
107126
name: ${{ env.NYC_REPORT_DIR }}
108127
path: ${{ env.NYC_REPORT_DIR }}.tar
109128
retention-days: 1
110129

111130
coverage:
112131
needs: [unit-test, e2e-test]
113-
runs-on: ubuntu-latest
132+
runs-on:
133+
group: databricks-protected-runner-group
134+
labels: linux-ubuntu-latest
114135
env:
115136
cache-name: cache-node-modules
116137

117138
steps:
118-
- uses: actions/checkout@v3
139+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
119140
- name: Cache node modules
120-
uses: actions/cache@v3
141+
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
121142
with:
122143
path: ~/.npm
123144
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
124145
restore-keys: |
125146
${{ runner.os }}-build-${{ env.cache-name }}-
126147
${{ runner.os }}-build-
127148
${{ runner.os }}-
128-
- uses: actions/download-artifact@v4
149+
- uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4
129150
with:
130151
pattern: coverage_*
131152
merge-multiple: true
@@ -135,7 +156,7 @@ jobs:
135156
rm coverage_*.tar
136157
- run: ls -la
137158
- name: Coverage
138-
uses: codecov/codecov-action@v3
159+
uses: codecov/codecov-action@ab904c41d6ece82784817410c45d8b8c02684457 # v3
139160
with:
140161
token: ${{ secrets.CODECOV_TOKEN }}
141162
fail_ci_if_error: true

.github/workflows/release.yml

Lines changed: 0 additions & 19 deletions
This file was deleted.

.npmrc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
package-lock=false
1+
package-lock=true

.prettierignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ node_modules
55
.nyc_output
66
coverage_e2e
77
coverage_unit
8+
coverage
89
.clinic
910

1011
dist

lib/DBSQLClient.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import Int64 from 'node-int64';
33
import os from 'os';
44

55
import { EventEmitter } from 'events';
6-
import { HeadersInit } from 'node-fetch';
76
import TCLIService from '../thrift/TCLIService';
87
import { TProtocolVersion } from '../thrift/TCLIService_types';
98
import IDBSQLClient, { ClientOptions, ConnectionOptions, OpenSessionRequest } from './contracts/IDBSQLClient';
@@ -498,6 +497,12 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
498497
this.config.telemetryAuthenticatedExport = options.telemetryAuthenticatedExport;
499498
}
500499

500+
// Persist userAgentEntry so telemetry and feature-flag call sites reuse
501+
// the same value as the primary Thrift connection's User-Agent.
502+
if (options.userAgentEntry !== undefined) {
503+
this.config.userAgentEntry = options.userAgentEntry;
504+
}
505+
501506
this.authProvider = this.createAuthProvider(options, authProvider);
502507

503508
this.connectionProvider = this.createConnectionProvider(options);
@@ -664,19 +669,13 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
664669
}
665670

666671
/**
667-
* Gets authentication headers for HTTP requests.
668-
* Used by telemetry and feature flag fetching to authenticate REST API calls.
669-
* @returns Promise resolving to headers object with authentication, or empty object if no auth
672+
* Returns the authentication provider associated with this client, if any.
673+
* Intended for internal telemetry/feature-flag call sites that need to
674+
* obtain auth headers directly without routing through `IClientContext`.
675+
*
676+
* @internal Not part of the public API. May change without notice.
670677
*/
671-
public async getAuthHeaders(): Promise<HeadersInit> {
672-
if (this.authProvider) {
673-
try {
674-
return await this.authProvider.authenticate();
675-
} catch (error) {
676-
this.logger.log(LogLevel.debug, `Error getting auth headers: ${error}`);
677-
return {};
678-
}
679-
}
680-
return {};
678+
public getAuthProvider(): IAuthentication | undefined {
679+
return this.authProvider;
681680
}
682681
}

lib/contracts/IClientContext.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import { HeadersInit } from 'node-fetch';
21
import IDBSQLLogger from './IDBSQLLogger';
32
import IDriver from './IDriver';
43
import IConnectionProvider from '../connection/contracts/IConnectionProvider';
54
import IThriftClient from './IThriftClient';
5+
import IAuthentication from '../connection/contracts/IAuthentication';
66

77
export interface ClientConfig {
88
directResultsDefaultMaxRows: number;
@@ -29,9 +29,16 @@ export interface ClientConfig {
2929
telemetryBatchSize?: number;
3030
telemetryFlushIntervalMs?: number;
3131
telemetryMaxRetries?: number;
32+
telemetryBackoffBaseMs?: number;
33+
telemetryBackoffMaxMs?: number;
34+
telemetryBackoffJitterMs?: number;
3235
telemetryAuthenticatedExport?: boolean;
3336
telemetryCircuitBreakerThreshold?: number;
3437
telemetryCircuitBreakerTimeout?: number;
38+
telemetryMaxPendingMetrics?: number;
39+
telemetryMaxErrorsPerStatement?: number;
40+
telemetryStatementTtlMs?: number;
41+
userAgentEntry?: string;
3542
}
3643

3744
export default interface IClientContext {
@@ -45,10 +52,5 @@ export default interface IClientContext {
4552

4653
getDriver(): Promise<IDriver>;
4754

48-
/**
49-
* Gets authentication headers for HTTP requests.
50-
* Used by telemetry and feature flag fetching to authenticate REST API calls.
51-
* @returns Promise resolving to headers object with authentication, or empty object if no auth
52-
*/
53-
getAuthHeaders(): Promise<HeadersInit>;
55+
getAuthProvider(): IAuthentication | undefined;
5456
}

lib/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import { LogLevel } from './contracts/IDBSQLLogger';
2323
// Re-export types for TypeScript users
2424
export type { default as ITokenProvider } from './connection/auth/tokenProvider/ITokenProvider';
2525

26+
export { CircuitBreakerOpenError, CIRCUIT_BREAKER_OPEN_CODE } from './telemetry/CircuitBreaker';
27+
2628
export const auth = {
2729
PlainHttpAuthentication,
2830
// Token provider classes for custom authentication

0 commit comments

Comments
 (0)