Revert "Report metrics on athenz client errors"

10 files changed, 41 insertions, 97 deletions

diff --git a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ConfigserverSslContextFactoryProvider.java b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ConfigserverSslContextFactoryProvider.java
index 880646e37e5..ae4a5933ac2 100644
--- a/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ConfigserverSslContextFactoryProvider.java
+++ b/athenz-identity-provider-service/src/main/java/com/yahoo/vespa/hosted/athenz/instanceproviderservice/ConfigserverSslContextFactoryProvider.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.athenz.instanceproviderservice;
 
 import com.google.inject.Inject;
 import com.yahoo.jdisc.http.ssl.impl.TlsContextBasedProvider;
+import java.util.logging.Level;
 import com.yahoo.security.KeyStoreBuilder;
 import com.yahoo.security.KeyStoreType;
 import com.yahoo.security.KeyUtils;
@@ -36,7 +37,6 @@ import java.util.UUID;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -66,8 +66,7 @@ public class ConfigserverSslContextFactoryProvider extends TlsContextBasedProvid
                                                  KeyProvider keyProvider,
                                                  AthenzProviderServiceConfig config) {
         this.athenzProviderServiceConfig = config;
-        this.ztsClient = new DefaultZtsClient.Builder(URI.create(athenzProviderServiceConfig.ztsUrl()))
-                .withIdentityProvider(bootstrapIdentity).build();
+        this.ztsClient = new DefaultZtsClient(URI.create(athenzProviderServiceConfig.ztsUrl()), bootstrapIdentity);
         this.keyProvider = keyProvider;
         this.configserverIdentity = new AthenzService(athenzProviderServiceConfig.domain(), athenzProviderServiceConfig.serviceName());
 
diff --git a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiTest.java b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiTest.java
index bf2a6719842..343a9feeed6 100644
--- a/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiTest.java
+++ b/athenz-identity-provider-service/src/test/java/com/yahoo/vespa/hosted/ca/restapi/CertificateAuthorityApiTest.java
@@ -8,11 +8,11 @@ import com.yahoo.security.KeyUtils;
 import com.yahoo.security.Pkcs10Csr;
 import com.yahoo.security.Pkcs10CsrUtils;
 import com.yahoo.security.X509CertificateUtils;
-import com.yahoo.slime.SlimeUtils;
 import com.yahoo.text.StringUtilities;
 import com.yahoo.vespa.athenz.api.AthenzPrincipal;
 import com.yahoo.vespa.athenz.api.AthenzService;
 import com.yahoo.vespa.athenz.client.zts.DefaultZtsClient;
+import com.yahoo.slime.SlimeUtils;
 import com.yahoo.vespa.hosted.ca.CertificateTester;
 import org.apache.http.client.ResponseHandler;
 import org.apache.http.client.methods.HttpUriRequest;
@@ -224,7 +224,7 @@ public class CertificateAuthorityApiTest extends ContainerTester {
         private final X509Certificate certificate;
 
         public TestZtsClient(Principal principal, X509Certificate certificate, URI ztsUrl, SSLContext sslContext) {
-            super(ztsUrl, () -> sslContext, null, ErrorHandler.empty());
+            super(ztsUrl, sslContext);
             this.principal = principal;
             this.certificate = certificate;
         }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzClientFactoryImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzClientFactoryImpl.java
index 323d49e4639..173729c7472 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzClientFactoryImpl.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzClientFactoryImpl.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.hosted.controller.athenz.impl;
 
 import com.google.inject.Inject;
-import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.athenz.api.AthenzIdentity;
 import com.yahoo.vespa.athenz.client.zms.DefaultZmsClient;
 import com.yahoo.vespa.athenz.client.zms.ZmsClient;
@@ -11,32 +10,21 @@ import com.yahoo.vespa.athenz.client.zts.ZtsClient;
 import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
 import com.yahoo.vespa.hosted.controller.athenz.config.AthenzConfig;
-import org.apache.http.client.methods.HttpUriRequest;
 
 import java.net.URI;
-import java.util.HashMap;
-import java.util.Map;
 
 /**
  * @author bjorncs
  */
 public class AthenzClientFactoryImpl implements AthenzClientFactory {
 
-    private static final String METRIC_NAME = "athenz.request.error";
-    private static final String ATHENZ_SERVICE_DIMENSION = "athenzService";
-    private static final String EXCEPTION_DIMENSION = "exception";
-
     private final AthenzConfig config;
     private final ServiceIdentityProvider identityProvider;
-    private final Metric metrics;
-    private final Map<String, Metric.Context> metricContexts;
 
     @Inject
-    public AthenzClientFactoryImpl(ServiceIdentityProvider identityProvider, AthenzConfig config, Metric metrics) {
+    public AthenzClientFactoryImpl(ServiceIdentityProvider identityProvider, AthenzConfig config) {
         this.identityProvider = identityProvider;
         this.config = config;
-        this.metrics = metrics;
-        this.metricContexts = new HashMap<>();
     }
 
     @Override
@@ -49,7 +37,7 @@ public class AthenzClientFactoryImpl implements AthenzClientFactory {
      */
     @Override
     public ZmsClient createZmsClient() {
-        return new DefaultZmsClient(URI.create(config.zmsUrl()), identityProvider, this::reportMetricErrorHandler);
+        return new DefaultZmsClient(URI.create(config.zmsUrl()), identityProvider);
     }
 
     /**
@@ -57,7 +45,7 @@ public class AthenzClientFactoryImpl implements AthenzClientFactory {
      */
     @Override
     public ZtsClient createZtsClient() {
-        return new DefaultZtsClient.Builder(URI.create(config.ztsUrl())).withIdentityProvider(identityProvider).build();
+        return new DefaultZtsClient(URI.create(config.ztsUrl()), identityProvider);
     }
 
     @Override
@@ -65,11 +53,4 @@ public class AthenzClientFactoryImpl implements AthenzClientFactory {
         return true;
     }
 
-    private void reportMetricErrorHandler(HttpUriRequest request, Exception error) {
-        String hostname = request.getURI().getHost();
-        Metric.Context context = metricContexts.computeIfAbsent(hostname, host -> metrics.createContext(
-                Map.of(ATHENZ_SERVICE_DIMENSION, host,
-                        EXCEPTION_DIMENSION, error.getClass().getSimpleName())));
-        metrics.add(METRIC_NAME, 1, context);
-    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java
index 0a29da57220..d6c08a820cd 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java
@@ -198,7 +198,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer {
         HostnameVerifier ztsHostNameVerifier = useInternalZts
                 ? new AthenzIdentityVerifier(Set.of(configserverIdentity))
                 : null;
-        try (ZtsClient ztsClient = new DefaultZtsClient.Builder(ztsEndpoint).withIdentityProvider(hostIdentityProvider).withHostnameVerifier(ztsHostNameVerifier).build()) {
+        try (ZtsClient ztsClient = new DefaultZtsClient(ztsEndpoint, hostIdentityProvider, ztsHostNameVerifier)) {
             InstanceIdentity instanceIdentity =
                     ztsClient.registerInstance(
                             configserverIdentity,
@@ -227,7 +227,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer {
             HostnameVerifier ztsHostNameVerifier = useInternalZts
                     ? new AthenzIdentityVerifier(Set.of(configserverIdentity))
                     : null;
-            try (ZtsClient ztsClient = new DefaultZtsClient.Builder(ztsEndpoint).withSslContext(containerIdentitySslContext).withHostnameVerifier(ztsHostNameVerifier).build()) {
+            try (ZtsClient ztsClient = new DefaultZtsClient(ztsEndpoint, containerIdentitySslContext, ztsHostNameVerifier)) {
                 InstanceIdentity instanceIdentity =
                         ztsClient.refreshInstance(
                                 configserverIdentity,
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/aws/AwsCredentials.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/aws/AwsCredentials.java
index 30ff63fb108..b027e7272ea 100644
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/aws/AwsCredentials.java
+++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/aws/AwsCredentials.java
@@ -35,11 +35,11 @@ public class AwsCredentials {
     }
 
     public AwsCredentials(URI ztsUrl, ServiceIdentityProvider identityProvider, AthenzDomain athenzDomain, AwsRole awsRole) {
-        this(new DefaultZtsClient.Builder(ztsUrl).withIdentityProvider(identityProvider).build(), athenzDomain, awsRole);
+        this(new DefaultZtsClient(ztsUrl, identityProvider), athenzDomain, awsRole);
     }
 
     public AwsCredentials(URI ztsUrl, SSLContext sslContext, AthenzDomain athenzDomain, AwsRole awsRole) {
-        this(new DefaultZtsClient.Builder(ztsUrl).withSslContext(sslContext).build(), athenzDomain, awsRole);
+        this(new DefaultZtsClient(ztsUrl, sslContext), athenzDomain, awsRole);
     }
 
     /**
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/common/ClientBase.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/common/ClientBase.java
index 37ef513c786..c1ce45c35da 100644
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/common/ClientBase.java
+++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/common/ClientBase.java
@@ -39,15 +39,12 @@ public abstract class ClientBase implements AutoCloseable {
 
     private final CloseableHttpClient client;
     private final ClientExceptionFactory exceptionFactory;
-    private final ErrorHandler errorHandler;
 
     protected ClientBase(String userAgent,
                          Supplier<SSLContext> sslContextSupplier,
                          ClientExceptionFactory exceptionFactory,
-                         HostnameVerifier hostnameVerifier,
-                         ErrorHandler errorHandler) {
+                         HostnameVerifier hostnameVerifier) {
         this.exceptionFactory = exceptionFactory;
-        this.errorHandler = errorHandler;
         this.client = createHttpClient(userAgent, sslContextSupplier, hostnameVerifier);
     }
 
@@ -55,17 +52,10 @@ public abstract class ClientBase implements AutoCloseable {
         try {
             return client.execute(request, responseHandler);
         } catch (IOException e) {
-            try {
-                reportError(request, e);
-            } catch (Exception _ignored) {}
             throw new UncheckedIOException(e);
         }
     }
 
-    private void reportError(HttpUriRequest request, Exception e) {
-        errorHandler.reportError(request, e);
-    }
-
     protected StringEntity toJsonStringEntity(Object entity) {
         try {
             return new StringEntity(objectMapper.writeValueAsString(entity), ContentType.APPLICATION_JSON);
@@ -124,11 +114,4 @@ public abstract class ClientBase implements AutoCloseable {
     protected interface ClientExceptionFactory {
         RuntimeException createException(int errorCode, String description);
     }
-
-    public interface ErrorHandler {
-        static ErrorHandler empty() {
-            return (r,e)->{};
-        }
-        void reportError(HttpUriRequest request, Exception error);
-    }
 }
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java
index 3742996c274..33cb6d7d5d4 100644
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java
+++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zms/DefaultZmsClient.java
@@ -25,6 +25,7 @@ import javax.net.ssl.SSLContext;
 import java.net.URI;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.OptionalInt;
 import java.util.Set;
 import java.util.function.Supplier;
@@ -39,16 +40,16 @@ public class DefaultZmsClient extends ClientBase implements ZmsClient {
     private final URI zmsUrl;
     private final AthenzIdentity identity;
 
-    public DefaultZmsClient(URI zmsUrl, AthenzIdentity identity, SSLContext sslContext, ErrorHandler errorHandler) {
-        this(zmsUrl, identity, () -> sslContext, errorHandler);
+    public DefaultZmsClient(URI zmsUrl, AthenzIdentity identity, SSLContext sslContext) {
+        this(zmsUrl, identity, () -> sslContext);
     }
 
-    public DefaultZmsClient(URI zmsUrl, ServiceIdentityProvider identityProvider, ErrorHandler errorHandler) {
-        this(zmsUrl, identityProvider.identity(), identityProvider::getIdentitySslContext, errorHandler);
+    public DefaultZmsClient(URI zmsUrl, ServiceIdentityProvider identityProvider) {
+        this(zmsUrl, identityProvider.identity(), identityProvider::getIdentitySslContext);
     }
 
-    private DefaultZmsClient(URI zmsUrl, AthenzIdentity identity, Supplier<SSLContext> sslContextSupplier, ErrorHandler errorHandler) {
-        super("vespa-zms-client", sslContextSupplier, ZmsClientException::new, null, errorHandler);
+    private DefaultZmsClient(URI zmsUrl, AthenzIdentity identity, Supplier<SSLContext> sslContextSupplier) {
+        super("vespa-zms-client", sslContextSupplier, ZmsClientException::new, null);
         this.zmsUrl = addTrailingSlash(zmsUrl);
         this.identity = identity;
     }
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java
index 28119dc1f5a..c05213c8008 100644
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java
+++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java
@@ -37,7 +37,6 @@ import java.security.KeyPair;
 import java.security.cert.X509Certificate;
 import java.time.Duration;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
@@ -53,8 +52,25 @@ import static java.util.stream.Collectors.toList;
 public class DefaultZtsClient extends ClientBase implements ZtsClient {
 
     private final URI ztsUrl;
-    protected DefaultZtsClient(URI ztsUrl, Supplier<SSLContext> sslContextSupplier, HostnameVerifier hostnameVerifier, ErrorHandler errorHandler) {
-        super("vespa-zts-client", sslContextSupplier, ZtsClientException::new, hostnameVerifier, errorHandler);
+
+    public DefaultZtsClient(URI ztsUrl, SSLContext sslContext) {
+        this(ztsUrl, sslContext, null);
+    }
+
+    public DefaultZtsClient(URI ztsUrl, SSLContext sslContext, HostnameVerifier hostnameVerifier) {
+        this(ztsUrl, () -> sslContext, hostnameVerifier);
+    }
+
+    public DefaultZtsClient(URI ztsUrl, ServiceIdentityProvider identityProvider) {
+        this(ztsUrl, identityProvider::getIdentitySslContext, null);
+    }
+
+    public DefaultZtsClient(URI ztsUrl, ServiceIdentityProvider identityProvider, HostnameVerifier hostnameVerifier) {
+        this(ztsUrl, identityProvider::getIdentitySslContext, hostnameVerifier);
+    }
+
+    private DefaultZtsClient(URI ztsUrl, Supplier<SSLContext> sslContextSupplier, HostnameVerifier hostnameVerifier) {
+        super("vespa-zts-client", sslContextSupplier, ZtsClientException::new, hostnameVerifier);
         this.ztsUrl = addTrailingSlash(ztsUrl);
     }
 
@@ -223,41 +239,5 @@ public class DefaultZtsClient extends ClientBase implements ZtsClient {
         else
             return URI.create(ztsUrl.toString() + '/');
     }
-    public static class Builder {
-        private URI ztsUrl;
-        private ClientBase.ErrorHandler errorHandler = ErrorHandler.empty();
-        private HostnameVerifier hostnameVerifier = null;
-        private Supplier<SSLContext> sslContextSupplier = null;
-
-        public Builder(URI ztsUrl) {
-            this.ztsUrl = ztsUrl;
-        }
 
-        public Builder withErrorHandler(ClientBase.ErrorHandler errorHandler) {
-            this.errorHandler = errorHandler;
-            return this;
-        }
-
-        public Builder withHostnameVerifier(HostnameVerifier hostnameVerifier) {
-            this.hostnameVerifier = hostnameVerifier;
-            return this;
-        }
-
-        public Builder withSslContext(SSLContext sslContext) {
-            this.sslContextSupplier = () -> sslContext;
-            return this;
-        }
-
-        public Builder withIdentityProvider(ServiceIdentityProvider identityProvider) {
-            this.sslContextSupplier = identityProvider::getIdentitySslContext;
-            return this;
-        }
-
-        public DefaultZtsClient build() {
-            if (Objects.isNull(sslContextSupplier)) {
-                throw new IllegalArgumentException("No ssl context or identity provider available to set up zts client");
-            }
-            return new DefaultZtsClient(ztsUrl, sslContextSupplier, hostnameVerifier, errorHandler);
-        }
-    }
 }
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java
index 612f9caa691..8e029906c30 100644
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java
+++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java
@@ -81,7 +81,7 @@ class AthenzCredentialsService {
                 document.ipAddresses(),
                 keyPair);
 
-        try (ZtsClient ztsClient = new DefaultZtsClient.Builder(ztsEndpoint).withIdentityProvider(nodeIdentityProvider).build()) {
+        try (ZtsClient ztsClient = new DefaultZtsClient(ztsEndpoint, nodeIdentityProvider)) {
             InstanceIdentity instanceIdentity =
                     ztsClient.registerInstance(
                             configserverIdentity,
@@ -102,7 +102,7 @@ class AthenzCredentialsService {
                 document.ipAddresses(),
                 newKeyPair);
 
-        try (ZtsClient ztsClient = new DefaultZtsClient.Builder(ztsEndpoint).withSslContext(sslContext).build()) {
+        try (ZtsClient ztsClient = new DefaultZtsClient(ztsEndpoint, sslContext)) {
             InstanceIdentity instanceIdentity =
                     ztsClient.refreshInstance(
                             configserverIdentity,
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java
index 724a3059f6d..65574d7583e 100644
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java
+++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java
@@ -301,7 +301,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen
     }
 
     private DefaultZtsClient createZtsClient() {
-        return new DefaultZtsClient.Builder(ztsEndpoint).withSslContext(getIdentitySslContext()).build();
+        return new DefaultZtsClient(ztsEndpoint, getIdentitySslContext());
     }
 
     @Override