draft public protos #414
draft public protos #414
Conversation
| enum public_record_type { | ||
| data = 0; | ||
| international_roaming_data = 1; | ||
| connection = 2; |
There was a problem hiding this comment.
Aren’t connections prerequisites of any other kind of record? Could counting these result in inflated numbers if a consumer were under the impression a “connection” were distinct from a data session?
There was a problem hiding this comment.
Connections and sessions are related but provide two distinct offerings to downstream services. They are also published at different cadences, with connections at a much faster cadence. We may or may not want to include connections in the public bucket but it is included for now as a 1-1 match for our internal feeds
There was a problem hiding this comment.
i agree it's valid to output both connections and "sessions" but it's redundant to have a public_cdr_connection_v1 which would logically always have a RecordType: PublicRecordType::Connection; the two "report" messages should be session_v1 and connection_v1 and then this record_type enum should be limited to use in the "session" messages with variants like "data", "international_roaming_data", etc. should we also account for "voice" and "text" traffic or is that not differentiated when being processed by hotspots?
There was a problem hiding this comment.
Agree, no need for RecordType in the connection proto. At this time I as I understand it we are only interested in outputting data record types
|
|
||
| enum public_traffic_source { | ||
| helium_mobile = 0; | ||
| other = 1; |
There was a problem hiding this comment.
Is there any option for an “other” right now? We should avoid this catch all until we know we need it. If it’s for an “unknown” it should probably be the first option since that feels like a safer default value than attributing to any specific carrier/provider
There was a problem hiding this comment.
Other is currently in use as a placeholder until a confirmed list of sources is provided
There was a problem hiding this comment.
do we need a placeholder when adding variants to a protobuf enum is fully backward compatible?
There was a problem hiding this comment.
this is a draft, its there as a placeholder until things are confirmed and should we end up omitting a source, then it will be designated as other
| // origin of the cdr | ||
| public_traffic_source traffic_source = 4; | ||
| // the hotspot serial number | ||
| string agw_sn = 5; |
There was a problem hiding this comment.
agw_sn probably too specific to helium mobile. hotspot_sn?
There was a problem hiding this comment.
i also wonder if this is an opportunity to start distancing ourselves from the cruft of the "split identity" that a cbrs radio/hotspot pair comes with by collapsing the gateway_sn and radio_sn fields into a single hotspot_sn field which would have a oneof type and then the two variants of the oneof would be something like:
message radio_identity {
string gateway_sn = 1;
string radio_sn = 2;
}
message session_v1 {
string id = 1;
string pubkey = 2;
oneof hotspot_id {
string unified = 3;
radio_identity split = 4;
}
}
which would have some benefits when decoding/parsing the message into a language-native data type to immediately determine the type of hotspot/radio combo and avoid the need for things like handling options of strings in rust for the 99% of hotspots producing reports
|
public is in use in this draft to differentiate between the internal representations of the protos. Both internal and this public representation will be outputted by the same service |
| string radio_serial = 7; | ||
| // the cbsd_id of a cbrs radio | ||
| string cbsd_id = 8; | ||
| uint64 total_volume = 9; |
There was a problem hiding this comment.
Can we please add some comment indicating the units for each type of usage volume? I believe these are all in bytes?
| uint64 uplink_volume = 10; | ||
| uint64 downlink_volume = 11; | ||
| // timestamp in millisecs of when the record was created in the network | ||
| uint64 timestamp = 12; |
There was a problem hiding this comment.
Optionally rename this to timestamp_ms? Could do similar for any other type which has a unit, so received_timestamp -> received_timestamp_ms etc would also be nice.
There was a problem hiding this comment.
makes sense to differentiate the "publicly visible" session reports from the carrier-internal ones but avoid the "public" designation specifically since public is a keyword in proto3 (related to importing) and also a keyword in many languages that might be consuming these and could conceivably cause perceptual confusion even if it doesn't confuse any LSPs
| enum public_record_type { | ||
| data = 0; | ||
| international_roaming_data = 1; | ||
| connection = 2; |
There was a problem hiding this comment.
i agree it's valid to output both connections and "sessions" but it's redundant to have a public_cdr_connection_v1 which would logically always have a RecordType: PublicRecordType::Connection; the two "report" messages should be session_v1 and connection_v1 and then this record_type enum should be limited to use in the "session" messages with variants like "data", "international_roaming_data", etc. should we also account for "voice" and "text" traffic or is that not differentiated when being processed by hotspots?
|
|
||
| enum public_traffic_source { | ||
| helium_mobile = 0; | ||
| other = 1; |
There was a problem hiding this comment.
do we need a placeholder when adding variants to a protobuf enum is fully backward compatible?
| // origin of the cdr | ||
| public_traffic_source traffic_source = 4; | ||
| // the hotspot serial number | ||
| string agw_sn = 5; |
There was a problem hiding this comment.
i also wonder if this is an opportunity to start distancing ourselves from the cruft of the "split identity" that a cbrs radio/hotspot pair comes with by collapsing the gateway_sn and radio_sn fields into a single hotspot_sn field which would have a oneof type and then the two variants of the oneof would be something like:
message radio_identity {
string gateway_sn = 1;
string radio_sn = 2;
}
message session_v1 {
string id = 1;
string pubkey = 2;
oneof hotspot_id {
string unified = 3;
radio_identity split = 4;
}
}
which would have some benefits when decoding/parsing the message into a language-native data type to immediately determine the type of hotspot/radio combo and avoid the need for things like handling options of strings in rust for the 99% of hotspots producing reports
No description provided.