Skip to content

Commit d94f5a0

Browse files
committed
chore: simplify the internal API, add instrumentation
This drops a bunch of "async" functions and Result returns, which aren't being used anyway. It also adds a bit more instrumentation, and some test tooling, allowing one to enable flamechart traces for tests.
1 parent 1997349 commit d94f5a0

File tree

14 files changed

+76
-60
lines changed

14 files changed

+76
-60
lines changed

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77
.envrc
88

99
*.cdx.json
10+
*.folded
1011

11-
/flamegraph*.svg
12+
/flame*.svg
1213
/perf.data*
1314
/dump.sql
1415
.ai_history.txt

Cargo.lock

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

modules/fundamental/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ test-log = { workspace = true, features = ["log", "trace"] }
8686
tokio-util = { workspace = true }
8787
tracing = { workspace = true, features = ["std"]}
8888
tracing-core = { workspace = true }
89-
tracing-flame = { workspace = true }
9089
tracing-opentelemetry = { workspace = true }
9190
tracing-subscriber = { workspace = true }
9291
trustify-cvss = { workspace = true }

modules/fundamental/src/advisory/model/mod.rs

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::{Error, organization::model::OrganizationSummary};
99
use sea_orm::{ConnectionTrait, LoaderTrait, ModelTrait, prelude::Uuid};
1010
use serde::{Deserialize, Serialize};
1111
use time::OffsetDateTime;
12+
use tracing::instrument;
1213
use trustify_common::memo::Memo;
1314
use trustify_entity::{advisory, labels::Labels, organization};
1415
use utoipa::ToSchema;
@@ -54,21 +55,20 @@ pub struct AdvisoryHead {
5455
}
5556

5657
impl AdvisoryHead {
58+
#[instrument(skip_all, fields(advisory.id = ?advisory.id), err(level=tracing::Level::INFO))]
5759
pub async fn from_advisory<C: ConnectionTrait>(
5860
advisory: &advisory::Model,
5961
issuer: Memo<organization::Model>,
6062
tx: &C,
6163
) -> Result<Self, Error> {
6264
let issuer = match &issuer {
63-
Memo::Provided(Some(issuer)) => Some(OrganizationSummary::from_entity(issuer).await?),
65+
Memo::Provided(Some(issuer)) => Some(OrganizationSummary::from_entity(issuer)),
6466
Memo::Provided(None) => None,
65-
Memo::NotProvided => {
66-
if let Some(issuer) = advisory.find_related(organization::Entity).one(tx).await? {
67-
Some(OrganizationSummary::from_entity(&issuer).await?)
68-
} else {
69-
None
70-
}
71-
}
67+
Memo::NotProvided => advisory
68+
.find_related(organization::Entity)
69+
.one(tx)
70+
.await?
71+
.map(|issuer| OrganizationSummary::from_entity(&issuer)),
7272
};
7373

7474
Ok(Self {
@@ -93,11 +93,7 @@ impl AdvisoryHead {
9393
let issuers = entities.load_one(organization::Entity, tx).await?;
9494

9595
for (advisory, issuer) in entities.iter().zip(issuers) {
96-
let issuer = if let Some(issuer) = issuer {
97-
Some(OrganizationSummary::from_entity(&issuer).await?)
98-
} else {
99-
None
100-
};
96+
let issuer = issuer.map(|issuer| OrganizationSummary::from_entity(&issuer));
10197

10298
heads.push(Self {
10399
uuid: advisory.id,

modules/fundamental/src/organization/model/details/mod.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
1+
use crate::{Error, advisory::model::AdvisoryHead, organization::model::OrganizationHead};
12
use sea_orm::{ConnectionTrait, ModelTrait};
23
use serde::{Deserialize, Serialize};
3-
use utoipa::ToSchema;
4-
5-
use crate::advisory::model::AdvisoryHead;
64
use trustify_entity::{advisory, organization};
7-
8-
use crate::Error;
9-
use crate::organization::model::OrganizationHead;
5+
use utoipa::ToSchema;
106

117
#[derive(Serialize, Deserialize, Clone, Debug, ToSchema)]
128
pub struct OrganizationDetails {
@@ -24,7 +20,7 @@ impl OrganizationDetails {
2420
) -> Result<Self, Error> {
2521
let advisories = org.find_related(advisory::Entity).all(tx).await?;
2622
Ok(OrganizationDetails {
27-
head: OrganizationHead::from_entity(org).await?,
23+
head: OrganizationHead::from_entity(org),
2824
advisories: AdvisoryHead::from_entities(&advisories, tx).await?,
2925
})
3026
}

modules/fundamental/src/organization/model/mod.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
use serde::{Deserialize, Serialize};
2+
use trustify_entity::organization;
23
use utoipa::ToSchema;
34
use uuid::Uuid;
45

56
mod details;
67
mod summary;
78

8-
use crate::Error;
99
pub use details::*;
1010
pub use summary::*;
11-
use trustify_entity::organization;
1211

1312
/// An organization who may issue advisories, product SBOMs, or
1413
/// otherwise be involved in supply-chain evidence.
@@ -30,12 +29,12 @@ pub struct OrganizationHead {
3029
}
3130

3231
impl OrganizationHead {
33-
pub async fn from_entity(organization: &organization::Model) -> Result<Self, Error> {
34-
Ok(OrganizationHead {
32+
pub fn from_entity(organization: &organization::Model) -> Self {
33+
OrganizationHead {
3534
id: organization.id,
3635
name: organization.name.clone(),
3736
cpe_key: organization.cpe_key.clone(),
3837
website: organization.website.clone(),
39-
})
38+
}
4039
}
4140
}
Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use crate::Error;
21
use crate::organization::model::OrganizationHead;
32
use serde::{Deserialize, Serialize};
43
use trustify_entity::organization;
@@ -11,21 +10,16 @@ pub struct OrganizationSummary {
1110
}
1211

1312
impl OrganizationSummary {
14-
pub async fn from_entity(organization: &organization::Model) -> Result<Self, Error> {
15-
Ok(OrganizationSummary {
16-
head: OrganizationHead::from_entity(organization).await?,
17-
})
18-
}
19-
20-
pub async fn from_entities(organizations: &[organization::Model]) -> Result<Vec<Self>, Error> {
21-
let mut summaries = Vec::new();
22-
23-
for org in organizations {
24-
summaries.push(OrganizationSummary {
25-
head: OrganizationHead::from_entity(org).await?,
26-
});
13+
pub fn from_entity(organization: &organization::Model) -> Self {
14+
Self {
15+
head: OrganizationHead::from_entity(organization),
2716
}
17+
}
2818

29-
Ok(summaries)
19+
pub fn from_entities(organizations: &[organization::Model]) -> Vec<Self> {
20+
organizations
21+
.iter()
22+
.map(OrganizationSummary::from_entity)
23+
.collect()
3024
}
3125
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ impl OrganizationService {
3737

3838
Ok(PaginatedResults {
3939
total,
40-
items: OrganizationSummary::from_entities(&limiter.fetch().await?).await?,
40+
items: OrganizationSummary::from_entities(&limiter.fetch().await?),
4141
})
4242
}
4343
pub async fn fetch_organization<C: ConnectionTrait>(

modules/fundamental/src/product/model/details.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,7 @@ impl ProductDetails {
2929
.find_related(product_version::Entity)
3030
.all(tx)
3131
.await?;
32-
let vendor = if let Some(org) = org {
33-
Some(OrganizationSummary::from_entity(&org).await?)
34-
} else {
35-
None
36-
};
32+
let vendor = org.map(|org| OrganizationSummary::from_entity(&org));
3733
Ok(ProductDetails {
3834
head: ProductHead::from_entity(product).await?,
3935
versions: ProductVersionDetails::from_entities(&product_versions, tx).await?,

modules/fundamental/src/product/model/summary.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,7 @@ impl ProductSummary {
2222
org: Option<organization::Model>,
2323
versions: &[product_version::Model],
2424
) -> Result<Self, Error> {
25-
let vendor = if let Some(org) = org {
26-
Some(OrganizationSummary::from_entity(&org).await?)
27-
} else {
28-
None
29-
};
25+
let vendor = org.map(|org| OrganizationSummary::from_entity(&org));
3026
Ok(ProductSummary {
3127
head: ProductHead::from_entity(product).await?,
3228
versions: ProductVersionHead::from_entities(versions).await?,

modules/fundamental/tests/issues/oom.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,22 @@
22
33
use std::str::FromStr;
44
use test_context::test_context;
5-
use test_log::test;
6-
use tracing::instrument;
75
use trustify_common::id::Id;
86
use trustify_module_fundamental::sbom::service::SbomService;
9-
use trustify_test_context::TrustifyContext;
7+
use trustify_test_context::{TrustifyContext, flame::setup_global_subscriber};
108

119
#[test_context(TrustifyContext, skip_teardown)]
12-
#[test(tokio::test)]
13-
#[instrument]
10+
#[tokio::test]
1411
#[ignore = "Only works with a pre-existing database and a specific dump"]
1512
async fn fetch(ctx: TrustifyContext) -> anyhow::Result<()> {
13+
let _guard = setup_global_subscriber();
14+
1615
// this requires an imported dataset
1716

1817
let service = SbomService::new(ctx.db.clone());
18+
// update this digest to point to a "large SBOM"
1919
let id =
20-
Id::from_str("sha256:e2fba0cf6d3c79cf6994b31e172b5f11ee5e3f9dd7629ac0f1a5ae5cae2d6135")?;
20+
Id::from_str("sha256:f293eb898192085804419f9dd40a738f20d67dd81846e88c6720f692ec5f3081")?;
2121
let statuses: Vec<String> = vec!["affected".to_string()];
2222

2323
let result = service.fetch_sbom_details(id, statuses, &ctx.db).await?;

test-context/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ test-context = { workspace = true }
2828
tokio = { workspace = true }
2929
tokio-util = { workspace = true }
3030
tracing = { workspace = true }
31+
tracing-flame = { workspace = true }
32+
tracing-subscriber = { workspace = true }
3133
utoipa-actix-web = { workspace = true }
3234

3335
[dev-dependencies]

test-context/src/flame.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
use tracing_flame::FlameLayer;
2+
use tracing_subscriber::{fmt, prelude::*, registry::Registry};
3+
4+
/// set up a global tracing subscriber, generating a trace file for flamecharts.
5+
///
6+
/// ## Usage
7+
///
8+
/// Add this to a test, using the following pattern:
9+
///
10+
/// ```rust
11+
/// use trustify_test_context::flame::setup_global_subscriber;
12+
///
13+
/// #[test]
14+
/// fn test() {
15+
/// let _guard = setup_global_subscriber();
16+
///
17+
/// // test code
18+
/// }
19+
/// ```
20+
///
21+
/// If the test ran successfully, this will create a file named `tracing.folded` containing the
22+
/// result.
23+
///
24+
#[allow(clippy::test_attr_in_doctest)]
25+
#[allow(clippy::unwrap_used)]
26+
pub fn setup_global_subscriber() -> impl Drop {
27+
let fmt_layer = fmt::Layer::default();
28+
29+
let (flame_layer, _guard) = FlameLayer::with_file("./tracing.folded").unwrap();
30+
31+
let subscriber = Registry::default().with(fmt_layer).with(flame_layer);
32+
33+
tracing::subscriber::set_global_default(subscriber).expect("Could not set global default");
34+
_guard
35+
}

test-context/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
pub mod app;
44
pub mod auth;
55
pub mod call;
6+
pub mod flame;
67
pub mod spdx;
78
pub mod subset;
89

0 commit comments

Comments
 (0)