Conversation
| ResolveLatency resolve_latency = 4; | ||
| repeated ResolveRate resolve_rate = 5; | ||
| StateAge state_age = 6; |
There was a problem hiding this comment.
Can we add all the tags we want to have? I believe env and client_id are missing
There was a problem hiding this comment.
I was thinking they'd be known by flag-resolver-service through auth? Env is part of client right?
There was a problem hiding this comment.
what client_id? the sdk wouldn't know this.
| // Timestamp client_time = 3; | ||
|
|
||
| ResolveLatency resolve_latency = 4; | ||
| repeated ResolveRate resolve_rate = 5; |
There was a problem hiding this comment.
Is there a way to encode that we intend this to be a Set?
There was a problem hiding this comment.
We could do a map from ResolveReason to count. But that will break if we ever want to add a new tag. I think it's acceptable that the wire format doesn't enforce business rules..
| fn reason_to_telemetry(reason: i32) -> Option<Reason> { | ||
| match reason { | ||
| x if x == ResolveReason::Match as i32 => Some(Reason::Match), | ||
| x if x == ResolveReason::NoSegmentMatch as i32 => Some(Reason::NoSegmentMatch), | ||
| x if x == ResolveReason::FlagArchived as i32 => Some(Reason::FlagArchived), | ||
| x if x == ResolveReason::TargetingKeyError as i32 => Some(Reason::TargetingKeyError), | ||
| _ => None, | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe we should skip trying to have specific errors in the metric and just use reason?
| let start = WasmHost::current_time(); | ||
| let result = resolver.resolve_flags_sticky(&request); | ||
| let end = WasmHost::current_time(); |
There was a problem hiding this comment.
This is wrong. We want to measure the whole request which might be more than one call into the WASM so we need to store the start time in the response and have it roundtrip etc..
3 participants
Proposal for wire format for monitoring data