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

feat: exposes baseline metrics #12

Merged
merged 6 commits into from
May 6, 2024

Conversation

saltiyazan
Copy link
Contributor

@saltiyazan saltiyazan commented Apr 30, 2024

Description

  • Exposes basic Go metrics on the Metric endpoint
  • Adds a test case
  • Improves configYaml struct to specify required keys

The solution creates a custom handler for the metrics endpoint, that handler will run for any collectors registered in the metrics registry, for now we only have the collector of some basic Go metrics, but this should be easy to extend in the future with custom collectors.

To test this, run the service and curl the /metrics endpoint.
To test this with prometheus, configure prometheus with the following job:

scrape_configs:
  - job_name: gocert
    scrape_interval: 10s
    static_configs:
    - targets:
      - localhost:443

Run prometheus then curl the metrics endpoint of prometheus, for example:
`curl http://localhost:9090/metrics | grep gocerts'

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that validate the behaviour of the software
  • I validated that new and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@saltiyazan saltiyazan requested a review from a team as a code owner April 30, 2024 12:13
internal/api/handlers.go Outdated Show resolved Hide resolved
internal/api/handlers.go Outdated Show resolved Hide resolved
@saltiyazan saltiyazan marked this pull request as draft May 1, 2024 15:37
@saltiyazan
Copy link
Contributor Author

Converted to draft, figuring out why are unit tests failing

Creates a package for metrics where handlers are moved
@saltiyazan saltiyazan force-pushed the TLSENG-225-exposes-baseline-metrics branch from d2d383c to 8ae91a9 Compare May 2, 2024 07:23
@saltiyazan saltiyazan force-pushed the TLSENG-225-exposes-baseline-metrics branch from ad11777 to c7442d4 Compare May 2, 2024 09:08
@saltiyazan saltiyazan marked this pull request as ready for review May 2, 2024 09:09
@saltiyazan saltiyazan requested review from kayra1 and gruyaume May 2, 2024 09:09
internal/api/handlers.go Outdated Show resolved Hide resolved
internal/api/server.go Outdated Show resolved Hide resolved
internal/metrics/metrics.go Outdated Show resolved Hide resolved
MetricsHandler interface is removed
Handler is instantiated in handlers.go
@saltiyazan saltiyazan requested a review from gruyaume May 2, 2024 12:11
Copy link
Collaborator

@gruyaume gruyaume left a comment

Choose a reason for hiding this comment

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

Looks good, the description should be adapted to the new approach (i.e. metrics are not under /api/v1.

I tested this locally and it looks good :)

image

Copy link
Contributor

@kayra1 kayra1 left a comment

Choose a reason for hiding this comment

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

perfect. just one small thing.

internal/api/handlers_test.go Outdated Show resolved Hide resolved
@saltiyazan saltiyazan enabled auto-merge (squash) May 6, 2024 09:25
@saltiyazan saltiyazan merged commit 3fcdac6 into main May 6, 2024
9 checks passed
@saltiyazan saltiyazan deleted the TLSENG-225-exposes-baseline-metrics branch May 6, 2024 09:39
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.

3 participants