You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/06/02 19:42:40 UTC

[GitHub] [trafficcontrol] rawlinp commented on a diff in pull request #6881: Default routeID to 0, if it is not present in the request context

rawlinp commented on code in PR #6881:
URL: https://github.com/apache/trafficcontrol/pull/6881#discussion_r888353554


##########
traffic_ops/traffic_ops_golang/routing/middleware/wrappers.go:
##########
@@ -212,7 +212,11 @@ func WrapAccessLog(secret string, h http.Handler) http.HandlerFunc {
 					imsType = IMSMISS
 				}
 			}
-			log.EventfRaw(`%s - %s [%s] "%v %v?%v %s" %v %v %v "%v" %v %s`, r.RemoteAddr, user, time.Now().Format(AccessLogTimeFormat), r.Method, r.URL.Path, r.URL.RawQuery, r.Proto, iw.Code, iw.ByteCount, int(time.Now().Sub(start)/time.Millisecond), r.UserAgent(), r.Context().Value(RouteID), imsType)
+			routeID := r.Context().Value(RouteID)

Review Comment:
   Can we safely type-assert this to an int instead of checking nil? It looks strange because routeID looks like a pointer, yet we're assigning an int to it.
   ```
   routeID, _ := r.Context().Value(RouteID).(int)
   log.EventfRaw(...)
   ```
   if it was nil, this would make routeID the zero-value of type int, which is 0. If non-nil, we have a real routeID int. We could also change the log format from `%v` to `%d` for the route ID



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org