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

switch from warning to debug when upstream labels are empty #724

Merged
merged 2 commits into from
May 29, 2024

Conversation

pdabelf5
Copy link
Collaborator

@pdabelf5 pdabelf5 commented May 28, 2024

Proposed changes

Turn down logging when upstream labels are empty.

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

@pdabelf5 pdabelf5 requested a review from a team as a code owner May 28, 2024 14:30
Copy link

codecov bot commented May 28, 2024

Codecov Report

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

Project coverage is 3.87%. Comparing base (15b84c1) to head (b64d93e).

Files Patch % Lines
collector/nginx_plus.go 0.00% 7 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main    #724   +/-   ##
=====================================
  Coverage   3.87%   3.87%           
=====================================
  Files          5       5           
  Lines       1316    1316           
=====================================
  Hits          51      51           
  Misses      1265    1265           

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

@pdabelf5 pdabelf5 force-pushed the upstream-labels-log branch from da248e4 to af8953b Compare May 28, 2024 14:57
@pdabelf5 pdabelf5 force-pushed the upstream-labels-log branch from 9b0ea5e to b64d93e Compare May 28, 2024 14:58
Copy link

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

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

LGTM

@reddyblokesh
Copy link

@pdabelf5 @oseoin @j1m-ryan this issue is still not resolved and we are getting level=debug ts=2024-06-17T07:28:09.172Z caller=nginx_plus.go:849 msg="wrong number of labels for upstream peer, empty labels will be used instead" upstream=tritonExternalSelfHosted-dev peer expected=1 got=0

can you please look into this at earliest since this is serious performance degradation for nginx-plus users .

lucacome added a commit that referenced this pull request Jul 17, 2024
lucacome added a commit that referenced this pull request Jul 24, 2024
lucacome added a commit that referenced this pull request Jul 24, 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
7 participants