Skip to content

Commit ca12caf

Browse files
committed
fix: use opened compute for telemetry SPOG and prune closed coordinator states
1 parent f61bd63 commit ca12caf

7 files changed

Lines changed: 83 additions & 3 deletions

File tree

dbt/adapters/databricks/connections.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,9 @@ def connect() -> DatabricksHandle:
522522
databricks_connection.http_path
523523
)
524524

525-
telemetry_hooks.on_connection_open(creds, credentials_manager)
525+
telemetry_hooks.on_connection_open(
526+
creds, credentials_manager, databricks_connection.http_path
527+
)
526528
return conn
527529
else:
528530
raise DbtDatabaseError("Failed to create connection")

dbt/adapters/databricks/telemetry/builder.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,18 @@ def build_invocation_config(config: Any) -> models.InvocationConfig:
186186
)
187187

188188

189+
def _profile_http_paths(creds: DatabricksCredentials) -> list[str]:
190+
paths: list[str] = []
191+
default = getattr(creds, "http_path", None)
192+
if default:
193+
paths.append(default)
194+
for cfg in (getattr(creds, "compute", None) or {}).values():
195+
path = cfg.get("http_path") if cfg else None
196+
if path:
197+
paths.append(path)
198+
return paths
199+
200+
189201
def build_connection_config(creds: DatabricksCredentials) -> models.ConnectionConfig:
190202
http_path = getattr(creds, "http_path", None)
191203
connection_parameters = getattr(creds, "connection_parameters", None) or {}
@@ -194,7 +206,9 @@ def build_connection_config(creds: DatabricksCredentials) -> models.ConnectionCo
194206
configured_auth_family=classify_auth_family(creds),
195207
named_compute_count=len(getattr(creds, "compute", None) or {}),
196208
# Parse only the `o` parameter; discard its value.
197-
spog_routing_configured=extract_workspace_id(http_path) is not None,
209+
spog_routing_configured=any(
210+
extract_workspace_id(path) is not None for path in _profile_http_paths(creds)
211+
),
198212
use_kernel=bool(connection_parameters.get("use_kernel")),
199213
)
200214

dbt/adapters/databricks/telemetry/coordinator.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ def set_transport(self, invocation_id: str, transport: Transport) -> None:
119119

120120
def mark_start(self, invocation_id: str) -> None:
121121
with self._lock:
122+
self._states = {
123+
key: state
124+
for key, state in self._states.items()
125+
if key == invocation_id or not state.closed
126+
}
122127
self._state(invocation_id)
123128

124129
def is_closed(self, invocation_id: str) -> bool:

dbt/adapters/databricks/telemetry/hooks.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from typing import Any, Optional
33

44
from dbt.adapters.databricks.credentials import DatabricksCredentials
5+
from dbt.adapters.databricks.spog.extract import extract_workspace_id
56
from dbt.adapters.databricks.telemetry import builder, listener
67
from dbt.adapters.databricks.telemetry.config import (
78
has_reusable_transport,
@@ -73,6 +74,7 @@ def on_post_parse(adapter: Any, manifest: Any) -> None:
7374
def on_connection_open(
7475
credentials: Optional[DatabricksCredentials],
7576
credentials_manager: Optional[Any],
77+
http_path: Optional[str] = None,
7678
) -> None:
7779
try:
7880
if (
@@ -84,10 +86,13 @@ def on_connection_open(
8486
invocation_id = _current_invocation_id()
8587
if not invocation_id:
8688
return
89+
workspace_id = extract_workspace_id(http_path)
90+
if workspace_id is None:
91+
workspace_id = getattr(credentials_manager, "workspace_id", None)
8792
transport = Transport(
8893
host=getattr(credentials_manager, "host", None),
8994
header_factory=getattr(credentials_manager, "header_factory", None),
90-
workspace_id=getattr(credentials_manager, "workspace_id", None),
95+
workspace_id=workspace_id,
9196
)
9297
coordinator().set_transport(invocation_id, transport)
9398
except Exception: # pragma: no cover - best-effort

tests/unit/telemetry/test_builder.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,16 @@ def test_spog_parameter_is_parsed_not_substring_matched(self):
162162
)
163163
assert cc.spog_routing_configured is False
164164

165+
def test_named_compute_o_parameter_sets_spog_flag(self):
166+
cc = builder.build_connection_config(
167+
_creds(
168+
token="dapi",
169+
http_path="/sql/1.0/warehouses/default",
170+
compute={"named": {"http_path": "/sql/1.0/warehouses/named?o=42"}},
171+
)
172+
)
173+
assert cc.spog_routing_configured is True
174+
165175

166176
class TestAggregateManifest:
167177
def _manifest(self):

tests/unit/telemetry/test_coordinator.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,20 @@ def start(self):
187187

188188
assert len(capture.calls) == 2
189189

190+
def test_mark_start_prunes_closed_invocations(self):
191+
c = coord_mod.Coordinator()
192+
c.mark_start("inv-1")
193+
c.close("inv-1")
194+
c.mark_start("inv-2")
195+
196+
assert "inv-1" not in c._states
197+
assert "inv-2" in c._states
198+
assert c.needs_post_parse("inv-2") is True
199+
200+
c.close("inv-1")
201+
assert c._states["inv-1"].closed
202+
assert not c._states["inv-2"].closed
203+
190204

191205
class TestPostRun:
192206
def _entry(self, call):

tests/unit/telemetry/test_hooks.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,33 @@ def test_run_end_exception_finalizes_stored_invocation_not_current_global(monkey
7676
assert build.call_args.args[0] == "inv-1"
7777
coord.set_post_run.assert_called_once_with("inv-1", "log")
7878
coord.close.assert_called_once_with("inv-1")
79+
80+
81+
def test_connection_open_uses_opened_path_workspace_id(monkeypatch):
82+
coord = Mock()
83+
monkeypatch.setattr(hooks, "coordinator", lambda: coord)
84+
monkeypatch.setattr(hooks, "_current_invocation_id", lambda: "inv-1")
85+
monkeypatch.setattr(hooks, "is_enabled_for_invocation", lambda _: True)
86+
monkeypatch.setattr(hooks, "has_reusable_transport", lambda _: True)
87+
manager = SimpleNamespace(host="https://h", header_factory=lambda: {}, workspace_id=None)
88+
89+
hooks.on_connection_open(
90+
SimpleNamespace(), manager, "/sql/1.0/warehouses/named?o=42"
91+
)
92+
93+
transport = coord.set_transport.call_args.args[1]
94+
assert transport.workspace_id == "42"
95+
96+
97+
def test_connection_open_falls_back_to_manager_workspace_id(monkeypatch):
98+
coord = Mock()
99+
monkeypatch.setattr(hooks, "coordinator", lambda: coord)
100+
monkeypatch.setattr(hooks, "_current_invocation_id", lambda: "inv-1")
101+
monkeypatch.setattr(hooks, "is_enabled_for_invocation", lambda _: True)
102+
monkeypatch.setattr(hooks, "has_reusable_transport", lambda _: True)
103+
manager = SimpleNamespace(host="https://h", header_factory=lambda: {}, workspace_id="7")
104+
105+
hooks.on_connection_open(SimpleNamespace(), manager, "/sql/1.0/warehouses/default")
106+
107+
transport = coord.set_transport.call_args.args[1]
108+
assert transport.workspace_id == "7"

0 commit comments

Comments
 (0)