Skip to content

[improve][schema] Do not print error log with stacktrace for 404 - #19130

Merged
codelipenghui merged 1 commit into
apache:masterfrom
codelipenghui:penghui/schema-log
Jan 5, 2023
Merged

[improve][schema] Do not print error log with stacktrace for 404#19130
codelipenghui merged 1 commit into
apache:masterfrom
codelipenghui:penghui/schema-log

Conversation

@codelipenghui

Copy link
Copy Markdown
Contributor

Motivation

We have the INFO log to show the schema not found, or the topic is found.
The error logs for the not found exception are just making noise.

2023-01-04T20:33:26,341 - INFO  - [metadata-store-12-1:Slf4jRequestLogWriter@62] - 127.0.0.1 - - [04/Jan/2023:20:33:26 +0800] "GET /admin/v2/schemas/public/default/test111/schema HTTP/1.1" 404 29 "-" "Pulsar-Java-v2.12.0-SNAPSHOT" 118

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@codelipenghui codelipenghui self-assigned this Jan 4, 2023
@codelipenghui codelipenghui added this to the 2.12.0 milestone Jan 4, 2023
@codelipenghui codelipenghui added release/2.11.1 release/2.9.5 release/2.10.4 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/broker labels Jan 4, 2023
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Jan 4, 2023
@codelipenghui

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui changed the title [improve][broker] Do not print error log with stacktrace for 404 [improve][schema] Do not print error log with stacktrace for 404 Jan 4, 2023

@nicoloboschi nicoloboschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about keeping them a debug level ?

@codecov-commenter

codecov-commenter commented Jan 4, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 32.00000% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.68%. Comparing base (9ec1d07) to head (537b010).
⚠️ Report is 3036 commits behind head on master.

Files with missing lines Patch % Lines
...apache/pulsar/broker/admin/v1/SchemasResource.java 10.00% 9 Missing ⚠️
...apache/pulsar/broker/admin/v2/SchemasResource.java 40.00% 5 Missing and 1 partial ⚠️
.../org/apache/pulsar/broker/admin/AdminResource.java 75.00% 0 Missing and 1 partial ⚠️
.../pulsar/broker/admin/impl/SchemasResourceBase.java 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19130      +/-   ##
============================================
- Coverage     47.73%   45.68%   -2.05%     
- Complexity    10819    11046     +227     
============================================
  Files           712      772      +60     
  Lines         69645    74422    +4777     
  Branches       7481     8015     +534     
============================================
+ Hits          33242    33997     +755     
- Misses        32699    36615    +3916     
- Partials       3704     3810     +106     
Flag Coverage Δ
unittests 45.68% <32.00%> (-2.05%) ⬇️

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

Files with missing lines Coverage Δ
.../org/apache/pulsar/broker/admin/AdminResource.java 66.21% <75.00%> (-0.38%) ⬇️
.../pulsar/broker/admin/impl/SchemasResourceBase.java 89.21% <0.00%> (-0.89%) ⬇️
...apache/pulsar/broker/admin/v2/SchemasResource.java 72.28% <40.00%> (ø)
...apache/pulsar/broker/admin/v1/SchemasResource.java 26.50% <10.00%> (ø)

... and 97 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codelipenghui

Copy link
Copy Markdown
Contributor Author

What about keeping them a debug level ?

Hmmm, I don't think it will help something. Because 404 always means the schema is not found or the topic is not found depending on the API. We already have the streamlined log which I mentioned in the PR's description.

@codelipenghui
codelipenghui merged commit 3afc291 into apache:master Jan 5, 2023
@codelipenghui
codelipenghui deleted the penghui/schema-log branch January 5, 2023 02:12
codelipenghui added a commit that referenced this pull request Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker cherry-picked/branch-2.11 doc-not-needed Your PR changes do not impact docs ready-to-test release/2.11.1 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants