8000 feat(http): disable healthcheck access logs with env by deepankarm · Pull Request #4814 · jina-ai/serve · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(http): disable healthcheck access logs with env #4814

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 26, 2022
Merged

feat(http): disable healthcheck access logs with env #4814

merged 3 commits into from
May 26, 2022

Conversation

deepankarm
Copy link
Contributor

Goals:

  • ...
  • ...
  • check and update documentation. See guide and ask the team.

@github-actions github-actions bot added size/S area/core This issue/PR affects the core codebase area/testing This issue/PR affects testing labels May 20, 2022
@deepankarm deepankarm force-pushed the http-healthcheck-logs branch from 49a931e to 7cbb4c3 Compare May 20, 2022 16:01
JoanFM
JoanFM suggested changes May 20, 2022
View reviewed changes
Copy link
Contributor
@JoanFM JoanFM left a comment

Choose a reason for hiding this comment

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

We need to add some documentation about this

@codecov
Copy link
codecov bot commented May 20, 2022 •
edited
Loading

Codecov Report

Merging #4814 (04736ca) into master (1ec2655) will increase coverage by 1.39%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4814      +/-   ##
==========================================
+ Coverage   86.92%   88.32%   +1.39%     
==========================================
  Files         119      119              
  Lines        9020     9026       +6     
==========================================
+ Hits         7841     7972     +131     
+ Misses       1179     1054     -125
Flag Coverage Δ
jina 88.32% <100.00%> (+1.47%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
jina/__init__.py 65.06% <ø> (ø)
jina/serve/runtimes/gateway/http/__init__.py 100.00% <100.00%> (+2.22%) ⬆️
jina/serve/networking.py 89.14% <0.00%> (+0.25%) ⬆️
jina/jaml/__init__.py 94.79% <0.00%> (+0.37%) ⬆️
jina/orchestrate/deployments/config/k8s.py 100.00% <0.00%> (+0.64%) ⬆️
jina/helper.py 81.32% <0.00%> (+0.75%) ⬆️
jina/enums.py 88.07% <0.00%> (+0.91%) ⬆️
jina/clients/mixin.py 93.18% <0.00%> (+1.13%) ⬆️
jina/clients/base/http.py 95.31% <0.00%> (+1.56%) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ec2655...04736ca. Read the comment docs.

@github-actions
Copy link
github-actions bot commented May 20, 2022 •
edited
Loading

Latency summary

Current PR yields:

  • 🐎🐎🐎🐎 index QPS at 1481, delta to last 2 avg.: +21%
  • 🐎🐎🐎🐎 query QPS at 81, delta to last 2 avg.: +23%
  • 🐢🐢 avg flow time within 1.1463 seconds, delta to last 2 avg.: -12%
  • 🐢🐢 import jina within 0.472 seconds, delta to last 2 avg.: -8%

Breakdown

Version Index QPS Query QPS Avg Flow Time (s) Import Time (s)
current 1481 81 1.1463 0.472
3.4.7 1056 53 1.4458 0.5853
3.4.6 1390 77 1.1727 0.4515

Backed by latency-tracking. Further commits will update this comment.

@deepankarm
Copy link
Contributor Author

We need to add some documentation about this

Where should it be added? I don't see a section for health check.

JoanFM
JoanFM reviewed May 24, 2022
View reviewed changes
Copy link
Contributor
@JoanFM JoanFM left a comment

Choose a reason for hiding this comment

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

Add this environment variable to the list of environment variables, and then we will document in the ticket about documenting env vars

@JoanFM
Copy link
Contributor
JoanFM commented May 24, 2022

@deepankarm once the env var is added to the list in jina/__init__.py this env var can be considered in #4823

@deepankarm deepankarm requested a review from hanxiao as a code owner May 24, 2022 11:20
@github-actions github-actions bot added area/entrypoint This issue/PR affects the entrypoint codebase area/helper This issue/PR affects the helper functionality labels May 24, 2022
@deepankarm deepankarm closed this May 26, 2022
@deepankarm deepankarm reopened this May 26, 2022
@JoanFM JoanFM merged commit ed626d9 into master May 26, 2022
@JoanFM JoanFM deleted the http-healthcheck-logs branch May 26, 2022 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoanFM JoanFM JoanFM approved these changes

@hanxiao hanxiao Awaiting requested review from hanxiao hanxiao is a code owner

Assignees
No one assigned
Labels
area/core This issue/PR affects the core codebase area/entrypoint This issue/PR affects the entrypoint codebase area/helper This issue/PR affects the helper functionality area/testing This issue/PR affects testing size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@deepankarm @JoanFM
0