-
Notifications
You must be signed in to change notification settings - Fork 7
fix: missing headers for http response logs [WPB-21930] #3795
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
Conversation
|
|
| Branch | fix/http-logs-response-headers |
| Testbed | ubuntu-latest |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | Latency | microseconds (µs) |
|---|---|---|
| com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInFiles | 📈 view plot | 683.86 µs |
| com.wire.kalium.benchmarks.logic.CoreLogicBenchmark.createObjectInMemory | 📈 view plot | 480,601.73 µs |
| com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.messageInsertionBenchmark | 📈 view plot | 1,349,091.97 µs |
| com.wire.kalium.benchmarks.persistence.MessagesNoPragmaTuneBenchmark.queryMessagesBenchmark | 📈 view plot | 21,125.19 µs |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3795 +/- ##
===========================================
+ Coverage 60.04% 60.05% +0.01%
===========================================
Files 1823 1823
Lines 58046 58033 -13
Branches 6318 6313 -5
===========================================
- Hits 34852 34851 -1
+ Misses 20325 20312 -13
- Partials 2869 2870 +1
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|



PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Issues
There are no headers in the http responses logs even if the headers are allowed for the selected log level.
Causes (Optional)
The expression
whenwas used unnecessarily, causing log types to be mutually exclusive when they shouldn't. There are log levels where multiple log types can be enabled - for example,LogLevel. ALL, which has all three types enabled:info,headers, andbodyorLogLevel.HEADERSwhich has bothinfoandheadersenabled, but by usingwhen, it was only possible to display logs of one type, so eitherinfoorheadersorbody, but it was not possible to combine them unless the types had the same code duplicated.Solutions
Use
ifinstead ofwhento determine whether given log type can be used or not and apply corresponding logs if the given log type is available. This fixes the issue for missing headers in the response logs in case of a log level that has bothinfoandheadersenabled. Thanks to that, it's also possible to simplify the code for request logs and remove duplicated code applied to all three log types.Testing
How to Test
Have logging enabled in app (so the log level is
ALL) and check logs for the http responses - should contain headers just like requests logs.PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.