From 81880b4d88b08310d642d238d34b68ae8345fdd2 Mon Sep 17 00:00:00 2001 From: "hasan.toor" Date: Tue, 15 Sep 2026 17:49:48 +0000 Subject: [PATCH] refactor(idp): use UEG for service dependency helpers Replace the legacy production dependency snapshot used by idp deps and assist with exact-ref, one-hour UEG runtime relationships. Preserve the helper response shape, update agent guidance, and cover positive and negative paths. --- agents/service-catalog.md | 4 +- skills/dd-idp/SKILL.md | 2 +- src/commands/idp/entity_query.rs | 2 +- src/commands/idp/mod.rs | 306 ++++++++++++++++++++++--------- src/main.rs | 21 ++- src/test_commands.rs | 9 +- 6 files changed, 238 insertions(+), 106 deletions(-) diff --git a/agents/service-catalog.md b/agents/service-catalog.md index e84720ee..bf0183a6 100644 --- a/agents/service-catalog.md +++ b/agents/service-catalog.md @@ -11,7 +11,7 @@ Help users understand and maintain Datadog's service and software catalog. Use t - Default to `pup idp kinds` and `pup idp entities query` when a read needs connected service context: ownership, on-call, systems, code, dependencies, health, work, operations, or security. UEG can return selected service and dependency context in one bounded request. Load the `dd-idp` skill for its schema-first workflow and DSL guidance when available. - Use `pup service-catalog list|get` for the legacy typed service registry. - Use `pup idp assist` for a fast curated single-service summary, metadata gaps, and suggested next actions; use `owner` for convenient owner/on-call resolution. These trade graph fidelity for a narrower opinionated result. -- Treat `find` as a simple legacy service-name lookup and `deps` as a production-only service-to-service snapshot. Use UEG for explicit schema, relation families, counts, pagination, and traversal. +- Treat `find` as a simple legacy service-name lookup. Use `deps` as a convenient one-hour UEG runtime service-to-service summary, and use `entities query` for another lookback or broader relation families, counts, pagination, and traversal. - Use `pup software-catalog entities|kinds|relations` for Catalog inventory and explicit Catalog mutations. - Use `pup idp register` to preserve the familiar file-oriented workflow across v1, v2, v2.1, v2.2, and v3 Catalog definitions. It sends raw YAML or JSON to the Catalog entity API and accepts multi-document YAML. @@ -40,7 +40,7 @@ pup --read-only idp entities query 'kind:service AND name:""' \ This is the graph's central value: service identity, ownership, health, and declared dependencies in one call. Choose a small relation family and report relation counts/truncation. Add runtime dependencies, datastores, queues, deployments, incidents, monitors, SLOs, or security relations only when the request needs them. -### Legacy service helpers +### Convenience service helpers ```bash pup --read-only idp assist diff --git a/skills/dd-idp/SKILL.md b/skills/dd-idp/SKILL.md index fe9b5bc3..18056485 100644 --- a/skills/dd-idp/SKILL.md +++ b/skills/dd-idp/SKILL.md @@ -16,7 +16,7 @@ Use Pup's read-only Unified Entity Graph (UEG) to discover software, ownership, - Default to `pup idp kinds` and `pup idp entities query` when the answer needs connected context across services, teams, systems, repositories, dependencies, work, operations, or security kinds. - Use `pup idp assist` when the user values a fast, curated single-service summary, metadata gaps, and suggested next actions over graph fidelity; use `owner` for convenient owner/on-call resolution. -- Treat `find` as a simple legacy service-name lookup and `deps` as a production-only service-to-service dependency snapshot. Use UEG for explicit schema, relation families, counts, pagination, and traversal. +- Treat `find` as a simple legacy service-name lookup. Use `deps` as a convenient one-hour UEG runtime service-to-service dependency summary, and `entities query` for another lookback or broader relation families, counts, pagination, and traversal. - Use product commands such as `pup incidents`, `pup slos`, `pup monitors`, `pup logs`, `pup traces`, or `pup security` when the user needs deeper or current telemetry. - Use `pup service-catalog` for the legacy typed service registry and `pup software-catalog` for Catalog entity/kind reads and writes. Do not use graph queries for mutations. diff --git a/src/commands/idp/entity_query.rs b/src/commands/idp/entity_query.rs index 71b97762..b8b0826b 100644 --- a/src/commands/idp/entity_query.rs +++ b/src/commands/idp/entity_query.rs @@ -477,7 +477,7 @@ fn string_attribute<'a>(attributes: &'a BTreeMap, key: &str) -> O attributes.get(key).and_then(Value::as_str) } -fn parse_relationship_data(value: &Value) -> Vec { +pub(super) fn parse_relationship_data(value: &Value) -> Vec { if value.is_null() { return Vec::new(); } diff --git a/src/commands/idp/mod.rs b/src/commands/idp/mod.rs index 682ff0b9..4eaa48bd 100644 --- a/src/commands/idp/mod.rs +++ b/src/commands/idp/mod.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use serde::Serialize; use crate::config::Config; @@ -11,6 +11,10 @@ mod entity_query; mod entity_types; mod migrate; +const RUNTIME_DEPENDENCY_RELATIONS: &str = "runtime_upstream_services,runtime_downstream_services"; +const ASSIST_RELATIONS: &str = "owner_teams,runtime_upstream_services,runtime_downstream_services"; +const RUNTIME_DEPENDENCY_LOOKBACK: &str = "1h"; + pub use entity_kinds::{describe_kind, list_kinds}; pub use entity_query::{query_entities, EntityQueryOptions}; pub use migrate::migrate_schema; @@ -122,7 +126,7 @@ struct SloCounts { no_data: i64, } -#[derive(Serialize)] +#[derive(Debug, PartialEq, Eq, Serialize)] struct DependencySummary { upstream: Vec, downstream: Vec, @@ -396,50 +400,54 @@ fn compute_next_actions(entity_name: &str, health: &HealthSummary, gaps: &[Strin actions } -// --------------------------------------------------------------------------- -// Parse dependencies from /api/v1/service_dependencies response -// Format: { "service_name": { "calls": ["dep1", "dep2"] }, ... } -// --------------------------------------------------------------------------- - -fn parse_dependencies(deps_data: &serde_json::Value, entity: &str) -> (Vec, Vec) { - let mut upstream = Vec::new(); - let mut downstream = Vec::new(); +fn relationship_service_names(entity: &serde_json::Value, relation: &str) -> Vec { + let mut names = entity + .get("relationships") + .and_then(|relationships| relationships.get(relation)) + .and_then(|relationship| relationship.get("data")) + .map(entity_query::parse_relationship_data) + .unwrap_or_default() + .into_iter() + .filter(|identifier| identifier.kind == "service") + .map(|identifier| { + identifier + .id + .strip_prefix("ref:service:") + .unwrap_or(&identifier.id) + .to_string() + }) + .collect::>(); + names.sort(); + names.dedup(); + names +} - if let Some(deps_map) = deps_data.as_object() { - // Downstream: services this entity calls - if let Some(calls) = deps_map - .get(entity) - .and_then(|v| v.get("calls")) - .and_then(|v| v.as_array()) - { - downstream = calls - .iter() - .filter_map(|v| v.as_str().map(String::from)) - .collect(); - } - // Upstream: services that call this entity - for (svc, entry) in deps_map { - if svc == entity { - continue; - } - if let Some(calls) = entry.get("calls").and_then(|v| v.as_array()) { - if calls.iter().any(|d| d.as_str() == Some(entity)) { - upstream.push(svc.clone()); - } - } - } +fn extract_runtime_dependencies(entity: &serde_json::Value) -> DependencySummary { + DependencySummary { + upstream: relationship_service_names(entity, "runtime_upstream_services"), + downstream: relationship_service_names(entity, "runtime_downstream_services"), } +} - (upstream, downstream) +fn first_entity<'a>(data: &'a serde_json::Value, entity: &str) -> Result<&'a serde_json::Value> { + data.get("data") + .and_then(serde_json::Value::as_array) + .context("entity graph response did not contain an entity list")? + .first() + .ok_or_else(|| anyhow::anyhow!("no entity found matching '{entity}'")) } // --------------------------------------------------------------------------- -// Build the UEG query URL for a service entity by name +// Build the UEG query URL for a service entity by concrete ref // --------------------------------------------------------------------------- fn entity_query_url(entity: &str, include: &str) -> String { - let query = util_ext::percent_encode(&format!("kind:service AND name:{entity}")); - let mut url = format!("/api/v2/idp/entity_graph/entities?query={query}&page%5Blimit%5D=1"); + let entity_ref = serde_json::to_string(&format!("ref:service:{entity}")) + .expect("serializing a string cannot fail"); + let query = util_ext::percent_encode(&format!("ref:{entity_ref}")); + let mut url = format!( + "/api/v2/idp/entity_graph/entities?query={query}&page%5Blimit%5D=1&time%5Bpast%5D={RUNTIME_DEPENDENCY_LOOKBACK}" + ); if !include.is_empty() { url.push_str(&format!("&include={include}")); } @@ -452,28 +460,11 @@ fn entity_query_url(entity: &str, include: &str) -> String { /// Flagship command: returns concise entity context + suggested next actions. pub async fn assist(cfg: &Config, entity: &str) -> Result<()> { - // Fan out: entity graph + dependencies in parallel - let entity_path = entity_query_url(entity, "owner_teams"); - let deps_path = "/api/v1/service_dependencies?env=prod"; - - let (entity_res, deps_res) = tokio::join!( - raw_client::raw_get(cfg, &entity_path, &[]), - raw_client::raw_get(cfg, deps_path, &[]), - ); - - let entity_data = entity_res?; + let entity_path = entity_query_url(entity, ASSIST_RELATIONS); + let entity_data = raw_client::raw_get(cfg, &entity_path, &[]).await?; // Parse entity from UEG response (JSON:API format: { data: [...], included: [...] }) - let entities = entity_data - .get("data") - .and_then(|d| d.as_array()) - .ok_or_else(|| anyhow::anyhow!("no entities found matching '{entity}'"))?; - - if entities.is_empty() { - anyhow::bail!("no entity found matching '{entity}'"); - } - - let primary = &entities[0]; + let primary = first_entity(&entity_data, entity)?; let attrs = &primary["attributes"]; let included = entity_data .get("included") @@ -494,21 +485,14 @@ pub async fn assist(cfg: &Config, entity: &str) -> Result<()> { let gaps = compute_metadata_gaps(&summary, attrs, &links); let next_actions = compute_next_actions(&summary.name, &health, &gaps); - // Parse dependencies - let (upstream, downstream) = match deps_res { - Ok(ref deps_data) => parse_dependencies(deps_data, entity), - Err(_) => (vec![], vec![]), - }; + let dependencies = extract_runtime_dependencies(primary); let response = AssistResponse { entity: summary, owner, on_call, health, - dependencies: DependencySummary { - upstream, - downstream, - }, + dependencies, metadata_gaps: gaps, links, suggested_next_actions: next_actions, @@ -567,16 +551,7 @@ pub async fn owner(cfg: &Config, entity: &str) -> Result<()> { let path = entity_query_url(entity, "owner_teams"); let data = raw_client::raw_get(cfg, &path, &[]).await?; - let entities = data - .get("data") - .and_then(|d| d.as_array()) - .ok_or_else(|| anyhow::anyhow!("no entities found matching '{entity}'"))?; - - if entities.is_empty() { - anyhow::bail!("no entity found matching '{entity}'"); - } - - let primary = &entities[0]; + let primary = first_entity(&data, entity)?; let included = data .get("included") .cloned() @@ -616,23 +591,25 @@ pub async fn owner(cfg: &Config, entity: &str) -> Result<()> { /// Show dependency and relationship context for an entity. pub async fn deps(cfg: &Config, entity: &str) -> Result<()> { - let deps_path = "/api/v1/service_dependencies?env=prod"; - let deps_data = raw_client::raw_get(cfg, deps_path, &[]).await?; - let (upstream, downstream) = parse_dependencies(&deps_data, entity); + let entity_path = entity_query_url(entity, RUNTIME_DEPENDENCY_RELATIONS); + let entity_data = raw_client::raw_get(cfg, &entity_path, &[]).await?; + let primary = first_entity(&entity_data, entity)?; + let dependencies = extract_runtime_dependencies(primary); + let dependency_count = dependencies.upstream.len() + dependencies.downstream.len(); let response = serde_json::json!({ "entity": entity, - "dependencies": { - "upstream": upstream, - "downstream": downstream, - } + "dependencies": dependencies, }); let meta = formatter::Metadata { - count: Some(upstream.len() + downstream.len()), + count: Some(dependency_count), truncated: false, command: Some(format!("idp deps {entity}")), - next_action: Some("Use `pup idp assist ` to inspect any dependency".to_string()), + next_action: Some( + "Use `pup idp entities query` for a different lookback or broader dependency relations" + .to_string(), + ), }; formatter::format_and_print( @@ -752,22 +729,66 @@ mod tests { use crate::test_support::*; use mockito::Matcher; + fn service_with_runtime_dependencies() -> serde_json::Value { + serde_json::json!({ + "type": "service", + "id": "ref:service:catalog-http", + "attributes": { + "name": "catalog-http", + "owner": "idp" + }, + "relationships": { + "runtime_upstream_services": { + "data": [ + {"type": "service", "id": "ref:service:web"}, + {"type": "service", "id": "api"}, + {"type": "team", "id": "ref:team:idp"}, + {"type": "service", "id": "ref:service:web"} + ] + }, + "runtime_downstream_services": { + "data": [ + {"type": "service", "id": "ref:service:database"}, + {"type": "service", "id": "ref:service:cache"} + ] + } + } + }) + } + + fn entity_graph_response() -> serde_json::Value { + serde_json::json!({ + "data": [service_with_runtime_dependencies()], + "included": [] + }) + } + #[test] fn test_entity_query_url_encodes_special_chars() { // Colons, spaces, and other characters in entity names and the query // syntax must be percent-encoded so the URL is well-formed. let url = entity_query_url("my service", ""); assert!( - url.contains("kind%3Aservice"), - "colon should be encoded: {url}" + url.contains("ref%3A%22ref%3Aservice%3Amy%20service%22"), + "concrete entity ref should be encoded: {url}" ); assert!( - url.contains("my%20service"), - "space should be encoded: {url}" + url.contains("time%5Bpast%5D=1h"), + "runtime lookback should be explicit: {url}" ); assert!(!url.contains("include="), "empty include should be omitted"); } + #[test] + fn test_entity_query_url_escapes_quoted_ref_values() { + let url = entity_query_url("quoted\"service\\name", ""); + + assert!( + url.contains("quoted%5C%22service%5C%5Cname"), + "quote and backslash should be escaped before encoding: {url}" + ); + } + #[test] fn test_entity_query_url_appends_include() { let url = entity_query_url("svc", "owner_teams"); @@ -801,6 +822,19 @@ mod tests { ); } + #[test] + fn test_extract_runtime_dependencies_normalizes_service_refs() { + let dependencies = extract_runtime_dependencies(&service_with_runtime_dependencies()); + + assert_eq!( + dependencies, + DependencySummary { + upstream: vec!["api".into(), "web".into()], + downstream: vec!["cache".into(), "database".into()], + } + ); + } + #[test] fn test_registration_metadata_surfaces_schema_warnings() { let response = serde_json::json!({ @@ -931,4 +965,98 @@ mod tests { assert!(error.to_string().contains("empty Catalog entity file")); cleanup_env(); } + + #[test] + fn test_extract_runtime_dependencies_handles_missing_relationships() { + let dependencies = extract_runtime_dependencies(&serde_json::json!({})); + + assert_eq!( + dependencies, + DependencySummary { + upstream: Vec::new(), + downstream: Vec::new(), + } + ); + } + + #[test] + fn test_first_entity_rejects_malformed_response() { + let error = first_entity(&serde_json::json!({"data": {}}), "catalog-http").unwrap_err(); + + assert!(error + .to_string() + .contains("entity graph response did not contain an entity list")); + } + + #[tokio::test] + async fn test_deps_uses_ueg_runtime_relationships() { + let _guard = crate::test_support::lock_env().await; + let mut server = mockito::Server::new_async().await; + let mock = server + .mock("GET", "/api/v2/idp/entity_graph/entities") + .match_query(Matcher::AllOf(vec![ + Matcher::UrlEncoded("query".into(), "ref:\"ref:service:catalog-http\"".into()), + Matcher::UrlEncoded("page[limit]".into(), "1".into()), + Matcher::UrlEncoded("time[past]".into(), RUNTIME_DEPENDENCY_LOOKBACK.into()), + Matcher::UrlEncoded("include".into(), RUNTIME_DEPENDENCY_RELATIONS.into()), + ])) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(entity_graph_response().to_string()) + .create_async() + .await; + let cfg = crate::test_support::test_config(&server.url()); + + deps(&cfg, "catalog-http").await.unwrap(); + + mock.assert_async().await; + crate::test_support::cleanup_env(); + } + + #[tokio::test] + async fn test_deps_errors_when_service_is_missing() { + let _guard = crate::test_support::lock_env().await; + let mut server = mockito::Server::new_async().await; + let mock = server + .mock("GET", "/api/v2/idp/entity_graph/entities") + .match_query(Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"data": []}"#) + .create_async() + .await; + let cfg = crate::test_support::test_config(&server.url()); + + let error = deps(&cfg, "missing").await.unwrap_err(); + + assert!(error + .to_string() + .contains("no entity found matching 'missing'")); + mock.assert_async().await; + crate::test_support::cleanup_env(); + } + + #[tokio::test] + async fn test_assist_fetches_runtime_dependencies_with_service_context() { + let _guard = crate::test_support::lock_env().await; + let mut server = mockito::Server::new_async().await; + let mock = server + .mock("GET", "/api/v2/idp/entity_graph/entities") + .match_query(Matcher::AllOf(vec![ + Matcher::UrlEncoded("include".into(), ASSIST_RELATIONS.into()), + Matcher::UrlEncoded("page[limit]".into(), "1".into()), + Matcher::UrlEncoded("time[past]".into(), RUNTIME_DEPENDENCY_LOOKBACK.into()), + ])) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(entity_graph_response().to_string()) + .create_async() + .await; + let cfg = crate::test_support::test_config(&server.url()); + + assist(&cfg, "catalog-http").await.unwrap(); + + mock.assert_async().await; + crate::test_support::cleanup_env(); + } } diff --git a/src/main.rs b/src/main.rs index 9cd0355c..d1b42e1c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1546,10 +1546,10 @@ enum Commands { /// CAPABILITIES: /// • Discover entity kinds and inspect their live query schemas /// • Query entities and traverse declared relationships - /// • Get an opinionated legacy service summary (assist) + /// • Get an opinionated service summary (assist) /// • Run a quick legacy service lookup (find) /// • Resolve service ownership and on-call (owner) - /// • Show legacy production service dependencies (deps) + /// • Show runtime service dependencies from UEG (deps) /// • Register Catalog entities from YAML or JSON (register) /// • Migrate service catalog YAML to v3 schema (migrate-schema) /// @@ -1564,7 +1564,7 @@ enum Commands { /// --include owner_teams,upstream_services,downstream_services \ /// --relation-limit 3 /// - /// # Get the opinionated legacy service summary + /// # Get the opinionated service summary /// pup idp assist checkout-api /// /// # Who owns this service? @@ -5321,10 +5321,10 @@ enum IdpActions { #[command(subcommand)] action: IdpEntitiesActions, }, - /// Get an opinionated legacy service summary with suggested next actions + /// Get an opinionated service summary with suggested next actions /// - /// Compatibility helper that combines UEG service data with legacy - /// production dependency and on-call lookups: + /// Compatibility helper that combines UEG service, runtime dependency, + /// and on-call context: /// /// RETURNS: /// • Entity info (name, kind, description, lifecycle, tier, owner) @@ -5379,11 +5379,12 @@ enum IdpActions { /// Entity name entity: String, }, - /// Show legacy production upstream and downstream service dependencies + /// Show runtime upstream and downstream service dependencies from UEG /// - /// Returns the legacy `env=prod` service-to-service dependency snapshot. - /// Use `idp entities query` for declared/runtime graph relations to - /// services, datastores, queues, external providers, or inferred services. + /// Returns UEG's runtime-observed service-to-service relations over the + /// past hour. Use `idp entities query` for another lookback, the broader + /// unified graph, or relations to datastores, queues, external providers, + /// or inferred services. /// /// EXAMPLES: /// pup idp deps catalog-http diff --git a/src/test_commands.rs b/src/test_commands.rs index f8692d4d..71414a28 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -871,7 +871,7 @@ fn test_idp_entity_graph_commands_parse() { } #[test] -fn test_idp_legacy_help_routes_connected_context_to_entity_graph() { +fn test_idp_convenience_help_routes_connected_context_to_entity_graph() { let idp = crate::Cli::command() .find_subcommand("idp") .expect("idp command should exist") @@ -888,11 +888,14 @@ fn test_idp_legacy_help_routes_connected_context_to_entity_graph() { .clone() .render_long_help() .to_string(); + let deps_help = deps_help.split_whitespace().collect::>().join(" "); assert!(assist_help.contains("idp entities query")); assert!(!assist_help.contains("flagship")); - assert!(deps_help.contains("env=prod")); - assert!(deps_help.contains("declared/runtime graph relations")); + assert!(deps_help.contains("runtime-observed")); + assert!(deps_help.contains("past hour")); + assert!(deps_help.contains("broader unified graph")); + assert!(!deps_help.contains("env=prod")); } #[test]