Skip to content

Conversation

lucacome
Copy link
Contributor

Proposed changes

After prometheus/exporter-toolkit#240 we can switch to using slog.

Closes #484

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING guide
  • I have proven my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have ensured the README is up to date
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch on my own fork

@lucacome lucacome requested a review from a team as a code owner September 10, 2024 00:25
@github-actions github-actions bot added enhancement Pull requests for new features/feature enhancements dependencies Pull requests that update a dependency file labels Sep 10, 2024
Copy link

codecov bot commented Sep 10, 2024

Codecov Report

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

Project coverage is 3.90%. Comparing base (46f7fbf) to head (c7e6d4d).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
exporter.go 0.00% 15 Missing ⚠️
collector/nginx_plus.go 0.00% 9 Missing ⚠️
collector/nginx.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #841   +/-   ##
=====================================
  Coverage   3.90%   3.90%           
=====================================
  Files          5       5           
  Lines       1307    1307           
=====================================
  Hits          51      51           
  Misses      1256    1256           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lucacome lucacome self-assigned this Sep 10, 2024
@lucacome lucacome requested review from a team and kate-osborn September 11, 2024 16:32
@kate-osborn
Copy link

To achieve consistency across all NGF logs would we need to implement slog.Handler and pass that to the slog.Logger?

@lucacome
Copy link
Contributor Author

logr has FromSlogHandler and ToSlogHandler that I think can be used for this

@kate-osborn
Copy link

logr has FromSlogHandler and ToSlogHandler that I think can be used for this

Awesome. Do you mind opening an issue in NGF backlog for that work?

@lucacome lucacome merged commit 5c317a6 into main Sep 11, 2024
14 of 15 checks passed
@lucacome lucacome deleted the enh/slog branch September 11, 2024 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support logger injection
3 participants