Closed calebschoepp closed 1 month ago
Do we want to remove key values everywhere from OTel traces? This seems valuable enough to me that I think we should just keep it and document it.
We could perhaps drop it by default but allow opting-in by level, e.g.
#[instrument(..., fields(key = Empty))]
...
// I believe this would allow e.g. RUST_LOG=spin_factor_key_value=debug
if tracing::enabled!(Level::DEBUG) {
span.record("key", key);
}
Ready for another round of review. I think that we should just keep keys for now and we can handle feedback in the future. I feel that it is a reasonable requirement that PII should be kept out of keys.
Do we want to remove key values everywhere from OTel traces? This seems valuable enough to me that I think we should just keep it and document it.
We could perhaps drop it by default but allow opting-in by level, e.g.
#[instrument(..., fields(key = Empty))] ... // I believe this would allow e.g. RUST_LOG=spin_factor_key_value=debug if tracing::enabled!(Level::DEBUG) { span.record("key", key); }
I tried this and it didn't work.
Ready for one more review and then for someone to merge it if they think it's good to go.
@calebschoepp looks like the failure in CI is legit:
error: expected statement after outer attribute
--> crates/factor-outbound-networking/src/lib.rs:258:1
|
3 | #[instrument(fields(db.address = Empty, server.port = Empty, db.namespace = Empty))]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@calebschoepp looks like the failure in CI is legit:
error: expected statement after outer attribute --> crates/factor-outbound-networking/src/lib.rs:258:1 | 3 | #[instrument(fields(db.address = Empty, server.port = Empty, db.namespace = Empty))] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Fixed
Summary
Questions
Fixe #2816