Skip to content

Conversation

@jonathanio
Copy link
Contributor

Refactor the basic Prometheus metrics handling to:

  • Allow a split between web and metrics endpoints, so general requests to the /alive and /healthz metrics in the metrics endpoint don't skew the metrics for the web endpoint.
  • Add support for response time summary and quantiles on a per-service basis rather than per-method/path/status.
  • Add support for response size metrics.
  • Add support for counting in-flight metrics.

Checklist

Before raising or requesting a review of the pull request, please check and confirm the following items have been performed, where possible:

  • I have performed a self-review of my code and run any tests locally to check
  • I have added tests that prove that any changes are effective and work correctly
  • I have made corresponding changes, as needed, to the repository documentation
  • Each commit in, and this pull request, have meaningful subjects and bodies for context
  • I have added release/..., type/..., and changes/... labels, as needed, to this pull request

Refacor the basic Prometheus metrics handling to:

- Allow a split between web and metrics endpoints, so general requests
  to the /alive and /healthz metrics in the metrics endpoint don't
  skew the metrics for the web endpoint.
- Add support for response time summary and quantiles on a per-service
  basis, rather than per-method/path/status.
- Add support for response size metrics.
- Add support for counting in-flight metrics.
Switch off the metrics endpoint access logs as it's unlikely they're
going to be of significant value, but allow them to be re-enabled by the
command-line or configuration file.
@jonathanio jonathanio added priority/normal This is a normal-priority issue or pull request release/update An update to an existing feature is made with this pull request type/refactoring A refactoring of existing code update/go Update with changes to Go files or applications labels Aug 15, 2024
@jonathanio jonathanio self-assigned this Aug 15, 2024
@jonathanio jonathanio enabled auto-merge August 15, 2024 20:42
@jonathanio jonathanio merged commit a6ff104 into main Aug 15, 2024
@jonathanio jonathanio deleted the refactor-prometheus-metrics branch August 15, 2024 20:45
@codecov
Copy link

codecov bot commented Aug 15, 2024

Codecov Report

Attention: Patch coverage is 0% with 29 lines in your changes missing coverage. Please review.

Project coverage is 9.74%. Comparing base (06195e9) to head (d7aa62e).
Report is 3 commits behind head on main.

Files Patch % Lines
internal/serve/middleware/prometheus.go 0.00% 21 Missing ⚠️
internal/cmd/serve.go 0.00% 3 Missing ⚠️
internal/serve/metrics/main.go 0.00% 3 Missing ⚠️
internal/serve/middleware/metrics.go 0.00% 1 Missing ⚠️
internal/serve/web/main.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main     #14      +/-   ##
=========================================
- Coverage   10.04%   9.74%   -0.30%     
=========================================
  Files          17      17              
  Lines         498     513      +15     
=========================================
  Hits           50      50              
- Misses        448     463      +15     
Flag Coverage Δ
unit-tests 9.74% <0.00%> (-0.30%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
internal/serve/middleware/logger.go 0.00% <ø> (ø)
internal/serve/middleware/metrics.go 0.00% <0.00%> (ø)
internal/serve/web/main.go 0.00% <0.00%> (ø)
internal/cmd/serve.go 0.00% <0.00%> (ø)
internal/serve/metrics/main.go 0.00% <0.00%> (ø)
internal/serve/middleware/prometheus.go 0.00% <0.00%> (ø)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority/normal This is a normal-priority issue or pull request release/update An update to an existing feature is made with this pull request type/refactoring A refactoring of existing code update/go Update with changes to Go files or applications

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants