Log the resource type associated with a download job #767
Conversation
- starts a pattern for adding fields to the contextual log (the log associated with the request). - using middleware to decorate functions with additional request level logging.
v1 stopped working in feb 2022
|
This pull request introduces 1 alert and fixes 1 when merging 102c94a into 3dbc1ed - view on LGTM.com new alerts:
fixed alerts:
|
whytheplatypus
requested review from
KyRobbins,
4ell0,
ericbuckley and
ian-sawyer
|
This pull request introduces 1 alert and fixes 1 when merging 5d1cc75 into 3dbc1ed - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request fixes 1 alert when merging d31fbfb into 3dbc1ed - view on LGTM.com fixed alerts:
|
Codecov Report
@@ Coverage Diff @@
## master #767 +/- ##
==========================================
+ Coverage 65.94% 66.46% +0.52%
==========================================
Files 65 65
Lines 5875 8126 +2251
==========================================
+ Hits 3874 5401 +1527
- Misses 1540 2262 +722
- Partials 461 463 +2
Continue to review full report at Codecov.
|
|
|
||
| func (rl *ResourceTypeLogger) LogJobResourceType(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| next.ServeHTTP(w, r) |
There was a problem hiding this comment.
Am I reading this correctly that only a 200 response from v1.ServeData will result in the resource type being logged? Not necessarily a bad thing, just wanted to make sure I was interpreting this middleware function correctly.
There was a problem hiding this comment.
This should run regardless, although if ServeData isn't returning a 200 it's likely that the same thing that caused that will mean there's no ResourceType to log (e.g. the job id was wrong, or something)
Fixes PACA-429
Proposed Changes
Change Details
Using the structured contextual logging provided by the chi middleware
Security Implications
None: adding only one non-pii/phi field to logging.
None
Acceptance Validation
Feedback Requested