Skip to content

Commit e7eb0ff

Browse files
committed
feat: wire kms.cache.operations.total, kms.hsm.operations.total
1 parent 8ebecb4 commit e7eb0ff

11 files changed

Lines changed: 141 additions & 28 deletions

File tree

.github/copilot-instructions.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -470,9 +470,7 @@ If any answer is "yes", fix it before finishing.
470470
> Create the branch-specific file if it does not exist yet.
471471
> Determine the branch name by running `git branch --show-current` — never guess.
472472
473-
Update CHANGELOG/<branch_name_without_slashes>.md only when the change is high-level and worth a release note, such as a new feature, a user-visible behavior change, a bug fix, a security change, a compatibility change, or a significant refactor that materially affects users or operators. Skip changelog entries for routine implementation work, local cleanup, formatting, minor refactors, and test-only changes unless they affect observable behavior. Use the sections convention: `Features`, `Bug Fixes`, `Build`, `Refactor`, `Documentation`, `Testing`, `CI`, `Security`. Under a section, regroup by sub-feature or component when multiple entries relate to the same area.
474-
475-
**Decision rule**: If the change alters any of: public API signatures, CLI flags/output, config file keys, default behavior, supported algorithms, or error messages visible to operators — write a changelog entry. Otherwise skip.
473+
Update CHANGELOG/<branch_name_without_slashes>.md only when the change is high-level and worth a release note, such as a new feature, a user-visible behavior change, an important bug fix, a security change, a compatibility change, or a significant refactor that materially affects users or operators. Skip changelog entries for routine implementation work, local cleanup, formatting, minor refactors, and test-only changes unless they affect observable behavior. Use the sections convention: `Features`, `Bug Fixes`, `Build`, `Refactor`, `Documentation`, `Testing`, `CI`, `Security`. Under a section, regroup by sub-feature or component when multiple entries relate to the same area.
476474

477475
If appliable and the feature's code is complete, add the GitHub PR or issue link at the EOL: `([#XXX](https://github.com/Cosmian/kms/issues/XXX))`. Add at the bottom `Closes #xxx` lines as needed to automatically close related issues.
478476

CHANGELOG/feat_richOTELmetrics.md

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
## Features
44

5-
### Database Metrics Wiring (Step 1)
5+
### Database Metrics Wiring `kms.database.operations.total`, `kms.database.operation.duration` (Step 1)
66

77
- Wire `kms.database.operations.total` (counter) and `kms.database.operation.duration`
88
(histogram) at the `Database` facade layer with `operation`, `backend`, and `outcome`
@@ -15,7 +15,7 @@
1515
- `MainDbKind::as_str()` provides canonical backend labels
1616
(`"sqlite"`, `"postgresql"`, `"mysql"`, `"redis"`).
1717

18-
### HTTP Metrics Wiring (Step 2)
18+
### HTTP Metrics Wiring `kms.http.requests.total`, `kms.http.request.duration`, `kms.active.connections` (Step 2)
1919

2020
- Add `OtelHttpMetrics` Actix-web middleware (`crate/server/src/middlewares/otel_http_middleware.rs`)
2121
that records `kms.http.requests.total` (counter with `method`, `path`, `status`
@@ -54,7 +54,7 @@
5454
- Add `Database::count_all_non_destroyed_objects()` facade that sums counts across all
5555
registered stores with `saturating_add`, tolerating partial failures.
5656

57-
### Object Count Metric — Redis-findex backend (Step 3, continued)
57+
### Object Count Metric — `kms.objects.total`, Redis-findex backend (Step 3, continued)
5858

5959
- Implement `count_all_non_destroyed` for the `RedisWithFindex` backend using an
6060
O(1) counter key (`kms::metrics::live_object_count`) instead of a full key scan.
@@ -122,6 +122,36 @@ state is not `Destroyed` or `Destroyed_Compromised`.
122122
Runs without any hardware. Added `tokio` as a dev-dependency to
123123
`cosmian_kms_interfaces`.
124124

125+
126+
### Cache and HSM Operation Metrics — `kms.cache.operations.total`, `kms.hsm.operations.total` (Step 4)
127+
128+
129+
- Add `hsm_model_from_prefix(hsm_instances, prefix) -> &str` to
130+
`crate/server/src/core/uid_utils.rs`. Looks up the human-readable HSM model
131+
label (e.g. `"softhsm2"`, `"utimaco"`) from the configured `hsm_instances`
132+
slice and falls back to the prefix string itself when no matching instance is
133+
found, ensuring the metric label is always non-empty.
134+
- Export `HsmInstanceParams` from `crate/server/src/config/mod.rs` (re-exported
135+
through `params/mod.rs`) so that `uid_utils` and future callers can reference
136+
it without reaching into private sub-modules.
137+
- Wire `kms.cache.operations.total` in `get_unwrapped()`
138+
(`crate/server/src/core/kms/other_kms_methods.rs`): emit `record_cache_operation("get", "hit")`
139+
on a cache hit, `record_cache_operation("get", "miss")` on a miss, and
140+
`record_cache_operation("insert", "ok")` after a successful cache population.
141+
- Wire `kms.hsm.operations.total` at three dispatch points:
142+
- `perform_crypto_operation()` in `key_ops/crypto_op.rs` — covers all six
143+
oracle-routed operations (Encrypt, Decrypt, Sign, SignatureVerify, MAC,
144+
MACVerify) via the `ResolvedKey::Oracle` arm. Uses `Op::OP_NAME` as the
145+
operation label and `hsm_model_from_prefix` for the model label.
146+
- `wrap_using_crypto_oracle()` in `core/wrapping/wrap.rs` — emits
147+
`record_hsm_operation("Wrap", model)` after the oracle `encrypt` call.
148+
- `unwrap_using_crypto_oracle()` in `core/wrapping/unwrap.rs` — emits
149+
`record_hsm_operation("Unwrap", model)` after the oracle `decrypt` call.
150+
- Add 4 unit tests in `core::uid_utils::tests` covering legacy `"hsm"` prefix,
151+
new `"hsm::softhsm2"` format, multi-instance selection, and unknown-prefix
152+
fallback. All tests are sync, require no async runtime, and pass without
153+
hardware.
154+
125155
## Testing
126156

127157
### Mockall test infrastructure

Cargo.lock

Lines changed: 0 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crate/server/src/config/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ mod params;
33
pub mod wizard;
44

55
pub use command_line::*;
6-
pub use params::{KmipPolicyParams, OpenTelemetryConfig, ProxyParams, ServerParams, TlsParams};
6+
pub use params::{
7+
HsmInstanceParams, KmipPolicyParams, OpenTelemetryConfig, ProxyParams, ServerParams, TlsParams,
8+
};
79

810
#[derive(Debug, Clone)]
911
pub struct IdpConfig {

crate/server/src/config/params/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ mod tls_params;
77
pub use kmip_policy_params::KmipPolicyParams;
88
pub use open_telemetry_params::OpenTelemetryConfig;
99
pub use proxy_params::ProxyParams;
10-
pub use server_params::ServerParams;
10+
pub use server_params::{HsmInstanceParams, ServerParams};
1111
pub use tls_params::TlsParams;

crate/server/src/core/kms/other_kms_methods.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,17 @@ impl KMS {
6262
// check if we have it in the cache
6363
if let Some(u) = self.database.unwrapped_cache().peek(uid, object).await? {
6464
debug!("Unwrapped cache hit");
65+
if let Some(ref metrics) = self.metrics {
66+
metrics.record_cache_operation("get", "hit");
67+
}
6568
return Ok(u);
6669
}
6770

6871
// cache miss, try to unwrap
6972
debug!("Unwrapped cache miss. Calling unwrap");
73+
if let Some(ref metrics) = self.metrics {
74+
metrics.record_cache_operation("get", "miss");
75+
}
7076
let unwrapped_object = {
7177
let mut unwrapped_object = object.clone();
7278
unwrap_object(&mut unwrapped_object, self, user).await?;
@@ -78,6 +84,9 @@ impl KMS {
7884
.unwrapped_cache()
7985
.insert(uid.to_owned(), object, unwrapped_object.clone())
8086
.await?;
87+
if let Some(ref metrics) = self.metrics {
88+
metrics.record_cache_operation("insert", "ok");
89+
}
8190

8291
Ok(unwrapped_object)
8392
}

crate/server/src/core/operations/key_ops/crypto_op.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,15 @@ pub(crate) async fn perform_crypto_operation<Op: CryptoOpSpec>(
163163

164164
match resolve_key_for_operation::<Op>(unique_identifier, kms, user).await? {
165165
ResolvedKey::Oracle { uid, prefix } => {
166-
Op::execute_oracle(kms, &request, &uid, &prefix).await
166+
let result = Op::execute_oracle(kms, &request, &uid, &prefix).await;
167+
if let Some(ref metrics) = kms.metrics {
168+
let model = crate::core::uid_utils::hsm_model_from_prefix(
169+
&kms.params.hsm_instances,
170+
&prefix,
171+
);
172+
metrics.record_hsm_operation(Op::OP_NAME, model);
173+
}
174+
result
167175
}
168176
ResolvedKey::Local(owm) => {
169177
let mut owm = *owm;

crate/server/src/core/otel_metrics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,8 +454,8 @@ impl OtelMetrics {
454454
self.hsm_operations_total.add(
455455
1,
456456
&[
457-
KeyValue::new("operation", operation.to_owned()),
458457
KeyValue::new("hsm_model", hsm_model.to_owned()),
458+
KeyValue::new("operation", operation.to_owned()),
459459
],
460460
);
461461
}

crate/server/src/core/uid_utils.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::collections::HashSet;
33
use cosmian_kms_server_database::reexport::cosmian_kmip::kmip_2_1::kmip_types::UniqueIdentifier;
44

55
use crate::{
6+
config::HsmInstanceParams,
67
core::KMS,
78
result::{KResult, KResultHelper},
89
};
@@ -31,6 +32,26 @@ pub(crate) fn has_prefix(uid: &str) -> Option<&str> {
3132
None
3233
}
3334

35+
/// Resolve a human-readable HSM model label from a routing prefix.
36+
///
37+
/// Searches the configured `hsm_instances` for an entry whose `prefix` matches
38+
/// the given prefix and returns its `model` field (e.g. `"softhsm2"`).
39+
/// Falls back to the prefix string itself when no matching instance is found,
40+
/// ensuring call sites always get a usable label without panicking.
41+
///
42+
/// # Arguments
43+
/// * `hsm_instances` — the slice from `ServerParams::hsm_instances`
44+
/// * `prefix` — a routing prefix as returned by [`has_prefix`]
45+
pub(crate) fn hsm_model_from_prefix<'a>(
46+
hsm_instances: &'a [HsmInstanceParams],
47+
prefix: &'a str,
48+
) -> &'a str {
49+
hsm_instances
50+
.iter()
51+
.find(|i| i.prefix == prefix)
52+
.map_or(prefix, |i| i.model.as_str())
53+
}
54+
3455
/// Determine the list of possible UIDs from a Unique Identifier,
3556
/// that may contain tags.
3657
/// # Arguments
@@ -52,3 +73,56 @@ pub(super) async fn uids_from_unique_identifier(
5273
}
5374
Ok(HashSet::from([uid_or_tags.to_owned()]))
5475
}
76+
77+
#[cfg(test)]
78+
mod tests {
79+
use std::collections::HashMap;
80+
81+
use super::*;
82+
83+
fn make_instance(prefix: &str, model: &str) -> HsmInstanceParams {
84+
HsmInstanceParams {
85+
model: model.to_owned(),
86+
admin: vec![],
87+
slot_passwords: HashMap::new(),
88+
prefix: prefix.to_owned(),
89+
}
90+
}
91+
92+
#[test]
93+
fn test_hsm_model_legacy_prefix() {
94+
// Legacy format: UID "hsm::<slot>::<key>" → prefix == "hsm"
95+
let instances = vec![make_instance("hsm", "softhsm2")];
96+
assert_eq!(hsm_model_from_prefix(&instances, "hsm"), "softhsm2");
97+
}
98+
99+
#[test]
100+
fn test_hsm_model_new_format_prefix() {
101+
// New format: UID "hsm::softhsm2::<slot>::<key>" → prefix == "hsm::softhsm2"
102+
let instances = vec![make_instance("hsm::softhsm2", "softhsm2")];
103+
assert_eq!(
104+
hsm_model_from_prefix(&instances, "hsm::softhsm2"),
105+
"softhsm2"
106+
);
107+
}
108+
109+
#[test]
110+
fn test_hsm_model_second_instance() {
111+
// Multiple instances; prefix selects the correct one.
112+
let instances = vec![
113+
make_instance("hsm::softhsm2", "softhsm2"),
114+
make_instance("hsm::utimaco", "utimaco"),
115+
];
116+
assert_eq!(hsm_model_from_prefix(&instances, "hsm::utimaco"), "utimaco");
117+
}
118+
119+
#[test]
120+
fn test_hsm_model_fallback_to_prefix_when_unknown() {
121+
// Unknown prefix with empty instance list → falls back to the prefix itself.
122+
let instances: Vec<HsmInstanceParams> = vec![];
123+
assert_eq!(
124+
hsm_model_from_prefix(&instances, "hsm::unknown"),
125+
"hsm::unknown"
126+
);
127+
}
128+
}

crate/server/src/core/wrapping/unwrap.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,11 @@ async fn unwrap_using_crypto_oracle(
203203
let plaintext = crypto_oracle
204204
.decrypt(&unwrapping_key_uid, wrapped_key, None, None)
205205
.await?;
206+
if let Some(ref metrics) = kms.metrics {
207+
let model =
208+
crate::core::uid_utils::hsm_model_from_prefix(&kms.params.hsm_instances, prefix);
209+
metrics.record_hsm_operation("Unwrap", model);
210+
}
206211

207212
// decode the unwrapped key
208213
let key_value = decode_unwrapped_key(

0 commit comments

Comments
 (0)