Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add metrics endpoint with http metrics using Prometheus client lib #1045

Merged

Conversation

mmianl
Copy link
Contributor

@mmianl mmianl commented Mar 16, 2022

Fixes #408

Requirements

Enable basic Prometheus compatible instrumentation capabilities

Description of the Change

Add metrics endpoint with basic http and go related metrics when running api serve which can be enabled or disabled by a switch in the config file (= enableMetricsEndpoint)

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated
  • author name in AUTHORS

@mmianl mmianl force-pushed the issue-408-prometheus-instrumentation branch from 520889f to 36f07e3 Compare March 29, 2022 08:47
@mmianl
Copy link
Contributor Author

mmianl commented Mar 31, 2022

Hello there,
what do you think of this change? The metrics endpoint can now be enabled or disabled by config file.

@randombenj
Copy link
Contributor

Hi @mmianl
Thanks a lot for the changes! Do you think you could add some simple tests to verify the functionality?

@mmianl
Copy link
Contributor Author

mmianl commented Mar 31, 2022

Hi @randombenj
Yes, I can certainly take a look at functional tests, I don't think that unit tests make a lot of sense, what do you reckon?

@randombenj
Copy link
Contributor

@mmianl Yeah that's also what I would propose, thanks!

@randombenj randombenj self-requested a review March 31, 2022 14:11
@mmianl mmianl force-pushed the issue-408-prometheus-instrumentation branch 2 times, most recently from a0d6f8e to 4178f73 Compare April 1, 2022 07:41
@mmianl mmianl force-pushed the issue-408-prometheus-instrumentation branch from 4178f73 to 3085871 Compare April 7, 2022 11:25
@mmianl
Copy link
Contributor Author

mmianl commented Apr 11, 2022

Hello @randombenj,
Do you think this can be merged now?

Copy link
Contributor

@randombenj randombenj left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -55,6 +55,11 @@ jobs:
with:
go-version: ${{ matrix.go }}

- name: golangci-lint
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool change, thanks!

@randombenj randombenj merged commit db19a56 into aptly-dev:master Apr 12, 2022
@randombenj randombenj added this to the 1.5.0 milestone Jun 23, 2022
@mmianl mmianl deleted the issue-408-prometheus-instrumentation branch October 30, 2022 15:53
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.

Feature Request: Expose Metrics
2 participants