feat: add endpoint for batching different metrics in single request #16
Conversation
adklempner
force-pushed
the
feat/batched-metrics
branch
from
c8c5b07
to
243a92a
Compare
adklempner
force-pushed
the
feat/batched-metrics
branch
from
243a92a
to
50e88d3
Compare
telemetry/server.go
Outdated
| if err := decoder.Decode(&telemetryData); err != nil { | ||
| log.Println(err) | ||
| } |
There was a problem hiding this comment.
If it failed to decode the body, we should probably fail early here with a 4xx error
There was a problem hiding this comment.
updated in latest commit
telemetry/server.go
Outdated
|
|
||
| for _, data := range telemetryData { | ||
| switch data.TelemetryType { | ||
| case "ProtocolStats": |
There was a problem hiding this comment.
We can use the constant ProtocolStatsMetric here and same for other cases. Assuming go complains about the var type then we can change TelemetryType to be an alias instead of a type like this:
type TelemetryType = string
telemetry/receivedenevlope.go
Outdated
| if err != nil { | ||
| if errors.Is(err, sql.ErrNoRows) { | ||
| return nil | ||
| } else { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
ErrNoRows shouldnt happen.
| if err != nil { | |
| if errors.Is(err, sql.ErrNoRows) { | |
| return nil | |
| } else { | |
| return err | |
| } | |
| } | |
| if err != nil { | |
| return err | |
| } |
telemetry/receivedenevlope.go
Outdated
| lastInsertId, _ = res.LastInsertId() | ||
|
|
||
| if err != nil { |
There was a problem hiding this comment.
We should check for the error of stmt.Exec before retrieving the LastInsertId,
Also, we should check for the error in res.LastInsertId instead of using _.
There was a problem hiding this comment.
I end up seeing this error:
Error saving sent envelope: LastInsertId is not supported by this driver Id:8
It looks like postgres does not support LastInsertId. This is probably why the existing code for received envelopes uses QueryRow and Scan to get the id
adklempner
force-pushed
the
feat/batched-metrics
branch
from
50e88d3
to
fae79db
Compare
This commit adds a new endpoint which expects the body to be a JSON array of arbitrary metrics, allowing a single request to include more than one metric of one or more types. Also adds an optional status version field to tables.
adklempner
force-pushed
the
feat/batched-metrics
branch
from
fae79db
to
a19ec01
Compare
Note that this does not break backwards compatibility with older versions of status (pre this PR status-im/status-go#5251)