interledgerjs / ilp-connector

Reference implementation of an Interledger connector.
Other
136 stars 53 forks source link

Merge Prometheus ILP metrics with default metrics #489

Closed martinlowinski closed 5 years ago

martinlowinski commented 5 years ago

As discussed on Slack, this is a PR to merge the metrics from the Prometheus ILP registry with the default registry and export it via the endpoint /metrics.

Prior to v22.2.0, the metrics export via /metrics used the Prometheus default registry that contains both metrics (ILP and nodejs/process) while >22.2.0 creates its own registry for the ILP metrics. This additional registry is only exported as JSON via the /stats endpoint, but not via /metrics anymore.

codecov-io commented 5 years ago

Codecov Report

Merging #489 into master will decrease coverage by 0.14%. The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #489      +/-   ##
==========================================
- Coverage   66.79%   66.64%   -0.15%     
==========================================
  Files          44       44              
  Lines        1822     1826       +4     
  Branches      309      309              
==========================================
  Hits         1217     1217              
- Misses        479      483       +4     
  Partials      126      126
Impacted Files Coverage Δ
src/services/admin-api.ts 50.76% <0%> (-1.2%) :arrow_down:
src/services/stats.ts 96.42% <0%> (-3.58%) :arrow_down:

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 784c1d1...cc6536c. Read the comment docs.

adrianhopebailie commented 5 years ago

LGTM although @sentientwaffle may have some comments regarding things he's seen related to performance in the past.

Would like to get his input before merging.