Skip to content

Commit

Permalink
ipc: disable metrics if proxyd header is present
Browse files Browse the repository at this point in the history
Internally most IPC metrics will start coming from proxyd.
To avoid confusion the backend behind it should not publish
them if they are already provided for a request by the proxy.
  • Loading branch information
brharrington committed Oct 9, 2023
1 parent c042c97 commit c53e991
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2014-2021 Netflix, Inc.
* Copyright 2014-2023 Netflix, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -93,6 +93,8 @@ public final class IpcLogEntry {
private String remoteAddress;
private int remotePort;

private boolean disableMetrics;

private final Map<String, String> additionalTags = new HashMap<>();

private final StringBuilder builder = new StringBuilder();
Expand Down Expand Up @@ -154,7 +156,7 @@ public IpcLogEntry withLatency(long latency, TimeUnit unit) {
* the request completes it is recommended to call {@link #markEnd()}.
*/
public IpcLogEntry markStart() {
if (registry != null) {
if (registry != null && !disableMetrics) {
inflightId = getInflightId();
int n = logger.inflightRequests(inflightId).incrementAndGet();
registry.distributionSummary(inflightId).record(n);
Expand Down Expand Up @@ -526,16 +528,17 @@ public IpcLogEntry withResponseContentLength(long length) {
*/
public IpcLogEntry addRequestHeader(String name, String value) {
if (clientAsg == null && name.equalsIgnoreCase(NetflixHeader.ASG.headerName())) {
withClientAsg(value);
withClientAsg(value);
} else if (clientZone == null && name.equalsIgnoreCase(NetflixHeader.Zone.headerName())) {
withClientZone(value);
} else if (clientNode == null && name.equalsIgnoreCase(NetflixHeader.Node.headerName())) {
withClientNode(value);
} else if (vip == null && name.equalsIgnoreCase(NetflixHeader.Vip.headerName())) {
withVip(value);
} else {
this.requestHeaders.add(new Header(name, value));
} else if (name.equalsIgnoreCase(NetflixHeader.IngressCommonIpcMetrics.headerName())) {
disableMetrics();
}
this.requestHeaders.add(new Header(name, value));
return this;
}

Expand All @@ -552,9 +555,8 @@ public IpcLogEntry addResponseHeader(String name, String value) {
withServerNode(value);
} else if (endpoint == null && name.equalsIgnoreCase(NetflixHeader.Endpoint.headerName())) {
withEndpoint(value);
} else {
this.responseHeaders.add(new Header(name, value));
}
this.responseHeaders.add(new Header(name, value));
return this;
}

Expand Down Expand Up @@ -596,6 +598,15 @@ public IpcLogEntry addTag(String k, String v) {
return this;
}

/**
* Disable the metrics. The log will still get written, but none of the metrics will get
* updated.
*/
public IpcLogEntry disableMetrics() {
this.disableMetrics = true;
return this;
}

private void putTag(Map<String, String> tags, Tag tag) {
if (tag != null) {
tags.put(tag.key(), tag.value());
Expand Down Expand Up @@ -707,6 +718,10 @@ private Id getInflightId() {
}

private void recordClientMetrics() {
if (disableMetrics) {
return;
}

Id clientCall = createCallId(IpcMetric.clientCall.metricName());
PercentileTimer.builder(registry)
.withId(clientCall)
Expand All @@ -727,6 +742,10 @@ private void recordClientMetrics() {
}

private void recordServerMetrics() {
if (disableMetrics) {
return;
}

Id serverCall = createCallId(IpcMetric.serverCall.metricName());
PercentileTimer.builder(registry)
.withId(serverCall)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2014-2019 Netflix, Inc.
* Copyright 2014-2023 Netflix, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -26,36 +26,46 @@ public enum NetflixHeader {
* Server group name for the client or the server. It should follow the naming conventions
* expected by Frigga. See {@link ServerGroup} for more information.
*/
ASG,
ASG("Netflix-ASG"),

/**
* Availability zone of the client or server instance.
*/
Zone,
Zone("Netflix-Zone"),

/**
* Instance id of the client or server.
*/
Node,
Node("Netflix-Node"),

/**
* Route or route handler for a given path. It should have a fixed cardinality. For HTTP
* this would need to come from the server so there is agreement and the client will report
* the same value.
*/
Endpoint,
Endpoint("Netflix-Endpoint"),

/**
* VIP that was used to lookup instances for a service when using a client side load balancer.
* This should be set on the client request to the vip used for the lookup. In the case of NIWS,
* that would be the VIP used for the DeploymentContextBasedVipAddresses. If multiple VIPs are
* used, then the first VIP that caused a given server instance to be selected should be used.
*
* For server side load balancers the VIP header should be omitted.
* <p>For server side load balancers the VIP header should be omitted.
*/
Vip;
Vip("Netflix-Vip"),

private final String headerName = "Netflix-" + name();
/**
* Used to indicate that common IPC metrics are provided by a proxy and do not need to be
* reported locally. Reporting in multiple places can lead to confusing duplication.
*/
IngressCommonIpcMetrics("Netflix-Ingress-Common-IPC-Metrics");

private final String headerName;

private NetflixHeader(String headerName) {
this.headerName = headerName;
}

/** Return the fully qualified header name. */
public String headerName() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2014-2021 Netflix, Inc.
* Copyright 2014-2023 Netflix, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -718,6 +718,21 @@ public void clientMetricsValidateHttpSuccess() {
IpcMetric.validate(registry, true);
}

@Test
public void clientMetricsDisbled() {
Registry registry = new DefaultRegistry();
IpcLogger logger = new IpcLogger(registry, clock, LoggerFactory.getLogger(getClass()));

logger.createClientEntry()
.withOwner("test")
.disableMetrics() // must be before markStart for inflight
.markStart()
.markEnd()
.log();

Assertions.assertEquals(0, registry.stream().count());
}

@Test
public void serverMetricsValidate() {
Registry registry = new DefaultRegistry();
Expand All @@ -732,6 +747,36 @@ public void serverMetricsValidate() {
IpcMetric.validate(registry);
}

@Test
public void serverMetricsDisabled() {
Registry registry = new DefaultRegistry();
IpcLogger logger = new IpcLogger(registry, clock, LoggerFactory.getLogger(getClass()));

logger.createServerEntry()
.withOwner("test")
.disableMetrics()
.markStart()
.markEnd()
.log();

Assertions.assertEquals(0, registry.stream().count());
}

@Test
public void serverMetricsDisabledViaHeader() {
Registry registry = new DefaultRegistry();
IpcLogger logger = new IpcLogger(registry, clock, LoggerFactory.getLogger(getClass()));

logger.createServerEntry()
.withOwner("test")
.addRequestHeader("netflix-ingress-common-ipc-metrics", "true")
.markStart()
.markEnd()
.log();

Assertions.assertEquals(0, registry.stream().count());
}

@Test
public void endpointUnknownIfNotSet() {
Registry registry = new DefaultRegistry();
Expand Down

0 comments on commit c53e991

Please sign in to comment.