Codecov Report

Merging #2684 (21d0e09) into master (a5606ca) will decrease coverage by 0.02877%.
The diff coverage is 0.00000%.

@@                 Coverage Diff                 @@
##              master       #2684         +/-   ##
===================================================
- Coverage   56.33265%   56.30388%   -0.02877%     
===================================================
  Files             88          88                 
  Lines          19131       19139          +8     
===================================================
- Hits           10777       10776          -1     
- Misses          7765        7771          +6     
- Partials         589         592          +3     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5606ca...21d0e09. Read the comment docs.

@yondonfu are you ok with me just testing this locally by putting artificial hard coded errors in? Reproducing them in an environment seems tricky.

@yondonfu are you ok with me just testing this locally by putting artificial hard coded errors in? Reproducing them in an environment seems tricky.

That seems reasonable.

The PR itself looks good, but I can't connect the dots of how adding 3 new metrics fixes too high success rate metric.

From my understanding, adding 3 new metrics (what I see in these PR) will just add more data, however the old query in Grafana will display exactly the same as before. Do we plan to use a different query in Grafana now?

We continue using the same grafana query because these new calls to SegmentTranscodeFailed() mean that these failure scenarios get taken into account for the success metric. This is because SegmentTranscodeFailed() results in a call to countSegmentTranscoded() with the failed argument true here: https://github.com/livepeer/go-livepeer/blob/a5606ca7/monitor/census.go#L1369
This is the function that records the data used for the success rate metric, this same function is called in the successful case with the failed argument false.

Yondon had a good explanation on the ticket too if it helps: #2674

The PR itself looks good, but I can't connect the dots of how adding 3 new metrics fixes too high success rate metric.
From my understanding, adding 3 new metrics (what I see in these PR) will just add more data, however the old query in Grafana will display exactly the same as before. Do we plan to use a different query in Grafana now?

We continue using the same grafana query because these new calls to SegmentTranscodeFailed() mean that these failure scenarios get taken into account for the success metric. This is because SegmentTranscodeFailed() results in a call to countSegmentTranscoded() with the failed argument true here: https://github.com/livepeer/go-livepeer/blob/a5606ca7/monitor/census.go#L1369 This is the function that records the data used for the success rate metric, this same function is called in the successful case with the failed argument false.

Yondon had a good explanation on the ticket too if it helps: #2674

Ahh, I see. Thanks for the explanation!