Skip to content

Commit 2649c43

Browse files
ruromeroclaude
authored andcommitted
refactor: fix instrumentation patterns per review feedback
- Remove redundant .instrument() wrappers on self-calls that already have #[instrument] attributes - Use skip_all and err(level=INFO) per log_tracing.md conventions - Accept impl IntoIterator instead of &[Uuid] to avoid intermediate allocations - Add instrumentation spans to SeaORM load_one/load_many_to_many calls - Use HashMap<_, _> type inference for base_purl_map Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> (cherry picked from commit 8533cde)
1 parent 87ac5cb commit 2649c43

File tree

1 file changed

+28
-22
lines changed
  • modules/fundamental/src/purl/service

1 file changed

+28
-22
lines changed

modules/fundamental/src/purl/service/mod.rs

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -524,21 +524,18 @@ impl PurlService {
524524
return Ok(recommendations);
525525
}
526526

527-
let base_purls = Self::fetch_base_purls(&input_purls, connection)
528-
.instrument(info_span!("loading base purls"))
529-
.await?;
527+
let base_purls = Self::fetch_base_purls(&input_purls, connection).await?;
530528
if base_purls.is_empty() {
531529
for ip in &input_purls {
532530
recommendations.insert(ip.purl.to_string(), Vec::new());
533531
}
534532
return Ok(recommendations);
535533
}
536534

537-
let versioned_by_base = Self::fetch_versioned_purls_by_base(&base_purls, connection)
538-
.instrument(info_span!("loading versioned purls"))
539-
.await?;
535+
let versioned_by_base =
536+
Self::fetch_versioned_purls_by_base(&base_purls, connection).await?;
540537

541-
let base_purl_map: HashMap<PurlKey<'_>, &base_purl::Model> = base_purls
538+
let base_purl_map: HashMap<_, _> = base_purls
542539
.iter()
543540
.map(|bp| (PurlKey::from_base_purl(bp), bp))
544541
.collect();
@@ -577,13 +574,12 @@ impl PurlService {
577574
}
578575

579576
// Batch fetch vulnerability statuses and qualified PURLs for all winners
580-
let winner_base_ids: Vec<_> = winners.iter().map(|w| w.base.id).unique().collect();
581-
let winner_vp_ids: Vec<_> = winners.iter().map(|w| w.versioned_purl.id).collect();
582-
583-
let statuses_by_base =
584-
Self::fetch_vulnerability_statuses(&winner_base_ids, &winner_vp_ids, connection)
585-
.instrument(info_span!("loading vulnerability statuses"))
586-
.await?;
577+
let statuses_by_base = Self::fetch_vulnerability_statuses(
578+
winners.iter().map(|w| w.base.id).unique(),
579+
winners.iter().map(|w| w.versioned_purl.id),
580+
connection,
581+
)
582+
.await?;
587583

588584
// Assemble recommendations from batched data
589585
for winner in winners {
@@ -596,15 +592,16 @@ impl PurlService {
596592

597593
/// Batch-loads vulnerability statuses for the winning versioned PURLs, grouped by base PURL ID.
598594
/// Chunks by base PURL IDs to stay within Postgres bind parameter limits.
599-
#[instrument(skip(winner_base_ids, winner_vp_ids, connection), err)]
595+
#[instrument(skip_all, err(level = tracing::Level::INFO))]
600596
async fn fetch_vulnerability_statuses<C: ConnectionTrait>(
601-
winner_base_ids: &[Uuid],
602-
winner_vp_ids: &[Uuid],
597+
winner_base_ids: impl IntoIterator<Item = Uuid>,
598+
winner_vp_ids: impl IntoIterator<Item = Uuid>,
603599
connection: &C,
604600
) -> Result<HashMap<Uuid, Vec<StatusInfo>>, Error> {
605601
let mut statuses_by_base: HashMap<_, Vec<StatusInfo>> = HashMap::new();
602+
let winner_vp_ids: Vec<_> = winner_vp_ids.into_iter().collect();
606603

607-
let base_chunks = chunked_with(1, winner_base_ids.iter().copied());
604+
let base_chunks = chunked_with(1, winner_base_ids.into_iter());
608605
for base_chunk in &base_chunks {
609606
let base_chunk: Vec<_> = base_chunk.collect();
610607
let all_statuses = purl_status::Entity::find()
@@ -629,13 +626,21 @@ impl PurlService {
629626
.arg(Expr::col((version_range::Entity, Asterisk))),
630627
))
631628
.all(connection)
629+
.instrument(info_span!("querying purl statuses"))
632630
.await?;
633631

634632
let vulns = all_statuses
635633
.load_one(vulnerability::Entity, connection)
634+
.instrument(info_span!("loading vulnerabilities"))
635+
.await?;
636+
let advisories_loaded = all_statuses
637+
.load_one(advisory::Entity, connection)
638+
.instrument(info_span!("loading advisories"))
639+
.await?;
640+
let status_models = all_statuses
641+
.load_one(status::Entity, connection)
642+
.instrument(info_span!("loading statuses"))
636643
.await?;
637-
let advisories_loaded = all_statuses.load_one(advisory::Entity, connection).await?;
638-
let status_models = all_statuses.load_one(status::Entity, connection).await?;
639644
let status_slug_map: HashMap<_, _> = status_models
640645
.into_iter()
641646
.flatten()
@@ -647,6 +652,7 @@ impl PurlService {
647652
remediation_purl_status::Entity,
648653
connection,
649654
)
655+
.instrument(info_span!("loading remediations"))
650656
.await?;
651657

652658
for (((vuln, advisory), ps), rems) in vulns
@@ -735,7 +741,7 @@ impl PurlService {
735741

736742
/// Batch-fetches base PURL entities for the deduplicated set of input PURLs.
737743
/// Chunks the OR conditions to stay within Postgres bind parameter limits.
738-
#[instrument(skip(input_purls, connection), err)]
744+
#[instrument(skip_all, err(level = tracing::Level::INFO))]
739745
async fn fetch_base_purls<C: ConnectionTrait>(
740746
input_purls: &[InputPurl],
741747
connection: &C,
@@ -767,7 +773,7 @@ impl PurlService {
767773

768774
/// Loads all versioned PURLs for the given base PURLs, grouped by base PURL ID.
769775
/// Chunks the IN clause to stay within Postgres bind parameter limits.
770-
#[instrument(skip(base_purls, connection), err)]
776+
#[instrument(skip_all, err(level = tracing::Level::INFO))]
771777
async fn fetch_versioned_purls_by_base<C: ConnectionTrait>(
772778
base_purls: &[base_purl::Model],
773779
connection: &C,

0 commit comments

Comments
 (0)