Skip to content

Commit

Permalink
Improve performance observability tooling in the graph
Browse files Browse the repository at this point in the history
  • Loading branch information
TimDiekmann committed Dec 13, 2024
1 parent 8087382 commit 15dc251
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 35 deletions.
2 changes: 1 addition & 1 deletion libs/@local/graph/api/src/rest/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::CheckAccountGroupPermission {
query_logger.capture(actor_id, OpenApiQuery::CheckAccountGroupPermission {
account_group_id,
permission,
});
Expand Down
15 changes: 9 additions & 6 deletions libs/@local/graph/api/src/rest/data_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetDataTypes(&request));
query_logger.capture(actor_id, OpenApiQuery::GetDataTypes(&request));
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -487,7 +487,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetDataTypeSubgraph(&request));
query_logger.capture(actor_id, OpenApiQuery::GetDataTypeSubgraph(&request));
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -890,9 +890,12 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetDataTypeAuthorizationRelationships {
data_type_id: &data_type_id,
});
query_logger.capture(
actor_id,
OpenApiQuery::GetDataTypeAuthorizationRelationships {
data_type_id: &data_type_id,
},
);
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -939,7 +942,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::CheckDataTypePermission {
query_logger.capture(actor_id, OpenApiQuery::CheckDataTypePermission {
data_type_id: &data_type_id,
permission,
});
Expand Down
39 changes: 30 additions & 9 deletions libs/@local/graph/api/src/rest/entity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::ValidateEntity(&body));
query_logger.capture(actor_id, OpenApiQuery::ValidateEntity(&body));
}

let params = ValidateEntityParams::deserialize(&body)
Expand Down Expand Up @@ -461,7 +461,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::CheckEntityPermission {
query_logger.capture(actor_id, OpenApiQuery::CheckEntityPermission {
entity_id,
permission,
});
Expand Down Expand Up @@ -633,7 +633,8 @@ struct GetEntitiesRequest<'q, 's, 'p> {
)]
#[tracing::instrument(
level = "info",
skip(store_pool, authorization_api_pool, temporal_client, request)
skip_all,
fields(actor=%actor_id, %request)
)]
async fn get_entities<S, A>(
AuthenticatedUserHeader(actor_id): AuthenticatedUserHeader,
Expand All @@ -648,7 +649,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetEntities(&request));
query_logger.capture(actor_id, OpenApiQuery::GetEntities(&request));
}

let authorization_api = authorization_api_pool
Expand All @@ -664,6 +665,14 @@ where
let request = GetEntitiesRequest::deserialize(&request)
.map_err(Report::from)
.map_err(report_to_response)?;

if request.limit == Some(0) {
tracing::warn!(
%actor_id,
"The limit is set to zero, so no entities will be returned."
);
}

let response = store
.get_entities(actor_id, GetEntitiesParams {
filter: request.filter,
Expand Down Expand Up @@ -788,7 +797,8 @@ struct GetEntitySubgraphResponse<'r> {
)]
#[tracing::instrument(
level = "info",
skip(store_pool, authorization_api_pool, temporal_client, request)
skip_all,
fields(actor=%actor_id, %request)
)]
async fn get_entity_subgraph<S, A>(
AuthenticatedUserHeader(actor_id): AuthenticatedUserHeader,
Expand All @@ -803,7 +813,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetEntitySubgraph(&request));
query_logger.capture(actor_id, OpenApiQuery::GetEntitySubgraph(&request));
}

let authorization_api = authorization_api_pool
Expand All @@ -819,6 +829,14 @@ where
let request = GetEntitySubgraphRequest::deserialize(&request)
.map_err(Report::from)
.map_err(report_to_response)?;

if request.limit == Some(0) {
tracing::warn!(
%actor_id,
"The limit is set to zero, so no entities will be returned"
);
}

let response = store
.get_entity_subgraph(actor_id, GetEntitySubgraphParams {
filter: request.filter,
Expand Down Expand Up @@ -897,7 +915,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::CountEntities(&request));
query_logger.capture(actor_id, OpenApiQuery::CountEntities(&request));
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -1069,7 +1087,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::DiffEntity(&params));
query_logger.capture(actor_id, OpenApiQuery::DiffEntity(&params));
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -1125,7 +1143,10 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetEntityAuthorizationRelationships { entity_id });
query_logger.capture(
actor_id,
OpenApiQuery::GetEntityAuthorizationRelationships { entity_id },
);
}

let authorization_api = authorization_api_pool
Expand Down
17 changes: 10 additions & 7 deletions libs/@local/graph/api/src/rest/entity_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,12 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetEntityTypeAuthorizationRelationships {
entity_type_id: &entity_type_id,
});
query_logger.capture(
actor_id,
OpenApiQuery::GetEntityTypeAuthorizationRelationships {
entity_type_id: &entity_type_id,
},
);
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -336,7 +339,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::CheckEntityTypePermission {
query_logger.capture(actor_id, OpenApiQuery::CheckEntityTypePermission {
entity_type_id: &entity_type_id,
permission,
});
Expand Down Expand Up @@ -725,7 +728,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetEntityTypes(&request));
query_logger.capture(actor_id, OpenApiQuery::GetEntityTypes(&request));
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -793,7 +796,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetClosedMultiEntityTypes(&request));
query_logger.capture(actor_id, OpenApiQuery::GetClosedMultiEntityTypes(&request));
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -874,7 +877,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetEntityTypeSubgraph(&request));
query_logger.capture(actor_id, OpenApiQuery::GetEntityTypeSubgraph(&request));
}

let authorization_api = authorization_api_pool
Expand Down
14 changes: 8 additions & 6 deletions libs/@local/graph/api/src/rest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,15 @@ impl QueryLogger {
}

#[expect(clippy::missing_panics_doc)]
pub fn capture(&mut self, query: OpenApiQuery<'_>) {
pub fn capture(&mut self, actor: AccountId, query: OpenApiQuery<'_>) {
let mut record = serde_json::to_value(query)
.change_context(QueryLoggingError)
.expect("query should be serializable");
record
.as_object_mut()
.map(|object| object.insert("actor".to_owned(), JsonValue::String(actor.to_string())));
self.value = Some(record);
self.created_at = Instant::now();
self.value = Some(
serde_json::to_value(query)
.change_context(QueryLoggingError)
.expect("query should be serializable"),
);
}

/// Sends the captured query to the query logger.
Expand Down
15 changes: 9 additions & 6 deletions libs/@local/graph/api/src/rest/property_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetPropertyTypes(&request));
query_logger.capture(actor_id, OpenApiQuery::GetPropertyTypes(&request));
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -480,7 +480,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetPropertyTypeSubgraph(&request));
query_logger.capture(actor_id, OpenApiQuery::GetPropertyTypeSubgraph(&request));
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -878,9 +878,12 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::GetPropertyTypeAuthorizationRelationships {
property_type_id: &property_type_id,
});
query_logger.capture(
actor_id,
OpenApiQuery::GetPropertyTypeAuthorizationRelationships {
property_type_id: &property_type_id,
},
);
}

let authorization_api = authorization_api_pool
Expand Down Expand Up @@ -929,7 +932,7 @@ where
A: AuthorizationApiPool + Send + Sync,
{
if let Some(query_logger) = &mut query_logger {
query_logger.capture(OpenApiQuery::CheckPropertyTypePermission {
query_logger.capture(actor_id, OpenApiQuery::CheckPropertyTypePermission {
property_type_id: &property_type_id,
permission,
});
Expand Down

0 comments on commit 15dc251

Please sign in to comment.