chore: add aibridge data to telemetry#20449
Conversation
| @@ -0,0 +1,33 @@ | |||
| CREATE TABLE telemetry_heartbeats ( | |||
| event_type TEXT NOT NULL CONSTRAINT telemetry_heartbeat_event_type_check CHECK (event_type IN ('aibridge_interceptions_snapshot')), | |||
| heartbeat_timestamp TIMESTAMP WITH TIME ZONE NOT NULL, | |||
There was a problem hiding this comment.
The table includes the term "heartbeat" already, and created_at aligns better with naming in other tables.
| heartbeat_timestamp TIMESTAMP WITH TIME ZONE NOT NULL, | |
| created_at TIMESTAMP WITH TIME ZONE NOT NULL, |
There was a problem hiding this comment.
created_at doesn't make sense because the creation time of the row is not the same as the period the heartbeat was calculated. I wanted to call it just timestamp but timestamp is a type in postgres so we'd have to use double quotes wherever we referenced it. So I just decided to name it heartbeat_timestamp instead :(
|
|
||
| COMMENT ON TABLE telemetry_heartbeats IS 'Telemetry heartbeat tracking table for deduplication of event types across replicas.'; | ||
| COMMENT ON COLUMN telemetry_heartbeats.event_type IS 'The type of event that was sent.'; | ||
| COMMENT ON COLUMN telemetry_heartbeats.heartbeat_timestamp IS 'The timestamp of the heartbeat event. Usually the end of the period for which the event contains data.'; |
There was a problem hiding this comment.
I think we need a better term than timestamp (or created_at) based on this comment.
In fact, the concept of a heartbeat is probably an unhelpful abstraction IMHO. You're effectively creating a table of durable locks, so this could be more general-purpose.
If we don't want to go too broad, maybe renaming this to telemetry_locks and explicitly name things like locked_until, reported_until, or something in that vein.
There was a problem hiding this comment.
Why do we need to make this more general purpose? What other telemetry-related construct would require locks other than an aggregation over a window like this? I'm fine with changing the name of the table if we don't like the term "heartbeat" but I don't think this needs to support any additional locking functionality that won't be used.
Our telemetry system can handle inaccurate/duplicate/missing data on the backend, so this only has to be best effort.
There was a problem hiding this comment.
It just feels like a reusable pattern, but yeah let's not inflate the scope right now.
The naming should change though.
|
|
||
| COMMENT ON TABLE telemetry_heartbeats IS 'Telemetry heartbeat tracking table for deduplication of event types across replicas.'; | ||
| COMMENT ON COLUMN telemetry_heartbeats.event_type IS 'The type of event that was sent.'; | ||
| COMMENT ON COLUMN telemetry_heartbeats.heartbeat_timestamp IS 'The timestamp of the heartbeat event. Usually the end of the period for which the event contains data.'; |
There was a problem hiding this comment.
It just feels like a reusable pattern, but yeah let's not inflate the scope right now.
The naming should change though.
| @@ -0,0 +1,33 @@ | |||
| CREATE TABLE telemetry_heartbeats ( | |||
| event_type TEXT NOT NULL CONSTRAINT telemetry_heartbeat_event_type_check CHECK (event_type IN ('aibridge_interceptions_snapshot')), | |||
| heartbeat_timestamp TIMESTAMP WITH TIME ZONE NOT NULL, | |||
Relates to https://github.com/coder/coder-telemetry-server/issues/27