Skip to content
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.

Sign up for GitHub

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

Ensure all usages of StatusCategory is via util #1694

Merged
merged 2 commits into from
Nov 3, 2023
+25 −14
Merged

Ensure all usages of StatusCategory is via util #1694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ebaa9c
Ensure all usages of StatusCategory is via util
gavinbunney Nov 3, 2023
0f2c2f3
Added clearStatus/OriginStatus methods
gavinbunney Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion zuul-core/src/main/java/com/netflix/zuul/context/CommonContextKeys.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
*/
public class CommonContextKeys {

public static final SessionContext.Key<StatusCategory> STATUS_CATGEORY = SessionContext.newKey("status_category");
public static final SessionContext.Key<StatusCategory> STATUS_CATEGORY = SessionContext.newKey("status_category");
public static final SessionContext.Key<StatusCategory> ORIGIN_STATUS_CATEGORY =
SessionContext.newKey("origin_status_category");
public static final SessionContext.Key<Integer> ORIGIN_STATUS = SessionContext.newKey("origin_status");
Expand Down
4 changes: 2 additions & 2 deletions zuul-core/src/main/java/com/netflix/zuul/filters/endpoint/ProxyEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions zuul-core/src/main/java/com/netflix/zuul/origins/BasicNettyOrigin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down
20 changes: 16 additions & 4 deletions zuul-core/src/main/java/com/netflix/zuul/stats/status/StatusCategoryUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,30 @@ 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);
}

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);
}

public static void setOriginStatusCategory(SessionContext ctx, StatusCategory statusCategory) {
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) {
Expand All @@ -67,9 +79,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);
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions zuul-core/src/test/java/com/netflix/zuul/netty/server/ClientResponseWriterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down