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.

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

ipc: disable metrics if proxyd header is present #1085

Merged
merged 2 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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;

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