Skip to content

Conversation

@plyr4
Copy link
Contributor

@plyr4 plyr4 commented Aug 9, 2022

what?

this adds query parameter support to the /metrics endpoint.

why?

without selective querying, the /metrics endpoint will perform expensive and exhaustive queries to retrieve information from the database

breaking change!

the default values for each query parameter are false so that you must selectively request additional information from the database, which means anyone using the /metrics endpoint as it works currently will stop receiving data unless they update their query from

curl 'localhost:8080/metrics'

to

curl 'localhost:8080/metrics?user_count=t&build_count=true'

and include each piece of data they would like to query for.

docs

once i get approval on the structure and usage of the parameters i will create the documentation before merging

@plyr4 plyr4 self-assigned this Aug 9, 2022
@plyr4 plyr4 requested a review from a team as a code owner August 9, 2022 19:38
@codecov
Copy link

codecov bot commented Aug 9, 2022

Codecov Report

Merging #682 (0a89f9e) into master (048364c) will decrease coverage by 0.12%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #682      +/-   ##
==========================================
- Coverage   55.20%   55.07%   -0.13%     
==========================================
  Files         201      201              
  Lines       15916    15952      +36     
==========================================
  Hits         8786     8786              
- Misses       6752     6788      +36     
  Partials      378      378              
Impacted Files Coverage Δ
api/metrics.go 0.00% <0.00%> (ø)

cognifloyd
cognifloyd previously approved these changes Aug 10, 2022
Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't use the metrics endpoint so far, but this looks sane from reading.

Copy link
Contributor

@ecrupper ecrupper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@plyr4 plyr4 merged commit 7d6c7d5 into master Aug 18, 2022
@plyr4 plyr4 deleted the enhance/metrics-query-params branch August 18, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants