From 4ebaa9cad1396754913359725d445f3532de65cc Mon Sep 17 00:00:00 2001 From: Gavin Bunney Date: Fri, 3 Nov 2023 10:56:08 -0700 Subject: [PATCH 1/2] Ensure all usages of StatusCategory is via util --- .../com/netflix/zuul/context/CommonContextKeys.java | 2 +- .../netflix/zuul/filters/endpoint/ProxyEndpoint.java | 4 ++-- .../com/netflix/zuul/origins/BasicNettyOrigin.java | 6 +++--- .../zuul/stats/status/StatusCategoryUtils.java | 12 ++++++++---- .../zuul/netty/server/ClientResponseWriterTest.java | 7 +++---- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/zuul-core/src/main/java/com/netflix/zuul/context/CommonContextKeys.java b/zuul-core/src/main/java/com/netflix/zuul/context/CommonContextKeys.java index a15a49a164..473b5ebafe 100644 --- a/zuul-core/src/main/java/com/netflix/zuul/context/CommonContextKeys.java +++ b/zuul-core/src/main/java/com/netflix/zuul/context/CommonContextKeys.java @@ -38,7 +38,7 @@ */ public class CommonContextKeys { - public static final SessionContext.Key STATUS_CATGEORY = SessionContext.newKey("status_category"); + public static final SessionContext.Key STATUS_CATEGORY = SessionContext.newKey("status_category"); public static final SessionContext.Key ORIGIN_STATUS_CATEGORY = SessionContext.newKey("origin_status_category"); public static final SessionContext.Key ORIGIN_STATUS = SessionContext.newKey("origin_status"); diff --git a/zuul-core/src/main/java/com/netflix/zuul/filters/endpoint/ProxyEndpoint.java b/zuul-core/src/main/java/com/netflix/zuul/filters/endpoint/ProxyEndpoint.java index 8fd36875f0..ea25d76f74 100644 --- a/zuul-core/src/main/java/com/netflix/zuul/filters/endpoint/ProxyEndpoint.java +++ b/zuul-core/src/main/java/com/netflix/zuul/filters/endpoint/ProxyEndpoint.java @@ -855,7 +855,7 @@ private HttpResponseMessage buildZuulHttpResponse( // Collect some info about the received response. origin.recordFinalResponse(zuulResponse); origin.recordFinalError(zuulRequest, ex); - zuulCtx.put(CommonContextKeys.STATUS_CATGEORY, statusCategory); + StatusCategoryUtils.setStatusCategory(zuulCtx, statusCategory); zuulCtx.setError(ex); zuulCtx.put("origin_http_status", Integer.toString(respStatus)); @@ -1129,7 +1129,7 @@ private void verifyOrigin( SessionContext context, HttpRequestMessage request, String restClientName, Origin primaryOrigin) { if (primaryOrigin == null) { // If no origin found then add specific error-cause metric tag, and throw an exception with 404 status. - context.put(CommonContextKeys.STATUS_CATGEORY, SUCCESS_LOCAL_NO_ROUTE); + StatusCategoryUtils.setStatusCategory(context, SUCCESS_LOCAL_NO_ROUTE); String causeName = "RESTCLIENT_NOTFOUND"; originNotFound(context, causeName); ZuulException ze = new ZuulException( diff --git a/zuul-core/src/main/java/com/netflix/zuul/origins/BasicNettyOrigin.java b/zuul-core/src/main/java/com/netflix/zuul/origins/BasicNettyOrigin.java index b8f4f67c01..8589c08b4e 100644 --- a/zuul-core/src/main/java/com/netflix/zuul/origins/BasicNettyOrigin.java +++ b/zuul-core/src/main/java/com/netflix/zuul/origins/BasicNettyOrigin.java @@ -163,8 +163,8 @@ public void recordFinalError(HttpRequestMessage requestMsg, Throwable throwable) // Choose StatusCategory based on the ErrorType. final ErrorType et = requestAttemptFactory.mapNettyToOutboundErrorType(throwable); final StatusCategory nfs = et.getStatusCategory(); - zuulCtx.put(CommonContextKeys.STATUS_CATGEORY, nfs); - zuulCtx.put(CommonContextKeys.ORIGIN_STATUS_CATEGORY, nfs); + StatusCategoryUtils.setStatusCategory(zuulCtx, nfs); + StatusCategoryUtils.setOriginStatusCategory(zuulCtx, nfs); zuulCtx.setError(throwable); } @@ -185,7 +185,7 @@ public void recordFinalResponse(HttpResponseMessage resp) { } else if (StatusCategoryUtils.isResponseHttpErrorStatus(originStatusCode)) { originNfs = FAILURE_ORIGIN; } - zuulCtx.put(CommonContextKeys.ORIGIN_STATUS_CATEGORY, originNfs); + StatusCategoryUtils.setOriginStatusCategory(zuulCtx, originNfs); // Choose the zuul StatusCategory based on the origin one... // ... but only if existing one has not already been set to a non-success value. StatusCategoryUtils.storeStatusCategoryIfNotAlreadyFailure(zuulCtx, originNfs); diff --git a/zuul-core/src/main/java/com/netflix/zuul/stats/status/StatusCategoryUtils.java b/zuul-core/src/main/java/com/netflix/zuul/stats/status/StatusCategoryUtils.java index 1f7b4f2fba..67fb51b4b3 100644 --- a/zuul-core/src/main/java/com/netflix/zuul/stats/status/StatusCategoryUtils.java +++ b/zuul-core/src/main/java/com/netflix/zuul/stats/status/StatusCategoryUtils.java @@ -39,11 +39,11 @@ public static StatusCategory getStatusCategory(ZuulMessage msg) { @Nullable public static StatusCategory getStatusCategory(SessionContext ctx) { - return ctx.get(CommonContextKeys.STATUS_CATGEORY); + return ctx.get(CommonContextKeys.STATUS_CATEGORY); } public static void setStatusCategory(SessionContext ctx, StatusCategory statusCategory) { - ctx.put(CommonContextKeys.STATUS_CATGEORY, statusCategory); + ctx.put(CommonContextKeys.STATUS_CATEGORY, statusCategory); } @Nullable @@ -51,6 +51,10 @@ public static StatusCategory getOriginStatusCategory(SessionContext ctx) { return ctx.get(CommonContextKeys.ORIGIN_STATUS_CATEGORY); } + public static void setOriginStatusCategory(SessionContext ctx, StatusCategory statusCategory) { + ctx.put(CommonContextKeys.ORIGIN_STATUS_CATEGORY, statusCategory); + } + public static boolean isResponseHttpErrorStatus(HttpResponseMessage response) { boolean isHttpError = false; if (response != null) { @@ -67,9 +71,9 @@ public static boolean isResponseHttpErrorStatus(int status) { public static void storeStatusCategoryIfNotAlreadyFailure( final SessionContext context, final StatusCategory statusCategory) { if (statusCategory != null) { - final StatusCategory nfs = context.get(CommonContextKeys.STATUS_CATGEORY); + final StatusCategory nfs = getStatusCategory(context); if (nfs == null || nfs.getGroup().getId() == ZuulStatusCategoryGroup.SUCCESS.getId()) { - context.put(CommonContextKeys.STATUS_CATGEORY, statusCategory); + setStatusCategory(context, statusCategory); } } } diff --git a/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java b/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java index 0087d400f8..bdd4398073 100644 --- a/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java +++ b/zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java @@ -17,7 +17,6 @@ package com.netflix.zuul.netty.server; import com.netflix.zuul.BasicRequestCompleteHandler; -import com.netflix.zuul.context.CommonContextKeys; import com.netflix.zuul.context.SessionContext; import com.netflix.zuul.message.http.HttpRequestMessage; import com.netflix.zuul.message.util.HttpRequestBuilder; @@ -41,7 +40,7 @@ void exemptClientTimeoutResponseBeforeRequestRead() { final EmbeddedChannel channel = new EmbeddedChannel(); final SessionContext context = new SessionContext(); - context.put(CommonContextKeys.STATUS_CATGEORY, ZuulStatusCategory.FAILURE_CLIENT_TIMEOUT); + StatusCategoryUtils.setStatusCategory(context, ZuulStatusCategory.FAILURE_CLIENT_TIMEOUT); final HttpRequestMessage request = new HttpRequestBuilder(context).withDefaults(); channel.attr(ClientRequestReceiver.ATTR_ZUUL_REQ).set(request); @@ -54,7 +53,7 @@ void flagResponseBeforeRequestRead() { final EmbeddedChannel channel = new EmbeddedChannel(); final SessionContext context = new SessionContext(); - context.put(CommonContextKeys.STATUS_CATGEORY, ZuulStatusCategory.FAILURE_LOCAL); + StatusCategoryUtils.setStatusCategory(context, ZuulStatusCategory.FAILURE_LOCAL); final HttpRequestMessage request = new HttpRequestBuilder(context).withDefaults(); channel.attr(ClientRequestReceiver.ATTR_ZUUL_REQ).set(request); @@ -76,7 +75,7 @@ protected boolean shouldAllowPreemptiveResponse(Channel channel) { final EmbeddedChannel channel = new EmbeddedChannel(); final SessionContext context = new SessionContext(); - context.put(CommonContextKeys.STATUS_CATGEORY, customStatus); + StatusCategoryUtils.setStatusCategory(context, customStatus); final HttpRequestMessage request = new HttpRequestBuilder(context).withDefaults(); channel.attr(ClientRequestReceiver.ATTR_ZUUL_REQ).set(request); From 0f2c2f372a08c402b35ec4303da24a99b9acf451 Mon Sep 17 00:00:00 2001 From: Gavin Bunney Date: Fri, 3 Nov 2023 11:39:41 -0700 Subject: [PATCH 2/2] Added clearStatus/OriginStatus methods --- .../netflix/zuul/stats/status/StatusCategoryUtils.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/zuul-core/src/main/java/com/netflix/zuul/stats/status/StatusCategoryUtils.java b/zuul-core/src/main/java/com/netflix/zuul/stats/status/StatusCategoryUtils.java index 67fb51b4b3..493787aad7 100644 --- a/zuul-core/src/main/java/com/netflix/zuul/stats/status/StatusCategoryUtils.java +++ b/zuul-core/src/main/java/com/netflix/zuul/stats/status/StatusCategoryUtils.java @@ -46,6 +46,10 @@ public static void setStatusCategory(SessionContext ctx, StatusCategory statusCa ctx.put(CommonContextKeys.STATUS_CATEGORY, statusCategory); } + public static void clearStatusCategory(SessionContext ctx) { + ctx.remove(CommonContextKeys.STATUS_CATEGORY); + } + @Nullable public static StatusCategory getOriginStatusCategory(SessionContext ctx) { return ctx.get(CommonContextKeys.ORIGIN_STATUS_CATEGORY); @@ -55,6 +59,10 @@ public static void setOriginStatusCategory(SessionContext ctx, StatusCategory st ctx.put(CommonContextKeys.ORIGIN_STATUS_CATEGORY, statusCategory); } + public static void clearOriginStatusCategory(SessionContext ctx) { + ctx.remove(CommonContextKeys.ORIGIN_STATUS_CATEGORY); + } + public static boolean isResponseHttpErrorStatus(HttpResponseMessage response) { boolean isHttpError = false; if (response != null) {