Closed sgfn closed 1 year ago
I don't see any usage of those metrics too but Phoenix Team wouldn't include something that is useless, would it?
Cloud says they don't need them, as every metric update in Prometheus comes with a timestamp, anyway. But it doesn't hurt to keep them, I guess
Merging #62 (a9156b3) into main (1302b8a) will decrease coverage by
1.89%
. Report is 1 commits behind head on main. The diff coverage is27.77%
.
@@ Coverage Diff @@
## main #62 +/- ##
==========================================
- Coverage 86.11% 84.23% -1.89%
==========================================
Files 33 34 +1
Lines 504 520 +16
==========================================
+ Hits 434 438 +4
- Misses 70 82 +12
Files Changed | Coverage Δ | |
---|---|---|
lib/jellyfish/application.ex | 80.00% <ø> (ø) |
|
lib/jellyfish_web/telemetry/metrics_aggregator.ex | 23.52% <23.52%> (ø) |
|
lib/jellyfish_web/telemetry.ex | 100.00% <100.00%> (+20.00%) |
:arrow_up: |
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 1302b8a...a9156b3. Read the comment docs.
The existing (and new) metric names were changed according to the Prometheus guidelines (see also this page from the
telemetry_metrics_prometheus_core
docs).Explanation for the Phoenix metrics can be found here. Personally, I don't really see any use for the metrics which use
system_time
measurements (they return the system time of when a given event occured, so their graph in time is a linear function ¯_(ツ)_/¯), so I think we should drop them -- but we can discuss this.EDIT: Right now, as we're subscribed to ICE events, the traffic metrics DO NOT INCLUDE traffic from/to RTSP and HLS components. The Cloud team has agreed to this (right now, they're going to use these metrics as a PoC anyway), but we have to keep this in mind for the future.