diff --git a/solutions/ess-maker-skills/scripts/adk_telemetry.py b/solutions/ess-maker-skills/scripts/adk_telemetry.py index 842d34b4..613ad92f 100644 --- a/solutions/ess-maker-skills/scripts/adk_telemetry.py +++ b/solutions/ess-maker-skills/scripts/adk_telemetry.py @@ -77,7 +77,9 @@ # 1.4.0: added ``toolkit_git_sha`` + ``toolkit_git_branch`` common # dimensions for precise upgrade-posture reporting and CA-vs-DA # attribution — ADO 7943642. -SCHEMA_VERSION = "1.4.0" +# 1.5.0: added derived ``connector`` (workday|servicenow|"") on +# adk.flightcheck.run/result + adk.capability.use — ADO 7943641. +SCHEMA_VERSION = "1.5.0" # Surfaces the ADK emits from (spec enum: sdk | cli | studio | docs). The # Python skill scripts are the CLI surface. @@ -236,6 +238,55 @@ def normalize_capability(capability: str) -> str: return c if c in _CAPABILITY_SET else CAPABILITY_UNKNOWN +# --- Connector taxonomy (ADO 7943641) ------------------------------------- +# Attribute Connect + FlightCheck usage to the specific backend HR system so +# Workday vs ServiceNow adoption / reliability can be reported separately +# instead of collapsed under a single generic "connect" wedge. Bounded enum +# keeps the dashboard dimension controlled (cardinality never grows). +# +# Values: +# workday -> Workday connect flow, Workday-scope FlightCheck runs, +# checks in the Workday / Workday Tenant / Workday Extension +# categories. +# servicenow -> ServiceNow connect flow, ServiceNow-scope FlightCheck runs, +# checks in the ServiceNow category. +# legacy -> Explicit label for older events that emitted the generic +# "connect" capability WITHOUT a connector arg. Emitted by +# emit_capability.py when the caller passed no --connector +# flag AND the capability is one that a future maker MIGHT +# have connector context for (today: "connect"). Keeps the +# pre-attribution corpus queryable as its own bucket rather +# than double-counted against a real connector. +# unknown -> Out-of-taxonomy value provided by the caller (typo, +# future-connector name not yet in the enum). +# "" -> Legitimately not connector-scoped (most capabilities, +# checks in Environment / Authentication / Prerequisites / +# Local Files categories, "full"-scope FlightCheck runs +# that span multiple connectors). +CONNECTORS = ("workday", "servicenow") +_CONNECTOR_SET = frozenset(CONNECTORS) +CONNECTOR_LEGACY = "legacy" +CONNECTOR_UNKNOWN = "unknown" + + +def normalize_connector(connector: str) -> str: + """Normalize a ``connector`` value to the canonical enum. + + Empty stays empty (event is not connector-scoped). + Non-empty inputs are lower-cased / stripped and mapped to themselves if + they are in :data:`CONNECTORS`, to :data:`CONNECTOR_LEGACY` if the caller + explicitly passed that sentinel, else to :data:`CONNECTOR_UNKNOWN`. + """ + if not connector: + return "" + c = str(connector).strip().lower() + if c in _CONNECTOR_SET: + return c + if c == CONNECTOR_LEGACY: + return CONNECTOR_LEGACY + return CONNECTOR_UNKNOWN + + # Outcomes the spec treats as errors (must carry error_* fields). _ERROR_OUTCOMES = frozenset( {"client_error", "server_error", "timeout", "abandoned", "failure", "fail"} @@ -1271,11 +1322,16 @@ def emit_api_call( def emit_capability_use( - adk_capability: str, *, surface: str = SURFACE_CLI, block: bool = False + adk_capability: str, + *, + connector: str = "", + surface: str = SURFACE_CLI, + block: bool = False, ) -> dict[str, Any]: sid, _ = get_session(surface) data = common_dimensions(surface, session_id=sid) data["adk_capability"] = normalize_capability(adk_capability) + data["connector"] = normalize_connector(connector) return _emit(EVENT_CAPABILITY_USE, data, block=block) @@ -1283,6 +1339,7 @@ def emit_flightcheck_run( *, agent_id: str = "", adk_capability: str = "flightcheck", + connector: str = "", run_index: int = 0, surface: str = SURFACE_CLI, block: bool = False, @@ -1292,6 +1349,7 @@ def emit_flightcheck_run( data.update({ "agent_id": agent_id, "adk_capability": normalize_capability(adk_capability), + "connector": normalize_connector(connector), "run_index": int(run_index), }) return _emit(EVENT_FLIGHTCHECK_RUN, data, block=block) @@ -1301,6 +1359,7 @@ def emit_flightcheck_result( *, agent_id: str = "", adk_capability: str = "flightcheck", + connector: str = "", run_index: int = 0, result: str = "pass", duration_ms: int = 0, @@ -1312,6 +1371,7 @@ def emit_flightcheck_result( data.update({ "agent_id": agent_id, "adk_capability": normalize_capability(adk_capability), + "connector": normalize_connector(connector), "run_index": int(run_index), "result": result, "duration_ms": int(duration_ms), @@ -1322,6 +1382,7 @@ def emit_flightcheck_result( def emit_flightcheck_error( *, agent_id: str = "", + connector: str = "", error_code: str = "", error_category: str = "runtime", error_message: str = "", @@ -1330,7 +1391,10 @@ def emit_flightcheck_error( ) -> dict[str, Any]: sid, _ = get_session(surface) data = common_dimensions(surface, session_id=sid) - data.update({"agent_id": agent_id}) + data.update({ + "agent_id": agent_id, + "connector": normalize_connector(connector), + }) _apply_error_fields(data, "server_error", error_code, error_message, error_category) return _emit(EVENT_FLIGHTCHECK_ERROR, data, block=block) diff --git a/solutions/ess-maker-skills/scripts/emit_capability.py b/solutions/ess-maker-skills/scripts/emit_capability.py index ebe9fbe9..cc248b0e 100644 --- a/solutions/ess-maker-skills/scripts/emit_capability.py +++ b/solutions/ess-maker-skills/scripts/emit_capability.py @@ -46,6 +46,34 @@ sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +def _parse_connector(args: list[str]) -> tuple[str, list[str]]: + """Extract ``--connector `` (or ``--connector=``) from args. + + Returns (connector, remaining_args). Missing / malformed flag returns + ("", args) — the caller passes an empty string through, which + ``normalize_connector`` maps to "" (not connector-scoped). + """ + out = [] + connector = "" + i = 0 + while i < len(args): + a = args[i] + if a == "--connector": + if i + 1 < len(args): + connector = args[i + 1] + i += 2 + continue + i += 1 + continue + if a.startswith("--connector="): + connector = a.split("=", 1)[1] + i += 1 + continue + out.append(a) + i += 1 + return connector, out + + def main(argv: list[str]) -> int: args = argv[1:] @@ -57,9 +85,14 @@ def main(argv: list[str]) -> int: if not args or args[0] in ("-h", "--help"): print( - "Usage: python scripts/emit_capability.py \n" + "Usage: python scripts/emit_capability.py " + "[--connector ]\n" "Records that a maker used an ADK capability (best-effort, " "non-blocking).\n\n" + "The optional --connector flag attributes Connect + FlightCheck " + "usage to the specific backend HR system (Workday vs ServiceNow) " + "so PMs can report adoption / reliability by connector. Pass " + "'legacy' for older events that predate connector attribution.\n\n" "Valid capabilities:\n " + "\n ".join(adk_telemetry.ADK_CAPABILITIES) ) @@ -70,22 +103,33 @@ def main(argv: list[str]) -> int: print(cap) return 0 - if args[0] == "--worker": - if len(args) < 2: + # Extract --connector from anywhere in the argv tail so callers can put it + # before or after the capability positional. Worker mode uses the same + # parser so the subprocess round-trip preserves the flag. + connector, rest = _parse_connector(args) + + if rest and rest[0] == "--worker": + if len(rest) < 2: return 0 try: - adk_telemetry.emit_capability_use(args[1].strip(), block=True) + adk_telemetry.emit_capability_use( + rest[1].strip(), connector=connector, block=True + ) except Exception: # noqa: BLE001 — telemetry must never break a skill pass return 0 - capability = args[0].strip() + if not rest: + return 0 + capability = rest[0].strip() if not adk_telemetry.telemetry_enabled(): return 0 if adk_telemetry._SYNC: try: - adk_telemetry.emit_capability_use(capability, block=True) + adk_telemetry.emit_capability_use( + capability, connector=connector, block=True + ) except Exception: # noqa: BLE001 — telemetry must never break a skill pass return 0 @@ -105,10 +149,15 @@ def main(argv: list[str]) -> int: ) else: kwargs["start_new_session"] = True - worker = subprocess.Popen( - [sys.executable, os.path.abspath(__file__), "--worker", capability], - **kwargs, - ) + worker_argv = [ + sys.executable, + os.path.abspath(__file__), + "--worker", + capability, + ] + if connector: + worker_argv.extend(["--connector", connector]) + worker = subprocess.Popen(worker_argv, **kwargs) threading.Thread( target=worker.wait, name="adk-capability-worker-reaper", diff --git a/solutions/ess-maker-skills/scripts/flightcheck/cli.py b/solutions/ess-maker-skills/scripts/flightcheck/cli.py index 426ad533..8f7bccf5 100644 --- a/solutions/ess-maker-skills/scripts/flightcheck/cli.py +++ b/solutions/ess-maker-skills/scripts/flightcheck/cli.py @@ -1135,12 +1135,26 @@ def _run_single_checkpoint(args): # --scope emit so checkpoint runs also count toward the adk.* cubes. try: import adk_telemetry as _adk + from flightcheck.telemetry import derive_connector_from_category _agent_id = _active_agent.get("botId", "") if tenant_id or tenant_name: _adk.set_identity(tenant_id=tenant_id or "", tenant_name=tenant_name) _ridx = _adk.next_run_index(_agent_id) - _adk.emit_flightcheck_run(agent_id=_agent_id, run_index=_ridx) + # Single-checkpoint runs execute exactly one owning check, so the + # first result row's category is the run's connector (or "" for + # cross-cutting checkpoints like Environment / Authentication). + # Derived here rather than passed by the caller so the CLI runtime + # path matches the same connector attribution as the legacy + # ESSMakerKit.FlightCheck.* events (ADO 7943641 review). + _connector = "" + if result.results: + _connector = derive_connector_from_category( + getattr(result.results[0], "category", "") or "" + ) + _adk.emit_flightcheck_run( + agent_id=_agent_id, run_index=_ridx, connector=_connector + ) _result_map = { "READY": "pass", "READY_WITH_WARNINGS": "partial", @@ -1151,6 +1165,7 @@ def _run_single_checkpoint(args): run_index=_ridx, result=_result_map.get(result.overall, "fail"), duration_ms=int(getattr(result, "duration_secs", 0) * 1000), + connector=_connector, ) _adk.flush(timeout=3) except Exception: # noqa: BLE001 — adk telemetry must never break the run @@ -1778,12 +1793,20 @@ def main(): # the legacy ESSMakerKit.FlightCheck.* events; never affects the run. try: import adk_telemetry as _adk + from flightcheck.telemetry import derive_connector_from_scope _agent_id = active_agent.get("botId", "") if tenant_id or tenant_name: _adk.set_identity(tenant_id=tenant_id, tenant_name=tenant_name) _ridx = _adk.next_run_index(_agent_id) - _adk.emit_flightcheck_run(agent_id=_agent_id, run_index=_ridx) + # Derive connector from the CLI scope so scope-based runs get the + # same attribution as the legacy flightcheck events (ADO 7943641 + # review). "full" and cross-cutting scopes return "" — the finer + # per-check attribution lives on the check events, not run events. + _connector = derive_connector_from_scope(args.scope) + _adk.emit_flightcheck_run( + agent_id=_agent_id, run_index=_ridx, connector=_connector + ) _result_map = { "READY": "pass", "READY_WITH_WARNINGS": "partial", @@ -1794,6 +1817,7 @@ def main(): run_index=_ridx, result=_result_map.get(result.overall, "fail"), duration_ms=int(getattr(result, "duration_secs", 0) * 1000), + connector=_connector, ) _adk.flush(timeout=3) except Exception: # noqa: BLE001 — adk telemetry must never break the run diff --git a/solutions/ess-maker-skills/scripts/flightcheck/telemetry.py b/solutions/ess-maker-skills/scripts/flightcheck/telemetry.py index f13708aa..504e90a2 100644 --- a/solutions/ess-maker-skills/scripts/flightcheck/telemetry.py +++ b/solutions/ess-maker-skills/scripts/flightcheck/telemetry.py @@ -99,8 +99,10 @@ # Bump when the emitted field set changes so dashboards can version-gate. # 1.1: added derived ``tenantClass`` (internal vs customer) — ADO 7558661. # 1.2: added ``toolkitGitSha`` + ``toolkitGitBranch`` for precise -# upgrade-posture and CA-vs-DA attribution — ADO 7943642. -TELEMETRY_SCHEMA_VERSION = "1.2" +# upgrade-posture and CA-vs-DA attribution — ADO 7943642. +# 1.3: added derived ``connector`` (workday|servicenow|"") on run + check — +# ADO 7943641. +TELEMETRY_SCHEMA_VERSION = "1.3" # Short, fail-open timeout (connect, read) seconds. Telemetry runs at the # very end of a FlightCheck; we never want it to hang the CLI. @@ -817,6 +819,75 @@ def derive_run_outcome(run_result: Any) -> str: return RUN_OUTCOME_READY +# --- Connector attribution (ADO 7943641) ---------------------------------- +# Derive a bounded ``connector`` value (workday | servicenow | "") from the +# run scope (for the run event) and from the check's category (for each check +# event) so Aria can split adoption / reliability by backend HR system +# instead of collapsing everything under the FlightCheck-wide donut. +# +# Emitted at telemetry time (not stamped on the CheckResult in-process) +# because 1DS RTA cubes cannot compute one dimension from another; the +# derivation stays here alongside classify_tenant / derive_run_outcome. + +# Scopes explicitly bound to a single connector. "full" spans multiple +# connectors -> "" (drill down via the check-level connector dimension). +# Local / infra / auth / etc. are not connector-scoped. +_WORKDAY_SCOPES = frozenset({ + "workday", "workdaytenant", "workdayextension", + # Also connector-scoped for Workday even though the naming doesn't lead + # with the "workday" token: the "Workday DA" scope (workdayda) and the + # topic-authoring scope (topics, which SCOPE_MAP labels "Workday Topics") + # both exercise Workday paths exclusively. Missing these here left real + # Workday runs emitting connector="" (ADO 7943641 review). + "workdayda", "topics", +}) +_SERVICENOW_SCOPES = frozenset({"servicenow"}) + +# Check categories from checks/*.py. Category strings are set at CheckResult +# construction time (e.g. category="Workday", "Workday Tenant", "ServiceNow"). +# Match on the leading token so future subcategories ("Workday Workflows", +# "Workday Extension", "ServiceNow HRSD") inherit the same attribution +# without requiring a taxonomy edit here. +_WORKDAY_CATEGORY_PREFIX = "workday" +_SERVICENOW_CATEGORY_PREFIX = "servicenow" + + +def derive_connector_from_scope(scope: str) -> str: + """Return "workday" / "servicenow" / "" for a FlightCheck ``--scope`` value. + + "full" and other cross-connector scopes return "" (empty). The per-check + ``connector`` field carries the finer-grained attribution. + """ + if not scope: + return "" + s = str(scope).strip().lower() + if s in _WORKDAY_SCOPES: + return "workday" + if s in _SERVICENOW_SCOPES: + return "servicenow" + return "" + + +def derive_connector_from_category(category: str) -> str: + """Return "workday" / "servicenow" / "" for a CheckResult ``category``. + + Matches the leading token so "Workday", "Workday Tenant", "Workday + Extension", "Workday Workflows", "ServiceNow", "ServiceNow HRSD", etc. + all attribute correctly. Cross-cutting categories (Environment, + Authentication, Prerequisites, Local Files, Publishing, External + Systems, Licensing, Solution, Topics, Configuration) return "" — they + aren't scoped to a single backend. + """ + if not category: + return "" + c = str(category).strip().lower() + if c.startswith(_WORKDAY_CATEGORY_PREFIX): + return "workday" + if c.startswith(_SERVICENOW_CATEGORY_PREFIX): + return "servicenow" + return "" + + def _run_data( run_result: Any, *, @@ -845,6 +916,7 @@ def _run_data( "toolkitGitBranch": get_toolkit_git_branch(), "scope": scope, "invocationSource": invocation_source, + "connector": derive_connector_from_scope(scope), # derived: workday|servicenow|"" "overall": getattr(run_result, "overall", ""), "runOutcome": derive_run_outcome(run_result), # verdict donut split (errored|failed|warnings|ready) "durationSecs": getattr(run_result, "duration_secs", 0), @@ -871,6 +943,7 @@ def _check_data( tenant_name: str = "", ) -> dict[str, Any]: # Identifiers + enums ONLY. Never `result` / `remediation` (EUII risk). + _category = getattr(check, "category", "") return { "schemaVersion": TELEMETRY_SCHEMA_VERSION, "env": env, @@ -880,7 +953,8 @@ def _check_data( "tenantClass": classify_tenant(tenant_id), "tenantName": tenant_name, "checkpointId": getattr(check, "checkpoint_id", ""), - "category": getattr(check, "category", ""), + "category": _category, + "connector": derive_connector_from_category(_category), # derived "priority": getattr(check, "priority", ""), "status": getattr(check, "status", ""), "roles": ", ".join(getattr(check, "roles", []) or []), diff --git a/solutions/ess-maker-skills/scripts/telemetry_queries.kql b/solutions/ess-maker-skills/scripts/telemetry_queries.kql index 48a672a5..d0a163d7 100644 --- a/solutions/ess-maker-skills/scripts/telemetry_queries.kql +++ b/solutions/ess-maker-skills/scripts/telemetry_queries.kql @@ -183,6 +183,72 @@ essmakerkit_flightcheck_run p99 = percentile(durationSecs, 99) +// ----------------------------------------------------------------------------- +// 9) CONNECTOR ATTRIBUTION (ADO 7943641) +// Split Connect + FlightCheck usage/reliability by connector so PMs can +// report Workday vs ServiceNow adoption and reliability separately. +// +// Dim: connector = { "workday", "servicenow", "legacy", "unknown", "" } +// * "" (empty) = the event is not connector-scoped (topic authoring, +// workflow deletion, cross-cutting FlightCheck runs with scope="full", +// cross-cutting check categories like Environment/Authentication). +// * "legacy" = pre-attribution "connect" events (schema < 1.4.0). +// * "unknown" = caller passed an out-of-taxonomy value; investigate. +// +// NOTE: for FlightCheck the "connector" dim is DERIVED on the emit side, +// not passed by the maker: run event uses scope ("workday" / "servicenow"), +// check event uses category prefix. Cross-cutting categories emit "". +// ----------------------------------------------------------------------------- + +// 9a) Connect starts by connector (Workday vs ServiceNow adoption). +adk_capability_use +| where EventInfo_Time > ago(30d) +| where adk_capability == "connect" +| summarize Starts = count(), Tenants = dcount(tenant_id) by connector +| order by Starts desc + +// 9b) FlightCheck runs by connector — dedicated Workday / ServiceNow scopes +// surface separately from cross-cutting "full" runs (connector=""). +essmakerkit_flightcheck_run +| where EventInfo_Time > ago(30d) +| summarize Runs = count(), + Tenants = dcount(tenantId), + SuccessPct = round(100.0 * countif(overall == "READY") / count(), 1) + by connector +| order by Runs desc + +// 9c) Per-check reliability split by connector — the fine-grained view. +// "" is expected on cross-cutting categories (Environment / Auth / +// Prerequisites / etc.). +essmakerkit_flightcheck_check +| where EventInfo_Time > ago(30d) +| where status in ("Passed", "Failed", "Error") +| summarize Total = count(), + Passed = countif(status == "Passed"), + SuccessPct = round(100.0 * countif(status == "Passed") / count(), 1) + by connector, category +| order by connector asc, SuccessPct asc + +// 9d) Legacy-vs-attributed rollout: watch "legacy" go to zero over time as +// old ADK installs upgrade past 1.4.0. Buckets are null-safe: pre-1.4.0 +// events land in Kusto with no ``connector`` column at all (isnull) and +// 1.4.0+ non-connector-scoped events emit ``connector=""`` (isempty); +// both collapse into "unattributed". ``unknown`` is called out +// separately so a caller passing an out-of-taxonomy value stays +// visible instead of being folded into "attributed". +adk_capability_use +| where EventInfo_Time > ago(30d) +| where adk_capability == "connect" +| summarize Events = count() by bin(EventInfo_Time, 1d), + bucket = case( + isnull(connector) or isempty(connector), + "unattributed", + connector == "legacy", "legacy", + connector == "unknown", "unknown", + "attributed") +| order by EventInfo_Time asc + + // ----------------------------------------------------------------------------- // 8) VORPAL CLIENT EVENTS (schemaVersion 2) // ----------------------------------------------------------------------------- diff --git a/solutions/ess-maker-skills/src/skills/connect/SKILL.md b/solutions/ess-maker-skills/src/skills/connect/SKILL.md index 5a08c78f..df8406c2 100644 --- a/solutions/ess-maker-skills/src/skills/connect/SKILL.md +++ b/solutions/ess-maker-skills/src/skills/connect/SKILL.md @@ -8,15 +8,15 @@ or what files you are reading. ## Start -Record anonymous usage telemetry (best-effort, non-blocking — no user-facing -message, and it never fails the step): `python scripts/emit_capability.py connect` - If the user specified an integration as an argument (e.g., the user said "servicenow" or "workday", or the prompt was invoked as `/connect servicenow`), pass it to step1 as PRE_SELECTED_INTEGRATION. Step1 will skip the "which system" question and go directly to routing for that integration. -Read `src/skills/connect/step1.md` and follow it. +Read `src/skills/connect/step1.md` and follow it. That file records anonymous +usage telemetry after routing knows which integration was chosen, so the +Connect capability event carries the correct `connector` attribution +(workday vs servicenow) rather than being a generic "connect" wedge. (Step 1 asks which integration, detects existing state, and dispatches — ServiceNow to its own step files; Workday first by agent architecture, then diff --git a/solutions/ess-maker-skills/src/skills/connect/step1.md b/solutions/ess-maker-skills/src/skills/connect/step1.md index c7ba25d9..cf323e97 100644 --- a/solutions/ess-maker-skills/src/skills/connect/step1.md +++ b/solutions/ess-maker-skills/src/skills/connect/step1.md @@ -57,6 +57,10 @@ Wait for the user to respond. ### If the user chose ServiceNow (1 or "servicenow") +Record anonymous usage telemetry attributed to ServiceNow (best-effort, +non-blocking — no user-facing message, and it never fails the step): +`python scripts/emit_capability.py connect --connector servicenow` + Check if `.local/connect/servicenow/steps.md` exists. **If it exists and all items are checked:** @@ -208,6 +212,10 @@ Now read `src/skills/connect/servicenow/step1.md` and follow it. ### If the user chose Workday (2 or "workday") +Record anonymous usage telemetry attributed to Workday (best-effort, +non-blocking — no user-facing message, and it never fails the step): +`python scripts/emit_capability.py connect --connector workday` + Read `.local/config.json`, resolve `activeAgent` against `agents`, and fall back to the legacy `agent` object only when needed. For a DA agent, also read the canonical `.local/setup/config.json` `agents` record keyed by the active diff --git a/tests/flightcheck/test_cli_single_checkpoint.py b/tests/flightcheck/test_cli_single_checkpoint.py index 95e5be98..155e5136 100644 --- a/tests/flightcheck/test_cli_single_checkpoint.py +++ b/tests/flightcheck/test_cli_single_checkpoint.py @@ -705,6 +705,7 @@ def test_no_telemetry_flag_suppresses_emit( ) assert captured["called"] is False + def test_tenant_name_falls_back_to_cache_when_graph_unavailable( self, tmp_path: Path, @@ -783,3 +784,93 @@ def get_organization(self): # consulted so a previously-seen tenant still gets its display name. assert captured["kwargs"]["tenant_name"] == "Contoso Cached" assert captured["kwargs"]["tenant_id"] == cached_tid + + +class TestCheckpointAdkConnector: + """Single-checkpoint runs on the CLI runtime path must derive the + connector from the owning check's category and forward it to the ADK + ``emit_flightcheck_run`` / ``emit_flightcheck_result`` calls (ADO 7943641 + review, finding 1). Without this, only the legacy + ``ESSMakerKit.FlightCheck.*`` events were attributed and the ADK + ``adk.flightcheck.*`` event family emitted an empty connector for real + runs even though the standalone helper tests exercised the kwarg. + """ + + @staticmethod + def _row_with_category( + checkpoint_id: str, category: str, status: str = Status.PASSED.value + ) -> CheckResult: + return CheckResult( + checkpoint_id=checkpoint_id, + category=category, + priority=Priority.MEDIUM.value, + status=status, + description="fake", + result="fake", + ) + + @staticmethod + def _capture(monkeypatch: pytest.MonkeyPatch) -> dict: + from flightcheck import telemetry as _tele_mod + import adk_telemetry as _adk_mod + + captured: dict = {"run_kwargs": None, "result_kwargs": None} + + def _fake_run(**kwargs): + captured["run_kwargs"] = kwargs + + def _fake_result(**kwargs): + captured["result_kwargs"] = kwargs + + monkeypatch.setattr( + _tele_mod, + "emit_flightcheck_telemetry", + lambda *_a, **_k: {"sent": False, "events": 0, "status": None, + "env": "dev", "reason": "test"}, + ) + monkeypatch.setattr(_adk_mod, "set_identity", lambda *a, **k: None) + monkeypatch.setattr(_adk_mod, "next_run_index", lambda *a, **k: 1) + monkeypatch.setattr(_adk_mod, "emit_flightcheck_run", _fake_run) + monkeypatch.setattr(_adk_mod, "emit_flightcheck_result", _fake_result) + monkeypatch.setattr(_adk_mod, "flush", lambda *a, **k: None) + return captured + + def test_workday_category_row_forwards_workday_connector( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, _silence_output: None, + ) -> None: + monkeypatch.chdir(tmp_path) + TestHermeticRun._install_fake_plan( + monkeypatch, [self._row_with_category("WD-CFG-001", "Workday")], + ) + captured = self._capture(monkeypatch) + with pytest.raises(SystemExit): + cli._run_single_checkpoint(_args("WD-CFG-001", tmp_path, no_telemetry=False)) + assert captured["run_kwargs"] is not None + assert captured["run_kwargs"]["connector"] == "workday" + assert captured["result_kwargs"]["connector"] == "workday" + + def test_servicenow_subcategory_row_forwards_servicenow_connector( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, _silence_output: None, + ) -> None: + monkeypatch.chdir(tmp_path) + TestHermeticRun._install_fake_plan( + monkeypatch, [self._row_with_category("SN-HRSD-001", "ServiceNow HRSD")], + ) + captured = self._capture(monkeypatch) + with pytest.raises(SystemExit): + cli._run_single_checkpoint(_args("SN-HRSD-001", tmp_path, no_telemetry=False)) + assert captured["run_kwargs"]["connector"] == "servicenow" + assert captured["result_kwargs"]["connector"] == "servicenow" + + def test_cross_cutting_category_row_forwards_empty_connector( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, _silence_output: None, + ) -> None: + monkeypatch.chdir(tmp_path) + TestHermeticRun._install_fake_plan( + monkeypatch, [self._row_with_category("ENV-001", "Environment")], + ) + captured = self._capture(monkeypatch) + with pytest.raises(SystemExit): + cli._run_single_checkpoint(_args("ENV-001", tmp_path, no_telemetry=False)) + assert captured["run_kwargs"]["connector"] == "" + assert captured["result_kwargs"]["connector"] == "" diff --git a/tests/flightcheck/test_telemetry.py b/tests/flightcheck/test_telemetry.py index 9027c021..d21ad510 100644 --- a/tests/flightcheck/test_telemetry.py +++ b/tests/flightcheck/test_telemetry.py @@ -373,8 +373,8 @@ def test_classify_branch_privacy_bounded(): def test_telemetry_schema_version_bumped_for_toolkit_git_fields(): - """Version-gate the new dimensions so dashboards can pin on schema 1.2.""" - assert telemetry.TELEMETRY_SCHEMA_VERSION == "1.2" + """Version-gate the new dimensions so dashboards can pin on schema 1.3.""" + assert telemetry.TELEMETRY_SCHEMA_VERSION == "1.3" def test_derive_run_outcome_precedence(): @@ -570,3 +570,128 @@ def fake_post(*a, **k): assert out["sent"] is False assert out["reason"] == "disabled" assert called["n"] == 0 + + +# --- connector derivation (ADO 7943641) ----------------------------------- +@pytest.mark.parametrize("scope,expected", [ + ("workday", "workday"), + ("workdaytenant", "workday"), + ("workdayextension", "workday"), + # ADO 7943641 review — SCOPE_MAP also defines these Workday-only scopes; + # earlier revisions left them attributing to "" which under-counted real + # Workday runs on the connector-adoption rollups. + ("workdayda", "workday"), + ("topics", "workday"), # SCOPE_MAP label: "Workday Topics" + ("Workday", "workday"), + (" workday ", "workday"), + ("WORKDAYDA", "workday"), # case-insensitive + (" topics ", "workday"), # whitespace-tolerant + ("servicenow", "servicenow"), + ("ServiceNow", "servicenow"), +]) +def test_derive_connector_from_scope_known(scope, expected): + assert telemetry.derive_connector_from_scope(scope) == expected + + +@pytest.mark.parametrize("scope", [ + "full", # cross-connector run: per-check attribution wins + "authentication", + "environment", + "external", + "local", + "publishing", + "entraapp", + "", + None, +]) +def test_derive_connector_from_scope_non_connector_is_empty(scope): + assert telemetry.derive_connector_from_scope(scope) == "" + + +@pytest.mark.parametrize("category,expected", [ + ("Workday", "workday"), + ("Workday Tenant", "workday"), + ("Workday Extension", "workday"), + ("Workday Workflows", "workday"), + ("workday", "workday"), + ("ServiceNow", "servicenow"), + ("ServiceNow HRSD", "servicenow"), + ("servicenow", "servicenow"), +]) +def test_derive_connector_from_category_known(category, expected): + assert telemetry.derive_connector_from_category(category) == expected + + +@pytest.mark.parametrize("category", [ + "Environment", "Authentication", "Prerequisites", "Local Files", + "Publishing", "External Systems", "Licensing", "Solution", "Topics", + "Configuration", "", None, +]) +def test_derive_connector_from_category_cross_cutting_is_empty(category): + # Cross-cutting checks intentionally do not attribute to a connector so + # per-connector rollups aren't inflated by shared prerequisites. + assert telemetry.derive_connector_from_category(category) == "" + + +def test_run_event_carries_connector_from_scope(): + events = telemetry.build_events( + FakeRun(), + env="dev", + instance_id="i", + tenant_id="00000000-0000-0000-0000-0000000000ab", + tenant_name="Contoso", + agent_id="a", + agent_count=1, + scope="workday", + invocation_source="cli", + ikey_envelope=f"o:{DEV_TOKEN}", + run_id="r", + ) + assert events[0]["data"]["connector"] == "workday" + + +def test_run_event_connector_empty_for_full_scope(): + events = telemetry.build_events( + FakeRun(), + env="dev", + instance_id="i", + tenant_id="00000000-0000-0000-0000-0000000000ab", + tenant_name="Contoso", + agent_id="a", + agent_count=1, + scope="full", + invocation_source="cli", + ikey_envelope=f"o:{DEV_TOKEN}", + run_id="r", + ) + # "full" scope leaves the run-level connector empty — per-check + # categories give the finer split downstream. + assert events[0]["data"]["connector"] == "" + + +def test_check_events_carry_connector_from_category(): + run = FakeRun(results=[ + FakeCheck(checkpoint_id="WD-1", category="Workday Tenant"), + FakeCheck(checkpoint_id="SN-1", category="ServiceNow HRSD"), + FakeCheck(checkpoint_id="AUTH-1", category="Authentication"), + ], total=3, passed=3, failed=0) + events = telemetry.build_events( + run, + env="dev", + instance_id="i", + tenant_id="00000000-0000-0000-0000-0000000000ab", + tenant_name="Contoso", + agent_id="a", + agent_count=1, + scope="full", + invocation_source="cli", + ikey_envelope=f"o:{DEV_TOKEN}", + run_id="r", + ) + check_events = [e for e in events if e["name"] == telemetry.EVENT_CHECK] + connectors = [e["data"]["connector"] for e in check_events] + assert connectors == ["workday", "servicenow", ""] + + +def test_schema_version_bump_records_connector_dim(): + assert telemetry.TELEMETRY_SCHEMA_VERSION == "1.3" diff --git a/tests/scripts/test_emit_capability.py b/tests/scripts/test_emit_capability.py index 2cc2442a..dc2d85e5 100644 --- a/tests/scripts/test_emit_capability.py +++ b/tests/scripts/test_emit_capability.py @@ -54,6 +54,111 @@ def start(self): assert waits == [True] +def test_capability_emit_parent_forwards_connector_argv(monkeypatch) -> None: + # PARENT-PATH attribution round-trip (ADO 7943641): when the maker + # invokes ``emit_capability.py connect --connector workday``, the + # detached worker subprocess MUST be spawned with ``--connector + # workday`` in its argv so the worker's ``emit_capability_use`` call + # carries the attribution. Missing this on the parent leg silently + # dropped attribution before the worker even ran; the + # ``test_worker_emits_synchronously_with_connector`` test below only + # covers the worker leg of the same round-trip. + calls = [] + + def fake_popen(command, **kwargs): + calls.append((command, kwargs)) + return type("Worker", (), {"wait": lambda self: None})() + + class FakeThread: + def __init__(self, *, target, name, daemon): + assert daemon is True + self.target = target + + def start(self): + self.target() + + monkeypatch.setattr(emit_capability.subprocess, "Popen", fake_popen) + monkeypatch.setattr(emit_capability.threading, "Thread", FakeThread) + monkeypatch.setattr("adk_telemetry.telemetry_enabled", lambda: True) + monkeypatch.setattr("adk_telemetry._SYNC", False) + + result = emit_capability.main([ + "emit_capability.py", + "connect", + "--connector", + "workday", + ]) + + assert result == 0 + assert len(calls) == 1 + command, _kwargs = calls[0] + assert command == [ + sys.executable, + os.path.abspath(emit_capability.__file__), + "--worker", + "connect", + "--connector", + "workday", + ] + + # ``--connector=`` form and connector-before-capability ordering + # must also propagate — the shim parses both variants but the argv sent + # to the worker is canonicalized to the "--connector " form. + calls.clear() + result = emit_capability.main([ + "emit_capability.py", + "--connector=servicenow", + "connect", + ]) + assert result == 0 + assert len(calls) == 1 + command, _kwargs = calls[0] + assert command == [ + sys.executable, + os.path.abspath(emit_capability.__file__), + "--worker", + "connect", + "--connector", + "servicenow", + ] + + +def test_capability_emit_parent_omits_connector_when_not_supplied( + monkeypatch, +) -> None: + # Negative half of the parent-path attribution assertion: with no + # ``--connector`` flag on the parent invocation, the worker argv must + # contain no ``--connector`` at all so worker parsing falls through to + # ``connector=""`` (not to an inadvertent ``"unknown"`` normalization + # from an empty positional). + calls = [] + + def fake_popen(command, **kwargs): + calls.append((command, kwargs)) + return type("Worker", (), {"wait": lambda self: None})() + + class FakeThread: + def __init__(self, *, target, name, daemon): + assert daemon is True + self.target = target + + def start(self): + self.target() + + monkeypatch.setattr(emit_capability.subprocess, "Popen", fake_popen) + monkeypatch.setattr(emit_capability.threading, "Thread", FakeThread) + monkeypatch.setattr("adk_telemetry.telemetry_enabled", lambda: True) + monkeypatch.setattr("adk_telemetry._SYNC", False) + + result = emit_capability.main(["emit_capability.py", "topic_create"]) + + assert result == 0 + assert len(calls) == 1 + command, _kwargs = calls[0] + assert "--connector" not in command + assert command[-1] == "topic_create" + + def test_capability_emit_does_not_spawn_when_disabled(monkeypatch) -> None: monkeypatch.setattr( "adk_telemetry.telemetry_enabled", @@ -73,7 +178,9 @@ def test_worker_emits_synchronously(monkeypatch) -> None: monkeypatch.setattr( "adk_telemetry.emit_capability_use", - lambda capability, block: emitted.append((capability, block)), + lambda capability, connector, block: emitted.append( + (capability, connector, block) + ), ) result = emit_capability.main([ @@ -83,4 +190,30 @@ def test_worker_emits_synchronously(monkeypatch) -> None: ]) assert result == 0 - assert emitted == [("setup", True)] + assert emitted == [("setup", "", True)] + + +def test_worker_emits_synchronously_with_connector(monkeypatch) -> None: + # Attribution round-trip: parent shim -> detached worker subprocess -> + # emit_capability_use must carry the ``--connector`` value through the + # subprocess argv (ADO 7943641). Missing the flag was silently emitting + # the Connect capability with no attribution. + emitted = [] + + monkeypatch.setattr( + "adk_telemetry.emit_capability_use", + lambda capability, connector, block: emitted.append( + (capability, connector, block) + ), + ) + + result = emit_capability.main([ + "emit_capability.py", + "--worker", + "connect", + "--connector", + "workday", + ]) + + assert result == 0 + assert emitted == [("connect", "workday", True)] diff --git a/tests/test_adk_telemetry.py b/tests/test_adk_telemetry.py index 6cd45dbb..5582014b 100644 --- a/tests/test_adk_telemetry.py +++ b/tests/test_adk_telemetry.py @@ -1752,7 +1752,12 @@ def test_every_canonical_capability_is_actually_emitted(): for pat in (py_shim_pat, use_pat, kw_pat): for m in pat.finditer(text): emitted.add(m.group(1)) - for path in skills_dir.rglob("SKILL.md"): + # Scan every prompt-file the skills dispatch chain reads, not just + # SKILL.md. Some SKILLs (connect, in particular) defer their emit into a + # step*.md file so the ``--connector`` value can be attached AFTER the + # user picks Workday vs ServiceNow. Restricting the scan to SKILL.md + # would misclassify those deferred capabilities as dead. + for path in skills_dir.rglob("*.md"): text = path.read_text(encoding="utf-8") for m in md_pat.finditer(text): emitted.add(m.group(1)) @@ -1799,7 +1804,9 @@ def test_no_caller_passes_a_noncanonical_capability_to_the_shim(): cap = m.group(1) if cap not in adk.ADK_CAPABILITIES: offenders.append((str(path.relative_to(repo_root)), cap)) - for path in skills_dir.rglob("SKILL.md"): + # Scan every prompt file, not just SKILL.md — deferred emits live in + # step*.md (see the reverse scanner above for the same rationale). + for path in skills_dir.rglob("*.md"): text = path.read_text(encoding="utf-8") for m in md_pat.finditer(text): cap = m.group(1) @@ -2027,3 +2034,153 @@ def test_sanitize_tenant_id_rejects_non_guid(bad): def test_sanitize_tenant_id_preserves_empty(): assert adk._sanitize_tenant_id("") == "" assert adk._sanitize_tenant_id(" ") == "" + + +# --- connector attribution (ADO 7943641) ---------------------------------- +def test_normalize_connector_known_values_pass_through(): + for c in adk.CONNECTORS: + assert adk.normalize_connector(c) == c + + +def test_normalize_connector_empty_stays_empty(): + # Most events legitimately have no connector context (topic authoring, + # workflow deletion, etc.). Empty must NOT coerce to "unknown". + assert adk.normalize_connector("") == "" + assert adk.normalize_connector(None) == "" + + +def test_normalize_connector_case_and_whitespace_insensitive(): + assert adk.normalize_connector(" Workday ") == "workday" + assert adk.normalize_connector("SERVICENOW") == "servicenow" + + +def test_normalize_connector_legacy_sentinel_preserved(): + # The "legacy" sentinel labels events that predate connector attribution + # (generic "connect" capability without a --connector arg). It must + # round-trip verbatim so the pre-attribution corpus stays queryable as + # its own bucket instead of collapsing into "unknown". + assert adk.normalize_connector("legacy") == adk.CONNECTOR_LEGACY + assert adk.normalize_connector(" Legacy ") == adk.CONNECTOR_LEGACY + + +def test_normalize_connector_unknown_bucketed(): + # Out-of-taxonomy values still emit but land in the controlled bucket so + # the "connector" dimension never mints stray slices. + assert adk.normalize_connector("adp") == adk.CONNECTOR_UNKNOWN + assert adk.normalize_connector("workday-soap") == adk.CONNECTOR_UNKNOWN + assert adk.normalize_connector("wd") == adk.CONNECTOR_UNKNOWN + + +def test_emit_capability_use_stamps_connector(captured_post, monkeypatch): + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + adk.emit_capability_use("connect", connector="workday", block=True) + data = captured_post[0][1][0]["data"] + assert data["adk_capability"] == "connect" + assert data["connector"] == "workday" + + +def test_emit_capability_use_omitted_connector_is_empty(captured_post, monkeypatch): + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + adk.emit_capability_use("topic_create", block=True) + data = captured_post[0][1][0]["data"] + # Topic authoring is not connector-scoped; the field is always present + # (Kusto column shape stays stable) but empty. + assert data["connector"] == "" + + +def test_emit_capability_use_unknown_connector_bucketed(captured_post, monkeypatch): + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + adk.emit_capability_use("connect", connector="Sap", block=True) + assert captured_post[0][1][0]["data"]["connector"] == adk.CONNECTOR_UNKNOWN + + +def test_emit_flightcheck_run_carries_connector(captured_post, monkeypatch): + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + adk.emit_flightcheck_run(agent_id="a1", connector="servicenow", block=True) + data = captured_post[0][1][0]["data"] + assert data["connector"] == "servicenow" + + +def test_emit_flightcheck_result_carries_connector(captured_post, monkeypatch): + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + adk.emit_flightcheck_result(agent_id="a1", connector="workday", result="pass", block=True) + assert captured_post[0][1][0]["data"]["connector"] == "workday" + + +def test_emit_flightcheck_error_carries_connector(captured_post, monkeypatch): + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + adk.emit_flightcheck_error(agent_id="a1", connector="workday", error_code="X", block=True) + assert captured_post[0][1][0]["data"]["connector"] == "workday" + + +def test_schema_version_bump_records_connector_dim(): + # The connector dimension was added in 1.5.0. Older cubes / dashboards + # can version-gate on this to know whether "connector" will be present. + assert adk.SCHEMA_VERSION == "1.5.0" + + +# --- emit_capability.py shim --connector plumbing ------------------------- +def test_shim_parses_connector_flag_before_capability(captured_post, monkeypatch): + import emit_capability + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + rc = emit_capability.main([ + "emit_capability.py", "--connector", "servicenow", "connect", + ]) + assert rc == 0 + data = captured_post[0][1][0]["data"] + assert data["adk_capability"] == "connect" + assert data["connector"] == "servicenow" + + +def test_shim_parses_connector_flag_after_capability(captured_post, monkeypatch): + import emit_capability + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + rc = emit_capability.main([ + "emit_capability.py", "connect", "--connector", "workday", + ]) + assert rc == 0 + assert captured_post[0][1][0]["data"]["connector"] == "workday" + + +def test_shim_parses_connector_equals_form(captured_post, monkeypatch): + import emit_capability + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + rc = emit_capability.main([ + "emit_capability.py", "connect", "--connector=workday", + ]) + assert rc == 0 + assert captured_post[0][1][0]["data"]["connector"] == "workday" + + +def test_shim_omitted_connector_yields_empty(captured_post, monkeypatch): + import emit_capability + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + rc = emit_capability.main(["emit_capability.py", "topic_create"]) + assert rc == 0 + assert captured_post[0][1][0]["data"]["connector"] == "" + + +def test_shim_dangling_connector_flag_still_emits(captured_post, monkeypatch): + # Malformed CLI (`--connector` with no value at the end) must not fail + # the skill step; the emit still fires with an empty connector so the + # capability signal is not lost. + import emit_capability + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + rc = emit_capability.main(["emit_capability.py", "connect", "--connector"]) + assert rc == 0 + data = captured_post[0][1][0]["data"] + assert data["adk_capability"] == "connect" + assert data["connector"] == "" + + +def test_shim_worker_mode_preserves_connector(captured_post, monkeypatch): + # Async mode re-execs the shim with `--worker ` (+ optional + # `--connector `). Exercise the worker branch directly to + # prove the connector survives the subprocess round-trip. + import emit_capability + monkeypatch.setenv("ESS_ADK_ARIA_ENV", "dev") + rc = emit_capability.main([ + "emit_capability.py", "--worker", "connect", "--connector", "workday", + ]) + assert rc == 0 + assert captured_post[0][1][0]["data"]["connector"] == "workday"