ipc: disable metrics if proxyd header is present (#1085)

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.

spectator-ext-ipc/src/main/java/com/netflix/spectator/ipc/IpcLogEntry.java

@@ -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.
@@ -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();
@@ -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);
@@ -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;
   }
 
@@ -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;
   }
 
@@ -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());
@@ -707,6 +718,10 @@ private Id getInflightId() {
   }
 
   private void recordClientMetrics() {
+    if (disableMetrics) {
+      return;
+    }
+
     Id clientCall = createCallId(IpcMetric.clientCall.metricName());
     PercentileTimer.builder(registry)
         .withId(clientCall)
@@ -727,6 +742,10 @@ private void recordClientMetrics() {
   }
 
   private void recordServerMetrics() {
+    if (disableMetrics) {
+      return;
+    }
+
     Id serverCall = createCallId(IpcMetric.serverCall.metricName());
     PercentileTimer.builder(registry)
         .withId(serverCall)

spectator-ext-ipc/src/main/java/com/netflix/spectator/ipc/NetflixHeader.java

@@ -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.
@@ -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() {

spectator-ext-ipc/src/test/java/com/netflix/spectator/ipc/IpcLogEntryTest.java

@@ -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.
@@ -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();
@@ -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();