Skip to content

Conversation

@lucacome
Copy link
Contributor

@lucacome lucacome commented Oct 1, 2024

Proposed changes

This pull request introduces several changes to the NGINX Prometheus exporter to add context and timeout handling capabilities. The main updates include modifying the GetStubStats method to accept a context parameter, adding a timeout field to the NginxCollector and NginxPlusCollector structs, and updating the Collect methods to use context with timeout.

Context and Timeout Handling Enhancements:

  • client/nginx.go: Modified the GetStubStats method to accept a context.Context parameter, enabling more flexible request handling.

  • collector/nginx.go:

    • Added context and time imports to support context with timeout.
    • Added a timeout field to the NginxCollector struct and updated the NewNginxCollector function to accept a timeout parameter.
    • Updated the Collect method to use a context with timeout for fetching stub stats.
  • collector/nginx_plus.go:

    • Added time import to support context with timeout.
    • Added a timeout field to the NginxPlusCollector struct and updated the NewNginxPlusCollector function to accept a timeout parameter. [1] [2] [3] [4]
    • Updated the Collect method to use a context with timeout for fetching NGINX Plus stats.

Code Cleanup:

  • exporter.go:
    • Simplified the registerCollector function signature and removed the Timeout field from the http.Client configuration. [1] [2] [3]

Closes #858

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 October 1, 2024 02:51
@lucacome lucacome self-assigned this Oct 1, 2024
@github-actions github-actions bot added the enhancement Pull requests for new features/feature enhancements label Oct 1, 2024
@codecov
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

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

Project coverage is 3.89%. Comparing base (9cd3941) to head (487aac4).
Report is 44 commits behind head on main.

Files with missing lines Patch % Lines
collector/nginx.go 0.00% 5 Missing ⚠️
collector/nginx_plus.go 0.00% 5 Missing ⚠️
exporter.go 0.00% 3 Missing ⚠️
client/nginx.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #868      +/-   ##
========================================
- Coverage   3.90%   3.89%   -0.01%     
========================================
  Files          5       5              
  Lines       1307    1310       +3     
========================================
  Hits          51      51              
- Misses      1256    1259       +3     

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


// NewNginxCollector creates an NginxCollector.
func NewNginxCollector(nginxClient *client.NginxClient, namespace string, constLabels map[string]string, logger *slog.Logger) *NginxCollector {
func NewNginxCollector(nginxClient *client.NginxClient, namespace string, constLabels map[string]string, logger *slog.Logger, timeout time.Duration) *NginxCollector {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this function provide a sensible default for the timeout? Currently it breaks public API.

@lucacome lucacome closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Pull requests for new features/feature enhancements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use appropriate context to call NGINX Plus API

4 participants