core, grpclb: change policy selection strategy for Grpclb policy (take two: move logic of querying SRV into Grpclb's own resolver) (#6723)

Eliminated the code path of resolving Grpclb balancer addresses in grpc-core and moved it into GrpclbNameResolver, which is a subclass of DnsNameResolver. Main changes:

- Slightly changed ResourceResolver and its JNDI implementation. ResourceResolver#resolveSrv(String) returns a list of SrvRecord so that it only parse SRV records and does nothing more. It's gRPC's name resolver's logic to use information parsed from SRV records.

- Created a GrpclbNameResolver class that extends DnsNameResolver. Logic of using information from SRV records to set balancer addresses as ResolutionResult attributes is implemented in GrpclbNameResolver only.

- Refactored DnsNameResolver, mainly the resolveAll(...) method. Logics for resolving backend addresses and service config are modularized into resolveAddresses() and resolveServiceConfig() methods respectively. They are shared implementation for subclasses (i.e., GrpclbNameResolver).

api/src/test/java/io/grpc/NameResolverRegistryTest.java

@@ -21,7 +21,6 @@ import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 
 import io.grpc.NameResolver.ServiceConfigParser;
-import io.grpc.internal.BaseDnsNameResolverProvider;
 import io.grpc.internal.DnsNameResolverProvider;
 import java.lang.Thread.UncaughtExceptionHandler;
 import java.net.URI;
@@ -155,10 +154,11 @@ public class NameResolverRegistryTest {
   public void baseProviders() {
     List<NameResolverProvider> providers = NameResolverRegistry.getDefaultRegistry().providers();
     assertThat(providers).hasSize(2);
-    // 2 name resolvers from grpclb and core
+    // 2 name resolvers from grpclb and core, ordered with decreasing priorities.
-    for (NameResolverProvider provider : providers) {
+    assertThat(providers.get(0).getClass().getName())
-      assertThat(provider).isInstanceOf(BaseDnsNameResolverProvider.class);
+        .isEqualTo("io.grpc.grpclb.SecretGrpclbNameResolverProvider$Provider");
-    }
+    assertThat(providers.get(1).getClass().getName())
+        .isEqualTo("io.grpc.internal.DnsNameResolverProvider");
     assertThat(NameResolverRegistry.getDefaultRegistry().asFactory().getDefaultScheme())
         .isEqualTo("dns");
   }

core/src/main/java/io/grpc/internal/BaseDnsNameResolverProvider.java

@@ -1,70 +0,0 @@
-/*
- * Copyright 2019 The gRPC Authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package io.grpc.internal;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
-import com.google.common.base.Stopwatch;
-import io.grpc.InternalServiceProviders;
-import io.grpc.NameResolver;
-import io.grpc.NameResolverProvider;
-import java.net.URI;
-
-/**
- * Base provider of name resolvers for name agnostic consumption.
- */
-public abstract class BaseDnsNameResolverProvider extends NameResolverProvider {
-
-  private static final String SCHEME = "dns";
-
-  @VisibleForTesting
-  public static final String ENABLE_GRPCLB_PROPERTY_NAME =
-      "io.grpc.internal.DnsNameResolverProvider.enable_grpclb";
-
-  /** Returns boolean value of system property {@link #ENABLE_GRPCLB_PROPERTY_NAME}. */
-  protected abstract boolean isSrvEnabled();
-
-  @Override
-  public DnsNameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
-    if (SCHEME.equals(targetUri.getScheme())) {
-      String targetPath = Preconditions.checkNotNull(targetUri.getPath(), "targetPath");
-      Preconditions.checkArgument(targetPath.startsWith("/"),
-          "the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri);
-      String name = targetPath.substring(1);
-      return new DnsNameResolver(
-          targetUri.getAuthority(),
-          name,
-          args,
-          GrpcUtil.SHARED_CHANNEL_EXECUTOR,
-          Stopwatch.createUnstarted(),
-          InternalServiceProviders.isAndroid(getClass().getClassLoader()),
-          isSrvEnabled());
-    } else {
-      return null;
-    }
-  }
-
-  @Override
-  public String getDefaultScheme() {
-    return SCHEME;
-  }
-
-  @Override
-  protected boolean isAvailable() {
-    return true;
-  }
-}

core/src/main/java/io/grpc/internal/DnsNameResolver.java

@@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Throwables;
@@ -63,7 +64,7 @@ import javax.annotation.Nullable;
  *
  * @see DnsNameResolverProvider
  */
-final class DnsNameResolver extends NameResolver {
+public class DnsNameResolver extends NameResolver {
 
   private static final Logger logger = Logger.getLogger(DnsNameResolver.class.getName());
 
@@ -85,8 +86,6 @@ final class DnsNameResolver extends NameResolver {
 
   // From https://github.com/grpc/proposal/blob/master/A2-service-configs-in-dns.md
   private static final String SERVICE_CONFIG_NAME_PREFIX = "_grpc_config.";
-  // From https://github.com/grpc/proposal/blob/master/A5-grpclb-in-dns.md
-  private static final String GRPCLB_NAME_PREFIX = "_grpclb._tcp.";
 
   private static final String JNDI_PROPERTY =
       System.getProperty("io.grpc.internal.DnsNameResolverProvider.enable_jndi", "true");
@@ -115,7 +114,7 @@ final class DnsNameResolver extends NameResolver {
   @VisibleForTesting
   static boolean enableJndiLocalhost = Boolean.parseBoolean(JNDI_LOCALHOST_PROPERTY);
   @VisibleForTesting
-  static boolean enableTxt = Boolean.parseBoolean(JNDI_TXT_PROPERTY);
+  protected static boolean enableTxt = Boolean.parseBoolean(JNDI_TXT_PROPERTY);
 
   private static final ResourceResolverFactory resourceResolverFactory =
       getResourceResolverFactory(DnsNameResolver.class.getClassLoader());
@@ -128,7 +127,7 @@ final class DnsNameResolver extends NameResolver {
 
   private final Random random = new Random();
 
-  private volatile AddressResolver addressResolver = JdkAddressResolver.INSTANCE;
+  protected volatile AddressResolver addressResolver = JdkAddressResolver.INSTANCE;
   private final AtomicReference<ResourceResolver> resourceResolver = new AtomicReference<>();
 
   private final String authority;
@@ -142,13 +141,12 @@ final class DnsNameResolver extends NameResolver {
 
   // Following fields must be accessed from syncContext
   private final Stopwatch stopwatch;
-  private ResolutionResults cachedResolutionResults;
+  protected boolean resolved;
   private boolean shutdown;
   private Executor executor;
 
   /** True if using an executor resource that should be released after use. */
   private final boolean usingExecutorResource;
-  private final boolean enableSrv;
   private final ServiceConfigParser serviceConfigParser;
 
   private boolean resolving;
@@ -157,14 +155,13 @@ final class DnsNameResolver extends NameResolver {
   // from any thread.
   private NameResolver.Listener2 listener;
 
-  DnsNameResolver(
+  protected DnsNameResolver(
       @Nullable String nsAuthority,
       String name,
       Args args,
       Resource<Executor> executorResource,
       Stopwatch stopwatch,
-      boolean isAndroid,
+      boolean isAndroid) {
-      boolean enableSrv) {
     checkNotNull(args, "args");
     // TODO: if a DNS server is provided as nsAuthority, use it.
     // https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java
@@ -187,7 +184,6 @@ final class DnsNameResolver extends NameResolver {
     this.syncContext = checkNotNull(args.getSynchronizationContext(), "syncContext");
     this.executor = args.getOffloadExecutor();
     this.usingExecutorResource = executor == null;
-    this.enableSrv = enableSrv;
     this.serviceConfigParser = checkNotNull(args.getServiceConfigParser(), "serviceConfigParser");
   }
 
@@ -196,6 +192,11 @@ final class DnsNameResolver extends NameResolver {
     return authority;
   }
 
+  @VisibleForTesting
+  protected String getHost() {
+    return host;
+  }
+
   @Override
   public void start(Listener2 listener) {
     Preconditions.checkState(this.listener == null, "already started");
@@ -212,6 +213,87 @@ final class DnsNameResolver extends NameResolver {
     resolve();
   }
 
+  private List<EquivalentAddressGroup> resolveAddresses() {
+    List<? extends InetAddress> addresses;
+    Exception addressesException = null;
+    try {
+      addresses = addressResolver.resolveAddress(host);
+    } catch (Exception e) {
+      addressesException = e;
+      Throwables.throwIfUnchecked(e);
+      throw new RuntimeException(e);
+    } finally {
+      if (addressesException != null) {
+        logger.log(Level.FINE, "Address resolution failure", addressesException);
+      }
+    }
+    // Each address forms an EAG
+    List<EquivalentAddressGroup> servers = new ArrayList<>(addresses.size());
+    for (InetAddress inetAddr : addresses) {
+      servers.add(new EquivalentAddressGroup(new InetSocketAddress(inetAddr, port)));
+    }
+    return Collections.unmodifiableList(servers);
+  }
+
+  @Nullable
+  private ConfigOrError resolveServiceConfig() {
+    List<String> txtRecords = Collections.emptyList();
+    ResourceResolver resourceResolver = getResourceResolver();
+    if (resourceResolver != null) {
+      try {
+        txtRecords = resourceResolver.resolveTxt(SERVICE_CONFIG_NAME_PREFIX + host);
+      } catch (Exception e) {
+        logger.log(Level.FINE, "ServiceConfig resolution failure", e);
+      }
+    }
+    if (!txtRecords.isEmpty()) {
+      ConfigOrError rawServiceConfig = parseServiceConfig(txtRecords, random, getLocalHostname());
+      if (rawServiceConfig != null) {
+        if (rawServiceConfig.getError() != null) {
+          return ConfigOrError.fromError(rawServiceConfig.getError());
+        }
+
+        @SuppressWarnings("unchecked")
+        Map<String, ?> verifiedRawServiceConfig = (Map<String, ?>) rawServiceConfig.getConfig();
+        return serviceConfigParser.parseServiceConfig(verifiedRawServiceConfig);
+      }
+    } else {
+      logger.log(Level.FINE, "No TXT records found for {0}", new Object[]{host});
+    }
+    return null;
+  }
+
+  @Nullable
+  private EquivalentAddressGroup detectProxy() throws IOException {
+    InetSocketAddress destination =
+        InetSocketAddress.createUnresolved(host, port);
+    ProxiedSocketAddress proxiedAddr = proxyDetector.proxyFor(destination);
+    if (proxiedAddr != null) {
+      return new EquivalentAddressGroup(proxiedAddr);
+    }
+    return null;
+  }
+
+  /**
+   * Main logic of name resolution.
+   */
+  protected InternalResolutionResult doResolve(boolean forceTxt) {
+    InternalResolutionResult result = new InternalResolutionResult();
+    try {
+      result.addresses = resolveAddresses();
+    } catch (Exception e) {
+      if (!forceTxt) {
+        result.error =
+            Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e);
+        return result;
+      }
+    }
+    if (enableTxt) {
+      result.config = resolveServiceConfig();
+    }
+    return result;
+  }
+
   private final class Resolve implements Runnable {
     private final Listener2 savedListener;
 
@@ -224,105 +306,50 @@ final class DnsNameResolver extends NameResolver {
       if (logger.isLoggable(Level.FINER)) {
         logger.finer("Attempting DNS resolution of " + host);
       }
+      InternalResolutionResult result = null;
       try {
-        resolveInternal();
+        EquivalentAddressGroup proxiedAddr = detectProxy();
-      } finally {
+        ResolutionResult.Builder resolutionResultBuilder = ResolutionResult.newBuilder();
-        syncContext.execute(new Runnable() {
+        if (proxiedAddr != null) {
-            @Override
+          if (logger.isLoggable(Level.FINER)) {
-            public void run() {
+            logger.finer("Using proxy address " + proxiedAddr);
-              resolving = false;
+          }
-            }
+          resolutionResultBuilder.setAddresses(Collections.singletonList(proxiedAddr));
-          });
+        } else {
-      }
+          result = doResolve(false);
-    }
+          if (result.error != null) {
-
+            savedListener.onError(result.error);
-    @VisibleForTesting
+            return;
-    @SuppressWarnings("deprecation") // can migrate after service config error handling is finished
+          }
-    void resolveInternal() {
+          if (result.addresses != null) {
-      InetSocketAddress destination =
+            resolutionResultBuilder.setAddresses(result.addresses);
-          InetSocketAddress.createUnresolved(host, port);
+          }
-      ProxiedSocketAddress proxiedAddr;
+          if (result.config != null) {
-      try {
+            resolutionResultBuilder.setServiceConfig(result.config);
-        proxiedAddr = proxyDetector.proxyFor(destination);
+          }
+          if (result.attributes != null) {
+            resolutionResultBuilder.setAttributes(result.attributes);
+          }
+        }
+        savedListener.onResult(resolutionResultBuilder.build());
       } catch (IOException e) {
         savedListener.onError(
             Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e));
-        return;
+      } finally {
-      }
+        final boolean succeed = result != null && result.error == null;
-      if (proxiedAddr != null) {
-        if (logger.isLoggable(Level.FINER)) {
-          logger.finer("Using proxy address " + proxiedAddr);
-        }
-        EquivalentAddressGroup server = new EquivalentAddressGroup(proxiedAddr);
-        ResolutionResult resolutionResult =
-            ResolutionResult.newBuilder()
-                .setAddresses(Collections.singletonList(server))
-                .setAttributes(Attributes.EMPTY)
-                .build();
-        savedListener.onResult(resolutionResult);
-        return;
-      }
-
-      ResolutionResults resolutionResults;
-      try {
-        ResourceResolver resourceResolver = null;
-        if (shouldUseJndi(enableJndi, enableJndiLocalhost, host)) {
-          resourceResolver = getResourceResolver();
-        }
-        final ResolutionResults results = resolveAll(
-            addressResolver,
-            resourceResolver,
-            enableSrv,
-            enableTxt,
-            host);
-        resolutionResults = results;
         syncContext.execute(new Runnable() {
-            @Override
+          @Override
-            public void run() {
+          public void run() {
-              cachedResolutionResults = results;
+            if (succeed) {
+              resolved = true;
               if (cacheTtlNanos > 0) {
                 stopwatch.reset().start();
               }
             }
-          });
+            resolving = false;
-        if (logger.isLoggable(Level.FINER)) {
-          logger.finer("Found DNS results " + resolutionResults + " for " + host);
-        }
-      } catch (Exception e) {
-        savedListener.onError(
-            Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e));
-        return;
-      }
-      // Each address forms an EAG
-      List<EquivalentAddressGroup> servers = new ArrayList<>();
-      for (InetAddress inetAddr : resolutionResults.addresses) {
-        servers.add(new EquivalentAddressGroup(new InetSocketAddress(inetAddr, port)));
-      }
-
-      ResolutionResult.Builder resultBuilder = ResolutionResult.newBuilder().setAddresses(servers);
-      Attributes.Builder attributesBuilder = Attributes.newBuilder();
-      if (!resolutionResults.balancerAddresses.isEmpty()) {
-        attributesBuilder.set(GrpcAttributes.ATTR_LB_ADDRS, resolutionResults.balancerAddresses);
-      }
-      if (!resolutionResults.txtRecords.isEmpty()) {
-        ConfigOrError rawServiceConfig =
-            parseServiceConfig(resolutionResults.txtRecords, random, getLocalHostname());
-        if (rawServiceConfig != null) {
-          if (rawServiceConfig.getError() != null) {
-            savedListener.onError(rawServiceConfig.getError());
-            return;
           }
-
+        });
-          @SuppressWarnings("unchecked")
-          Map<String, ?> verifiedRawServiceConfig = (Map<String, ?>) rawServiceConfig.getConfig();
-          ConfigOrError parsedServiceConfig =
-              serviceConfigParser.parseServiceConfig(verifiedRawServiceConfig);
-          resultBuilder.setServiceConfig(parsedServiceConfig);
-        }
-      } else {
-        logger.log(Level.FINE, "No TXT records found for {0}", new Object[]{host});
       }
-      savedListener.onResult(resultBuilder.setAttributes(attributesBuilder.build()).build());
     }
   }
 
@@ -364,7 +391,7 @@ final class DnsNameResolver extends NameResolver {
   }
 
   private boolean cacheRefreshRequired() {
-    return cachedResolutionResults == null
+    return !resolved
         || cacheTtlNanos == 0
         || (cacheTtlNanos > 0 && stopwatch.elapsed(TimeUnit.NANOSECONDS) > cacheTtlNanos);
   }
@@ -384,69 +411,6 @@ final class DnsNameResolver extends NameResolver {
     return port;
   }
 
-  @VisibleForTesting
-  static ResolutionResults resolveAll(
-      AddressResolver addressResolver,
-      @Nullable ResourceResolver resourceResolver,
-      boolean requestSrvRecords,
-      boolean requestTxtRecords,
-      String name) {
-    List<? extends InetAddress> addresses = Collections.emptyList();
-    Exception addressesException = null;
-    List<EquivalentAddressGroup> balancerAddresses = Collections.emptyList();
-    Exception balancerAddressesException = null;
-    List<String> txtRecords = Collections.emptyList();
-    Exception txtRecordsException = null;
-
-    try {
-      addresses = addressResolver.resolveAddress(name);
-    } catch (Exception e) {
-      addressesException = e;
-    }
-    if (resourceResolver != null) {
-      if (requestSrvRecords) {
-        try {
-          balancerAddresses =
-              resourceResolver.resolveSrv(addressResolver, GRPCLB_NAME_PREFIX + name);
-        } catch (Exception e) {
-          balancerAddressesException = e;
-        }
-      }
-      if (requestTxtRecords) {
-        boolean balancerLookupFailedOrNotAttempted =
-            !requestSrvRecords || balancerAddressesException != null;
-        boolean dontResolveTxt =
-            (addressesException != null) && balancerLookupFailedOrNotAttempted;
-        // Only do the TXT record lookup if one of the above address resolutions succeeded.
-        if (!dontResolveTxt) {
-          try {
-            txtRecords = resourceResolver.resolveTxt(SERVICE_CONFIG_NAME_PREFIX + name);
-          } catch (Exception e) {
-            txtRecordsException = e;
-          }
-        }
-      }
-    }
-    try {
-      if (addressesException != null
-          && (balancerAddressesException != null || balancerAddresses.isEmpty())) {
-        Throwables.throwIfUnchecked(addressesException);
-        throw new RuntimeException(addressesException);
-      }
-    } finally {
-      if (addressesException != null) {
-        logger.log(Level.FINE, "Address resolution failure", addressesException);
-      }
-      if (balancerAddressesException != null) {
-        logger.log(Level.FINE, "Balancer resolution failure", balancerAddressesException);
-      }
-      if (txtRecordsException != null) {
-        logger.log(Level.FINE, "ServiceConfig resolution failure", txtRecordsException);
-      }
-    }
-    return new ResolutionResults(addresses, txtRecords, balancerAddresses);
-  }
-
   /**
    *
    * @throws IOException if one of the txt records contains improperly formatted JSON.
@@ -572,41 +536,64 @@ final class DnsNameResolver extends NameResolver {
   }
 
   /**
-   * Describes the results from a DNS query.
+   * Used as a DNS-based name resolver's internal representation of resolution result.
+   */
+  protected static final class InternalResolutionResult {
+    private Status error;
+    private List<EquivalentAddressGroup> addresses;
+    private ConfigOrError config;
+    public Attributes attributes;
+
+    private InternalResolutionResult() {}
+  }
+
+  /**
+   * Describes a parsed SRV record.
    */
   @VisibleForTesting
-  static final class ResolutionResults {
+  public static final class SrvRecord {
-    final List<? extends InetAddress> addresses;
+    public final String host;
-    final List<String> txtRecords;
+    public final int port;
-    final List<EquivalentAddressGroup> balancerAddresses;
 
-    ResolutionResults(
+    public SrvRecord(String host, int port) {
-        List<? extends InetAddress> addresses,
+      this.host = host;
-        List<String> txtRecords,
+      this.port = port;
-        List<EquivalentAddressGroup> balancerAddresses) {
+    }
-      this.addresses = Collections.unmodifiableList(checkNotNull(addresses, "addresses"));
+
-      this.txtRecords = Collections.unmodifiableList(checkNotNull(txtRecords, "txtRecords"));
+    @Override
-      this.balancerAddresses =
+    public int hashCode() {
-          Collections.unmodifiableList(checkNotNull(balancerAddresses, "balancerAddresses"));
+      return Objects.hashCode(host, port);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (this == obj) {
+        return true;
+      }
+      if (obj == null || getClass() != obj.getClass()) {
+        return false;
+      }
+      SrvRecord that = (SrvRecord) obj;
+      return port == that.port && host.equals(that.host);
     }
 
     @Override
     public String toString() {
-      return MoreObjects.toStringHelper(this)
+      return
-          .add("addresses", addresses)
+          MoreObjects.toStringHelper(this)
-          .add("txtRecords", txtRecords)
+              .add("host", host)
-          .add("balancerAddresses", balancerAddresses)
+              .add("port", port)
-          .toString();
+              .toString();
     }
   }
 
   @VisibleForTesting
-  void setAddressResolver(AddressResolver addressResolver) {
+  protected void setAddressResolver(AddressResolver addressResolver) {
     this.addressResolver = addressResolver;
   }
 
   @VisibleForTesting
-  void setResourceResolver(ResourceResolver resourceResolver) {
+  protected void setResourceResolver(ResourceResolver resourceResolver) {
     this.resourceResolver.set(resourceResolver);
   }
 
@@ -632,7 +619,8 @@ final class DnsNameResolver extends NameResolver {
   /**
    * AddressResolver resolves a hostname into a list of addresses.
    */
-  interface AddressResolver {
+  @VisibleForTesting
+  public interface AddressResolver {
     List<InetAddress> resolveAddress(String host) throws Exception;
   }
 
@@ -648,15 +636,18 @@ final class DnsNameResolver extends NameResolver {
   /**
    * {@link ResourceResolver} is a Dns ResourceRecord resolver.
    */
-  interface ResourceResolver {
+  @VisibleForTesting
+  public interface ResourceResolver {
     List<String> resolveTxt(String host) throws Exception;
 
-    List<EquivalentAddressGroup> resolveSrv(
+    List<SrvRecord> resolveSrv(String host) throws Exception;
-        AddressResolver addressResolver, String host) throws Exception;
   }
 
   @Nullable
-  private ResourceResolver getResourceResolver() {
+  protected ResourceResolver getResourceResolver() {
+    if (!shouldUseJndi(enableJndi, enableJndiLocalhost, host)) {
+      return null;
+    }
     ResourceResolver rr;
     if ((rr = resourceResolver.get()) == null) {
       if (resourceResolverFactory != null) {
@@ -724,7 +715,8 @@ final class DnsNameResolver extends NameResolver {
   }
 
   @VisibleForTesting
-  static boolean shouldUseJndi(boolean jndiEnabled, boolean jndiLocalhostEnabled, String target) {
+  protected static boolean shouldUseJndi(
+      boolean jndiEnabled, boolean jndiLocalhostEnabled, String target) {
     if (!jndiEnabled) {
       return false;
     }

core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java

@@ -16,6 +16,13 @@
 
 package io.grpc.internal;
 
+import com.google.common.base.Preconditions;
+import com.google.common.base.Stopwatch;
+import io.grpc.InternalServiceProviders;
+import io.grpc.NameResolver;
+import io.grpc.NameResolverProvider;
+import java.net.URI;
+
 /**
  * A provider for {@link DnsNameResolver}.
  *
@@ -31,14 +38,37 @@ package io.grpc.internal;
  *   <li>{@code "dns:///foo.googleapis.com"} (without port)</li>
  * </ul>
  */
-public final class DnsNameResolverProvider extends BaseDnsNameResolverProvider {
+public final class DnsNameResolverProvider extends NameResolverProvider {
 
-  private static final boolean SRV_ENABLED =
+  private static final String SCHEME = "dns";
-      Boolean.parseBoolean(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME, "false"));
 
   @Override
-  protected boolean isSrvEnabled() {
+  public DnsNameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
-    return SRV_ENABLED;
+    if (SCHEME.equals(targetUri.getScheme())) {
+      String targetPath = Preconditions.checkNotNull(targetUri.getPath(), "targetPath");
+      Preconditions.checkArgument(targetPath.startsWith("/"),
+          "the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri);
+      String name = targetPath.substring(1);
+      return new DnsNameResolver(
+          targetUri.getAuthority(),
+          name,
+          args,
+          GrpcUtil.SHARED_CHANNEL_EXECUTOR,
+          Stopwatch.createUnstarted(),
+          InternalServiceProviders.isAndroid(getClass().getClassLoader()));
+    } else {
+      return null;
+    }
+  }
+
+  @Override
+  public String getDefaultScheme() {
+    return SCHEME;
+  }
+
+  @Override
+  protected boolean isAvailable() {
+    return true;
   }
 
   @Override

core/src/main/java/io/grpc/internal/GrpcAttributes.java

@@ -19,24 +19,12 @@ package io.grpc.internal;
 import io.grpc.Attributes;
 import io.grpc.EquivalentAddressGroup;
 import io.grpc.Grpc;
-import io.grpc.NameResolver;
 import io.grpc.SecurityLevel;
-import java.util.List;
 
 /**
  * Special attributes that are only useful to gRPC.
  */
 public final class GrpcAttributes {
-  /**
-   * Attribute key for gRPC LB server addresses.
-   *
-   * <p>Deprecated: this will be used for grpclb specific logic, which will be moved out of core.
-   */
-  @Deprecated
-  @NameResolver.ResolutionResultAttr
-  public static final Attributes.Key<List<EquivalentAddressGroup>> ATTR_LB_ADDRS =
-      Attributes.Key.create("io.grpc.grpclb.lbAddrs");
-
   /**
    * The naming authority of a gRPC LB server address.  It is an address-group-level attribute,
    * present when the address group is a LoadBalancer.

core/src/main/java/io/grpc/internal/JndiResourceResolverFactory.java

@@ -19,14 +19,8 @@ package io.grpc.internal;
 import android.annotation.SuppressLint;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Verify;
-import io.grpc.Attributes;
-import io.grpc.EquivalentAddressGroup;
-import io.grpc.internal.DnsNameResolver.AddressResolver;
 import io.grpc.internal.DnsNameResolver.ResourceResolver;
-import java.net.InetAddress;
+import io.grpc.internal.DnsNameResolver.SrvRecord;
-import java.net.InetSocketAddress;
-import java.net.SocketAddress;
-import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -129,82 +123,42 @@ final class JndiResourceResolverFactory implements DnsNameResolver.ResourceResol
       return Collections.unmodifiableList(serviceConfigTxtRecords);
     }
 
-    @SuppressWarnings("deprecation")
     @Override
-    public List<EquivalentAddressGroup> resolveSrv(
+    public List<SrvRecord> resolveSrv(String host) throws Exception {
-        AddressResolver addressResolver, String grpclbHostname) throws Exception {
       if (logger.isLoggable(Level.FINER)) {
         logger.log(
-            Level.FINER, "About to query SRV records for {0}", new Object[]{grpclbHostname});
+            Level.FINER, "About to query SRV records for {0}", new Object[]{host});
       }
-      List<String> grpclbSrvRecords =
+      List<String> rawSrvRecords =
-          recordFetcher.getAllRecords("SRV", "dns:///" + grpclbHostname);
+          recordFetcher.getAllRecords("SRV", "dns:///" + host);
       if (logger.isLoggable(Level.FINER)) {
         logger.log(
-            Level.FINER, "Found {0} SRV records", new Object[]{grpclbSrvRecords.size()});
+            Level.FINER, "Found {0} SRV records", new Object[]{rawSrvRecords.size()});
       }
-      List<EquivalentAddressGroup> balancerAddresses =
+      List<SrvRecord> srvRecords = new ArrayList<>(rawSrvRecords.size());
-          new ArrayList<>(grpclbSrvRecords.size());
       Exception first = null;
       Level level = Level.WARNING;
-      for (String srvRecord : grpclbSrvRecords) {
+      for (String rawSrv : rawSrvRecords) {
         try {
-          SrvRecord record = parseSrvRecord(srvRecord);
+          String[] parts = whitespace.split(rawSrv);
+          Verify.verify(parts.length == 4, "Bad SRV Record: %s", rawSrv);
           // SRV requires the host name to be absolute
-          if (!record.host.endsWith(".")) {
+          if (!parts[3].endsWith(".")) {
-            throw new RuntimeException("Returned SRV host does not end in period: " + record.host);
+            throw new RuntimeException("Returned SRV host does not end in period: " + parts[3]);
-          }
-
-          // Strip trailing dot for appearance's sake. It _should_ be fine either way, but most
-          // people expect to see it without the dot.
-          String authority = record.host.substring(0, record.host.length() - 1);
-          // But we want to use the trailing dot for the IP lookup. The dot makes the name absolute
-          // instead of relative and so will avoid the search list like that in resolv.conf.
-          List<? extends InetAddress> addrs = addressResolver.resolveAddress(record.host);
-          List<SocketAddress> sockaddrs = new ArrayList<>(addrs.size());
-          for (InetAddress addr : addrs) {
-            sockaddrs.add(new InetSocketAddress(addr, record.port));
-          }
-          Attributes attrs = Attributes.newBuilder()
-              .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, authority)
-              .build();
-          balancerAddresses.add(
-              new EquivalentAddressGroup(Collections.unmodifiableList(sockaddrs), attrs));
-        } catch (UnknownHostException e) {
-          logger.log(level, "Can't find address for SRV record " + srvRecord, e);
-          // TODO(carl-mastrangelo): these should be added by addSuppressed when we have Java 7.
-          if (first == null) {
-            first = e;
-            level = Level.FINE;
           }
+          srvRecords.add(new SrvRecord(parts[3], Integer.parseInt(parts[2])));
         } catch (RuntimeException e) {
-          logger.log(level, "Failed to construct SRV record " + srvRecord, e);
+          logger.log(level, "Failed to construct SRV record " + rawSrv, e);
           if (first == null) {
             first = e;
             level = Level.FINE;
           }
         }
       }
-      if (balancerAddresses.isEmpty() && first != null) {
+      if (srvRecords.isEmpty() && first != null) {
         throw first;
       }
-      return Collections.unmodifiableList(balancerAddresses);
+      return Collections.unmodifiableList(srvRecords);
-    }
-
-    private static final class SrvRecord {
-      SrvRecord(String host, int port) {
-        this.host = host;
-        this.port = port;
-      }
-
-      final String host;
-      final int port;
-    }
-
-    private static SrvRecord parseSrvRecord(String rawRecord) {
-      String[] parts = whitespace.split(rawRecord);
-      Verify.verify(parts.length == 4, "Bad SRV Record: %s", rawRecord);
-      return new SrvRecord(parts[3], Integer.parseInt(parts[2]));
     }
 
     /**

core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java

@@ -16,12 +16,9 @@
 
 package io.grpc.internal;
 
-import static com.google.common.truth.Truth.assertThat;
-import static io.grpc.internal.BaseDnsNameResolverProvider.ENABLE_GRPCLB_PROPERTY_NAME;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeTrue;
 import static org.mockito.Mockito.mock;
 
 import io.grpc.ChannelLogger;
@@ -65,11 +62,4 @@ public class DnsNameResolverProviderTest {
     assertNull(
         provider.newNameResolver(URI.create("notdns:///localhost:443"), args));
   }
-
-  @Test
-  public void isSrvEnabled_falseByDefault() {
-    assumeTrue(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME) == null);
-
-    assertThat(provider.isSrvEnabled()).isFalse();
-  }
 }

core/src/test/java/io/grpc/internal/DnsNameResolverTest.java

@@ -26,6 +26,7 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -47,16 +48,15 @@ import io.grpc.NameResolver.ServiceConfigParser;
 import io.grpc.ProxyDetector;
 import io.grpc.StaticTestingClassLoader;
 import io.grpc.Status;
+import io.grpc.Status.Code;
 import io.grpc.SynchronizationContext;
 import io.grpc.internal.DnsNameResolver.AddressResolver;
-import io.grpc.internal.DnsNameResolver.ResolutionResults;
 import io.grpc.internal.DnsNameResolver.ResourceResolver;
 import io.grpc.internal.DnsNameResolver.ResourceResolverFactory;
 import io.grpc.internal.JndiResourceResolverFactory.JndiResourceResolver;
 import io.grpc.internal.JndiResourceResolverFactory.RecordFetcher;
 import io.grpc.internal.SharedResourceHolder.Resource;
 import java.io.IOException;
-import java.net.Inet4Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
@@ -87,7 +87,6 @@ import org.junit.rules.Timeout;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 import org.mockito.ArgumentCaptor;
-import org.mockito.ArgumentMatchers;
 import org.mockito.Captor;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnit;
@@ -142,6 +141,8 @@ public class DnsNameResolverTest {
   private NameResolver.Listener2 mockListener;
   @Captor
   private ArgumentCaptor<ResolutionResult> resultCaptor;
+  @Captor
+  private ArgumentCaptor<Status> errorCaptor;
   @Nullable
   private String networkaddressCacheTtlPropertyValue;
   @Mock
@@ -155,7 +156,7 @@ public class DnsNameResolverTest {
   private DnsNameResolver newResolver(String name, int defaultPort, boolean isAndroid) {
     return newResolver(
         name, defaultPort, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(),
-        isAndroid, false);
+        isAndroid);
   }
 
   private DnsNameResolver newResolver(
@@ -163,7 +164,7 @@ public class DnsNameResolverTest {
       int defaultPort,
       ProxyDetector proxyDetector,
       Stopwatch stopwatch) {
-    return newResolver(name, defaultPort, proxyDetector, stopwatch, false, false);
+    return newResolver(name, defaultPort, proxyDetector, stopwatch, false);
   }
 
   private DnsNameResolver newResolver(
@@ -171,8 +172,7 @@ public class DnsNameResolverTest {
       final int defaultPort,
       final ProxyDetector proxyDetector,
       Stopwatch stopwatch,
-      boolean isAndroid,
+      boolean isAndroid) {
-      boolean enableSrv) {
     NameResolver.Args args =
         NameResolver.Args.newBuilder()
             .setDefaultPort(defaultPort)
@@ -181,34 +181,22 @@ public class DnsNameResolverTest {
             .setServiceConfigParser(mock(ServiceConfigParser.class))
             .setChannelLogger(mock(ChannelLogger.class))
             .build();
-    return newResolver(name, stopwatch, isAndroid, args, enableSrv);
+    return newResolver(name, stopwatch, isAndroid, args);
-  }
-
-  private DnsNameResolver newResolver(
-      String name, Stopwatch stopwatch, boolean isAndroid, NameResolver.Args args) {
-    return newResolver(name, stopwatch, isAndroid, args, /* enableSrv= */ false);
   }
 
   private DnsNameResolver newResolver(
       String name,
       Stopwatch stopwatch,
       boolean isAndroid,
-      NameResolver.Args args,
+      NameResolver.Args args) {
-      boolean enableSrv) {
     DnsNameResolver dnsResolver =
         new DnsNameResolver(
-            null, name, args, fakeExecutorResource, stopwatch, isAndroid, enableSrv);
+            null, name, args, fakeExecutorResource, stopwatch, isAndroid);
     // By default, using the mocked ResourceResolver to avoid I/O
     dnsResolver.setResourceResolver(new JndiResourceResolver(recordFetcher));
     return dnsResolver;
   }
 
-  private DnsNameResolver newSrvEnabledResolver(String name, int defaultPort) {
-    return newResolver(
-        name, defaultPort, GrpcUtil.NOOP_PROXY_DETECTOR, Stopwatch.createUnstarted(),
-        false, true);
-  }
-
   @Before
   public void setUp() {
     DnsNameResolver.enableJndi = true;
@@ -529,7 +517,8 @@ public class DnsNameResolverTest {
   }
 
   @Test
-  public void resolve_emptyResult() {
+  public void resolve_emptyResult() throws Exception {
+    DnsNameResolver.enableTxt = true;
     DnsNameResolver nr = newResolver("dns:///addr.fake:1234", 443);
     nr.setAddressResolver(new AddressResolver() {
       @Override
@@ -537,18 +526,11 @@ public class DnsNameResolverTest {
         return Collections.emptyList();
       }
     });
-    nr.setResourceResolver(new ResourceResolver() {
+    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
-      @Override
+    when(mockResourceResolver.resolveTxt(anyString()))
-      public List<String> resolveTxt(String host) throws Exception {
+        .thenReturn(Collections.<String>emptyList());
-        return Collections.emptyList();
-      }
 
-      @Override
+    nr.setResourceResolver(mockResourceResolver);
-      public List<EquivalentAddressGroup> resolveSrv(AddressResolver addressResolver, String host)
-          throws Exception {
-        return Collections.emptyList();
-      }
-    });
 
     nr.start(mockListener);
     assertThat(fakeExecutor.runDueTasks()).isEqualTo(1);
@@ -559,29 +541,81 @@ public class DnsNameResolverTest {
     assertThat(ac.getValue().getAddresses()).isEmpty();
     assertThat(ac.getValue().getAttributes()).isEqualTo(Attributes.EMPTY);
     assertThat(ac.getValue().getServiceConfig()).isNull();
+    verify(mockResourceResolver, never()).resolveSrv(anyString());
   }
 
-  @SuppressWarnings("deprecation")
   @Test
-  public void resolve_balancerAddrsAsAttributes() throws Exception {
+  public void resolve_nullResourceResolver() throws Exception {
-    InetAddress backendAddr = InetAddress.getByAddress(new byte[] {127, 0, 0, 0});
+    DnsNameResolver.enableTxt = true;
-    final EquivalentAddressGroup balancerAddr =
+    InetAddress backendAddr = InetAddresses.fromInteger(0x7f000001);
-        new EquivalentAddressGroup(
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
-            new SocketAddress() {},
+    when(mockAddressResolver.resolveAddress(anyString()))
-            Attributes.newBuilder()
+        .thenReturn(Collections.singletonList(backendAddr));
-                .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "foo.example.com")
-                .build());
     String name = "foo.googleapis.com";
 
+    DnsNameResolver resolver = newResolver(name, 81);
+    resolver.setAddressResolver(mockAddressResolver);
+    resolver.setResourceResolver(null);
+    resolver.start(mockListener);
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener).onResult(resultCaptor.capture());
+    ResolutionResult result = resultCaptor.getValue();
+    InetSocketAddress resolvedBackendAddr =
+        (InetSocketAddress) Iterables.getOnlyElement(
+            Iterables.getOnlyElement(result.getAddresses()).getAddresses());
+    assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
+    verify(mockAddressResolver).resolveAddress(name);
+    assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
+    assertThat(result.getServiceConfig()).isNull();
+  }
+
+  @Test
+  public void resolve_nullResourceResolver_addressFailure() throws Exception {
+    DnsNameResolver.enableTxt = true;
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
+    when(mockAddressResolver.resolveAddress(anyString()))
+        .thenThrow(new IOException("no addr"));
+    String name = "foo.googleapis.com";
+
+    DnsNameResolver resolver = newResolver(name, 81);
+    resolver.setAddressResolver(mockAddressResolver);
+    resolver.setResourceResolver(null);
+    resolver.start(mockListener);
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener).onError(errorCaptor.capture());
+    Status errorStatus = errorCaptor.getValue();
+    assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
+    assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr");
+  }
+
+  @Test
+  public void resolve_presentResourceResolver() throws Exception {
+    DnsNameResolver.enableTxt = true;
+    InetAddress backendAddr = InetAddresses.fromInteger(0x7f000001);
     AddressResolver mockAddressResolver = mock(AddressResolver.class);
     when(mockAddressResolver.resolveAddress(anyString()))
         .thenReturn(Collections.singletonList(backendAddr));
     ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
-    when(mockResourceResolver.resolveTxt(anyString())).thenReturn(Collections.<String>emptyList());
+    when(mockResourceResolver.resolveTxt(anyString()))
-    when(mockResourceResolver.resolveSrv(ArgumentMatchers.any(AddressResolver.class), anyString()))
+        .thenReturn(
-        .thenReturn(Collections.singletonList(balancerAddr));
+            Collections.singletonList(
+                "grpc_config=[{\"clientLanguage\": [\"java\"], \"serviceConfig\": {}}]"));
+    ServiceConfigParser serviceConfigParser = new ServiceConfigParser() {
+      @Override
+      public ConfigOrError parseServiceConfig(Map<String, ?> rawServiceConfig) {
+        return ConfigOrError.fromConfig(rawServiceConfig);
+      }
+    };
+    NameResolver.Args args =
+        NameResolver.Args.newBuilder()
+            .setDefaultPort(DEFAULT_PORT)
+            .setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR)
+            .setSynchronizationContext(syncContext)
+            .setServiceConfigParser(serviceConfigParser)
+            .build();
 
-    DnsNameResolver resolver = newSrvEnabledResolver(name, 81);
+    String name = "foo.googleapis.com";
+    DnsNameResolver resolver = newResolver(name, Stopwatch.createUnstarted(), false, args);
     resolver.setAddressResolver(mockAddressResolver);
     resolver.setResourceResolver(mockResourceResolver);
 
@@ -593,123 +627,89 @@ public class DnsNameResolverTest {
         (InetSocketAddress) Iterables.getOnlyElement(
             Iterables.getOnlyElement(result.getAddresses()).getAddresses());
     assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
-    assertThat(result.getAttributes().get(GrpcAttributes.ATTR_LB_ADDRS))
+    assertThat(result.getServiceConfig().getConfig()).isNotNull();
-        .containsExactly(balancerAddr);
+    verify(mockAddressResolver).resolveAddress(name);
+    verify(mockResourceResolver).resolveTxt("_grpc_config." + name);
   }
 
   @Test
-  public void resolveAll_nullResourceResolver() throws Exception {
+  public void resolve_addressFailure_neverLookUpServiceConfig() throws Exception {
-    final String hostname = "addr.fake";
+    DnsNameResolver.enableTxt = true;
-    final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001);
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
-
+    when(mockAddressResolver.resolveAddress(anyString()))
-    AddressResolver mockResolver = mock(AddressResolver.class);
-    when(mockResolver.resolveAddress(anyString()))
-        .thenReturn(Collections.<InetAddress>singletonList(backendAddr));
-    ResourceResolver resourceResolver = null;
-    boolean resovleSrv = true;
-    boolean resolveTxt = true;
-
-    ResolutionResults res = DnsNameResolver.resolveAll(
-        mockResolver, resourceResolver, resovleSrv, resolveTxt, hostname);
-    assertThat(res.addresses).containsExactly(backendAddr);
-    assertThat(res.balancerAddresses).isEmpty();
-    assertThat(res.txtRecords).isEmpty();
-    verify(mockResolver).resolveAddress(hostname);
-  }
-
-  @Test
-  public void resolveAll_nullResourceResolver_addressFailure() throws Exception {
-    final String hostname = "addr.fake";
-
-    AddressResolver mockResolver = mock(AddressResolver.class);
-    when(mockResolver.resolveAddress(anyString()))
         .thenThrow(new IOException("no addr"));
-    ResourceResolver resourceResolver = null;
+    String name = "foo.googleapis.com";
-    boolean resovleSrv = true;
-    boolean resolveTxt = true;
 
-    thrown.expect(RuntimeException.class);
+    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
-    thrown.expectMessage("no addr");
+    DnsNameResolver resolver = newResolver(name, 81);
-
+    resolver.setAddressResolver(mockAddressResolver);
-    DnsNameResolver.resolveAll(mockResolver, resourceResolver, resovleSrv, resolveTxt, hostname);
+    resolver.setResourceResolver(mockResourceResolver);
+    resolver.start(mockListener);
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener).onError(errorCaptor.capture());
+    Status errorStatus = errorCaptor.getValue();
+    assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
+    assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr");
+    verify(mockResourceResolver, never()).resolveTxt(anyString());
   }
 
   @Test
-  public void resolveAll_presentResourceResolver() throws Exception {
+  public void resolve_serviceConfigLookupFails_nullServiceConfig() throws Exception {
-    final String hostname = "addr.fake";
+    DnsNameResolver.enableTxt = true;
-    final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001);
+    InetAddress backendAddr = InetAddresses.fromInteger(0x7f000001);
-    final EquivalentAddressGroup balancerAddr = new EquivalentAddressGroup(new SocketAddress() {});
-
     AddressResolver mockAddressResolver = mock(AddressResolver.class);
     when(mockAddressResolver.resolveAddress(anyString()))
-        .thenReturn(Collections.<InetAddress>singletonList(backendAddr));
+        .thenReturn(Collections.singletonList(backendAddr));
+    String name = "foo.googleapis.com";
     ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
     when(mockResourceResolver.resolveTxt(anyString()))
-        .thenReturn(Collections.singletonList("service config"));
-    when(mockResourceResolver.resolveSrv(ArgumentMatchers.any(AddressResolver.class), anyString()))
-        .thenReturn(Collections.singletonList(balancerAddr));
-    boolean resovleSrv = true;
-    boolean resolveTxt = true;
-
-    ResolutionResults res = DnsNameResolver.resolveAll(
-        mockAddressResolver, mockResourceResolver, resovleSrv, resolveTxt, hostname);
-    assertThat(res.addresses).containsExactly(backendAddr);
-    assertThat(res.balancerAddresses).containsExactly(balancerAddr);
-    assertThat(res.txtRecords).containsExactly("service config");
-    verify(mockAddressResolver).resolveAddress(hostname);
-    verify(mockResourceResolver).resolveTxt("_grpc_config." + hostname);
-    verify(mockResourceResolver).resolveSrv(mockAddressResolver, "_grpclb._tcp." + hostname);
-  }
-
-  @Test
-  public void resolveAll_onlyBalancers() throws Exception {
-    String hostname = "addr.fake";
-    EquivalentAddressGroup balancerAddr = new EquivalentAddressGroup(new SocketAddress() {});
-
-    AddressResolver mockAddressResolver = mock(AddressResolver.class);
-    when(mockAddressResolver.resolveAddress(anyString()))
-        .thenThrow(new UnknownHostException("I really tried"));
-    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
-    when(mockResourceResolver.resolveTxt(anyString()))
-        .thenReturn(Collections.<String>emptyList());
-    when(mockResourceResolver.resolveSrv(ArgumentMatchers.any(AddressResolver.class), anyString()))
-        .thenReturn(Collections.singletonList(balancerAddr));
-    boolean resovleSrv = true;
-    boolean resolveTxt = true;
-
-    ResolutionResults res = DnsNameResolver.resolveAll(
-        mockAddressResolver, mockResourceResolver, resovleSrv, resolveTxt, hostname);
-    assertThat(res.addresses).isEmpty();
-    assertThat(res.balancerAddresses).containsExactly(balancerAddr);
-    assertThat(res.txtRecords).isEmpty();
-    verify(mockAddressResolver).resolveAddress(hostname);
-    verify(mockResourceResolver).resolveTxt("_grpc_config." + hostname);
-    verify(mockResourceResolver).resolveSrv(mockAddressResolver, "_grpclb._tcp." + hostname);
-  }
-
-  @Test
-  public void resolveAll_balancerLookupFails() throws Exception {
-    final String hostname = "addr.fake";
-    final Inet4Address backendAddr = InetAddresses.fromInteger(0x7f000001);
-    AddressResolver mockAddressResolver = mock(AddressResolver.class);
-    when(mockAddressResolver.resolveAddress(anyString()))
-        .thenReturn(Collections.<InetAddress>singletonList(backendAddr));
-    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
-    when(mockResourceResolver.resolveTxt(anyString()))
-        .thenReturn(Collections.singletonList("service config"));
-    when(mockResourceResolver.resolveSrv(ArgumentMatchers.any(AddressResolver.class), anyString()))
         .thenThrow(new Exception("something like javax.naming.NamingException"));
-    boolean resovleSrv = true;
-    boolean resolveTxt = true;
 
-    ResolutionResults res = DnsNameResolver.resolveAll(
+    DnsNameResolver resolver = newResolver(name, 81);
-        mockAddressResolver, mockResourceResolver, resovleSrv, resolveTxt, hostname);
+    resolver.setAddressResolver(mockAddressResolver);
-    assertThat(res.addresses).containsExactly(backendAddr);
+    resolver.setResourceResolver(mockResourceResolver);
-    assertThat(res.balancerAddresses).isEmpty();
+    resolver.start(mockListener);
-    assertThat(res.txtRecords).containsExactly("service config");
+    assertEquals(1, fakeExecutor.runDueTasks());
-    verify(mockAddressResolver).resolveAddress(hostname);
+    verify(mockListener).onResult(resultCaptor.capture());
-    verify(mockResourceResolver).resolveTxt("_grpc_config." + hostname);
+    ResolutionResult result = resultCaptor.getValue();
-    verify(mockResourceResolver).resolveSrv(mockAddressResolver, "_grpclb._tcp." + hostname);
+    InetSocketAddress resolvedBackendAddr =
+        (InetSocketAddress) Iterables.getOnlyElement(
+            Iterables.getOnlyElement(result.getAddresses()).getAddresses());
+    assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
+    verify(mockAddressResolver).resolveAddress(name);
+    assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
+    assertThat(result.getServiceConfig()).isNull();
+    verify(mockResourceResolver).resolveTxt(anyString());
+  }
+
+  @Test
+  public void resolve_serviceConfigMalformed_serviceConfigError() throws Exception {
+    DnsNameResolver.enableTxt = true;
+    InetAddress backendAddr = InetAddresses.fromInteger(0x7f000001);
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
+    when(mockAddressResolver.resolveAddress(anyString()))
+        .thenReturn(Collections.singletonList(backendAddr));
+    String name = "foo.googleapis.com";
+    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
+    when(mockResourceResolver.resolveTxt(anyString()))
+        .thenReturn(Collections.singletonList("grpc_config=something invalid"));
+
+    DnsNameResolver resolver = newResolver(name, 81);
+    resolver.setAddressResolver(mockAddressResolver);
+    resolver.setResourceResolver(mockResourceResolver);
+    resolver.start(mockListener);
+    assertEquals(1, fakeExecutor.runDueTasks());
+    verify(mockListener).onResult(resultCaptor.capture());
+    ResolutionResult result = resultCaptor.getValue();
+    InetSocketAddress resolvedBackendAddr =
+        (InetSocketAddress) Iterables.getOnlyElement(
+            Iterables.getOnlyElement(result.getAddresses()).getAddresses());
+    assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
+    verify(mockAddressResolver).resolveAddress(name);
+    assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
+    assertThat(result.getServiceConfig()).isNotNull();
+    assertThat(result.getServiceConfig().getError()).isNotNull();
+    verify(mockResourceResolver).resolveTxt(anyString());
   }
 
   @Test

core/src/test/java/io/grpc/internal/JndiResourceResolverTest.java

@@ -21,16 +21,10 @@ import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import io.grpc.Attributes;
+import io.grpc.internal.DnsNameResolver.SrvRecord;
-import io.grpc.EquivalentAddressGroup;
-import io.grpc.internal.DnsNameResolver.AddressResolver;
 import io.grpc.internal.JndiResourceResolverFactory.JndiRecordFetcher;
 import io.grpc.internal.JndiResourceResolverFactory.JndiResourceResolver;
 import io.grpc.internal.JndiResourceResolverFactory.RecordFetcher;
-import java.net.InetAddress;
-import java.net.InetSocketAddress;
-import java.net.SocketAddress;
-import java.net.UnknownHostException;
 import java.util.Arrays;
 import java.util.List;
 import org.junit.Assume;
@@ -83,33 +77,15 @@ public class JndiResourceResolverTest {
   @SuppressWarnings("deprecation")
   @Test
   public void srvRecordLookup() throws Exception {
-    AddressResolver addressResolver = mock(AddressResolver.class);
-    when(addressResolver.resolveAddress("foo.example.com."))
-        .thenReturn(Arrays.asList(InetAddress.getByName("127.1.2.3")));
-    when(addressResolver.resolveAddress("bar.example.com."))
-        .thenReturn(Arrays.asList(
-            InetAddress.getByName("127.3.2.1"), InetAddress.getByName("::1")));
-    when(addressResolver.resolveAddress("unknown.example.com."))
-        .thenThrow(new UnknownHostException("unknown.example.com."));
     RecordFetcher recordFetcher = mock(RecordFetcher.class);
     when(recordFetcher.getAllRecords("SRV", "dns:///service.example.com"))
         .thenReturn(Arrays.asList(
-            "0 0 314 foo.example.com.", "0 0 42 bar.example.com.", "0 0 1 unknown.example.com."));
+            "0 0 314 foo.example.com.", "0 0 42 bar.example.com.", "0 0 1 discard.example.com"));
 
-    List<EquivalentAddressGroup> golden = Arrays.asList(
+    List<SrvRecord> golden = Arrays.asList(
-        new EquivalentAddressGroup(
+        new SrvRecord("foo.example.com.", 314),
-            Arrays.<SocketAddress>asList(new InetSocketAddress("127.1.2.3", 314)),
+        new SrvRecord("bar.example.com.", 42));
-            Attributes.newBuilder()
-              .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "foo.example.com")
-              .build()),
-        new EquivalentAddressGroup(
-            Arrays.<SocketAddress>asList(
-                new InetSocketAddress("127.3.2.1", 42),
-                new InetSocketAddress("::1", 42)),
-            Attributes.newBuilder()
-              .set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "bar.example.com")
-              .build()));
     JndiResourceResolver resolver = new JndiResourceResolver(recordFetcher);
-    assertThat(resolver.resolveSrv(addressResolver, "service.example.com")).isEqualTo(golden);
+    assertThat(resolver.resolveSrv("service.example.com")).isEqualTo(golden);
   }
 }

grpclb/src/main/java/io/grpc/grpclb/GrpclbConstants.java

@@ -42,9 +42,11 @@ public final class GrpclbConstants {
   static final Attributes.Key<String> TOKEN_ATTRIBUTE_KEY =
       Attributes.Key.create("lb-token");
 
-  @SuppressWarnings("deprecation")
+  /**
+   * Attribute key for gRPC LB server addresses.
+   */
   static final Attributes.Key<List<EquivalentAddressGroup>> ATTR_LB_ADDRS =
-      io.grpc.internal.GrpcAttributes.ATTR_LB_ADDRS;
+      Attributes.Key.create("io.grpc.grpclb.lbAddrs");
 
   @SuppressWarnings("deprecation")
   @EquivalentAddressGroup.Attr

grpclb/src/main/java/io/grpc/grpclb/GrpclbNameResolver.java

@@ -0,0 +1,142 @@
+/*
+ * Copyright 2020 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.grpclb;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Stopwatch;
+import io.grpc.Attributes;
+import io.grpc.EquivalentAddressGroup;
+import io.grpc.NameResolver;
+import io.grpc.internal.DnsNameResolver;
+import io.grpc.internal.SharedResourceHolder.Resource;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.Executor;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import javax.annotation.Nullable;
+
+/**
+ * A DNS-based {@link NameResolver} with gRPC LB specific add-ons for resolving balancer
+ * addresses via service records.
+ *
+ * @see SecretGrpclbNameResolverProvider
+ */
+final class GrpclbNameResolver extends DnsNameResolver {
+
+  private static final Logger logger = Logger.getLogger(GrpclbNameResolver.class.getName());
+
+  // From https://github.com/grpc/proposal/blob/master/A5-grpclb-in-dns.md
+  private static final String GRPCLB_NAME_PREFIX = "_grpclb._tcp.";
+
+  GrpclbNameResolver(
+      @Nullable String nsAuthority,
+      String name,
+      Args args,
+      Resource<Executor> executorResource,
+      Stopwatch stopwatch,
+      boolean isAndroid) {
+    super(nsAuthority, name, args, executorResource, stopwatch, isAndroid);
+  }
+
+  @Override
+  protected InternalResolutionResult doResolve(boolean forceTxt) {
+    List<EquivalentAddressGroup> balancerAddrs = resolveBalancerAddresses();
+    InternalResolutionResult result = super.doResolve(!balancerAddrs.isEmpty());
+    if (!balancerAddrs.isEmpty()) {
+      result.attributes =
+          Attributes.newBuilder()
+              .set(GrpclbConstants.ATTR_LB_ADDRS, balancerAddrs)
+              .build();
+    }
+    return result;
+  }
+
+  private List<EquivalentAddressGroup> resolveBalancerAddresses() {
+    List<SrvRecord> srvRecords = Collections.emptyList();
+    Exception srvRecordsException = null;
+    ResourceResolver resourceResolver = getResourceResolver();
+    if (resourceResolver != null) {
+      try {
+        srvRecords = resourceResolver.resolveSrv(GRPCLB_NAME_PREFIX + getHost());
+      } catch (Exception e) {
+        srvRecordsException = e;
+      }
+    }
+    List<EquivalentAddressGroup> balancerAddresses = new ArrayList<>(srvRecords.size());
+    Exception balancerAddressesException = null;
+    Level level = Level.WARNING;
+    for (SrvRecord record : srvRecords) {
+      try {
+        // Strip trailing dot for appearance's sake. It _should_ be fine either way, but most
+        // people expect to see it without the dot.
+        String authority = record.host.substring(0, record.host.length() - 1);
+        // But we want to use the trailing dot for the IP lookup. The dot makes the name absolute
+        // instead of relative and so will avoid the search list like that in resolv.conf.
+        List<? extends InetAddress> addrs = addressResolver.resolveAddress(record.host);
+        List<SocketAddress> sockAddrs = new ArrayList<>(addrs.size());
+        for (InetAddress addr : addrs) {
+          sockAddrs.add(new InetSocketAddress(addr, record.port));
+        }
+        Attributes attrs =
+            Attributes.newBuilder()
+                .set(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY, authority)
+                .build();
+        balancerAddresses.add(
+            new EquivalentAddressGroup(Collections.unmodifiableList(sockAddrs), attrs));
+      } catch (Exception e) {
+        logger.log(level, "Can't find address for SRV record " + record, e);
+        if (balancerAddressesException == null) {
+          balancerAddressesException = e;
+          level = Level.FINE;
+        }
+      }
+    }
+    if (srvRecordsException != null
+        || (balancerAddressesException != null && balancerAddresses.isEmpty())) {
+      logger.log(Level.FINE, "Balancer resolution failure", srvRecordsException);
+    }
+    return Collections.unmodifiableList(balancerAddresses);
+  }
+
+  @VisibleForTesting
+  @Override
+  protected void setAddressResolver(AddressResolver addressResolver) {
+    super.setAddressResolver(addressResolver);
+  }
+
+  @VisibleForTesting
+  @Override
+  protected void setResourceResolver(ResourceResolver resourceResolver) {
+    super.setResourceResolver(resourceResolver);
+  }
+
+  @VisibleForTesting
+  @Override
+  protected String getHost() {
+    return super.getHost();
+  }
+
+  @VisibleForTesting
+  static void setEnableTxt(boolean enableTxt) {
+    DnsNameResolver.enableTxt = enableTxt;
+  }
+}

grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java

@@ -16,10 +16,16 @@
 
 package io.grpc.grpclb;
 
-import io.grpc.internal.BaseDnsNameResolverProvider;
+import com.google.common.base.Preconditions;
+import com.google.common.base.Stopwatch;
+import io.grpc.InternalServiceProviders;
+import io.grpc.NameResolver.Args;
+import io.grpc.NameResolverProvider;
+import io.grpc.internal.GrpcUtil;
+import java.net.URI;
 
 /**
- * A provider for {@code io.grpc.internal.DnsNameResolver} for gRPC lb.
+ * A provider for {@code io.grpc.grpclb.GrpclbNameResolver}.
  *
  * <p>It resolves a target URI whose scheme is {@code "dns"}. The (optional) authority of the target
  * URI is reserved for the address of alternative DNS server (not implemented yet). The path of the
@@ -32,9 +38,6 @@ import io.grpc.internal.BaseDnsNameResolverProvider;
  *   yet))</li>
  *   <li>{@code "dns:///foo.googleapis.com"} (without port)</li>
  * </ul>
- *
- * <p>Note: the main difference between {@code io.grpc.DnsNameResolver} is service record is enabled
- * by default.
  */
 // Make it package-private so that it cannot be directly referenced by users.  Java service loader
 // requires the provider to be public, but we can hide it under a package-private class.
@@ -42,14 +45,39 @@ final class SecretGrpclbNameResolverProvider {
 
   private SecretGrpclbNameResolverProvider() {}
 
-  public static final class Provider extends BaseDnsNameResolverProvider {
+  public static final class Provider extends NameResolverProvider {
 
-    private static final boolean SRV_ENABLED =
+    private static final String SCHEME = "dns";
-        Boolean.parseBoolean(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME, "true"));
 
     @Override
-    protected boolean isSrvEnabled() {
+    public GrpclbNameResolver newNameResolver(URI targetUri, Args args) {
-      return SRV_ENABLED;
+      if (SCHEME.equals(targetUri.getScheme())) {
+        String targetPath = Preconditions.checkNotNull(targetUri.getPath(), "targetPath");
+        Preconditions.checkArgument(
+            targetPath.startsWith("/"),
+            "the path component (%s) of the target (%s) must start with '/'",
+            targetPath, targetUri);
+        String name = targetPath.substring(1);
+        return new GrpclbNameResolver(
+            targetUri.getAuthority(),
+            name,
+            args,
+            GrpcUtil.SHARED_CHANNEL_EXECUTOR,
+            Stopwatch.createUnstarted(),
+            InternalServiceProviders.isAndroid(getClass().getClassLoader()));
+      } else {
+        return null;
+      }
+    }
+
+    @Override
+    public String getDefaultScheme() {
+      return SCHEME;
+    }
+
+    @Override
+    protected boolean isAvailable() {
+      return true;
     }
 
     @Override

grpclb/src/test/java/io/grpc/grpclb/GrpclbNameResolverTest.java

@@ -0,0 +1,337 @@
+/*
+ * Copyright 2020 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.grpclb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.google.common.collect.Iterables;
+import io.grpc.Attributes;
+import io.grpc.ChannelLogger;
+import io.grpc.EquivalentAddressGroup;
+import io.grpc.NameResolver;
+import io.grpc.NameResolver.ConfigOrError;
+import io.grpc.NameResolver.ResolutionResult;
+import io.grpc.NameResolver.ServiceConfigParser;
+import io.grpc.Status;
+import io.grpc.Status.Code;
+import io.grpc.SynchronizationContext;
+import io.grpc.internal.DnsNameResolver.AddressResolver;
+import io.grpc.internal.DnsNameResolver.ResourceResolver;
+import io.grpc.internal.DnsNameResolver.SrvRecord;
+import io.grpc.internal.FakeClock;
+import io.grpc.internal.GrpcUtil;
+import io.grpc.internal.SharedResourceHolder.Resource;
+import java.io.IOException;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.UnknownHostException;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.Executor;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+import org.mockito.stubbing.Answer;
+
+/** Unit tests for {@link GrpclbNameResolver}. */
+@RunWith(JUnit4.class)
+public class GrpclbNameResolverTest {
+
+  @Rule
+  public final MockitoRule mocks = MockitoJUnit.rule();
+
+  private static final String NAME = "foo.googleapis.com";
+  private static final int DEFAULT_PORT = 887;
+
+  private final SynchronizationContext syncContext = new SynchronizationContext(
+      new Thread.UncaughtExceptionHandler() {
+        @Override
+        public void uncaughtException(Thread t, Throwable e) {
+          throw new AssertionError(e);
+        }
+      });
+
+  private final FakeClock fakeClock = new FakeClock();
+  private final FakeExecutorResource fakeExecutorResource = new FakeExecutorResource();
+
+  private final class FakeExecutorResource implements Resource<Executor> {
+
+    @Override
+    public Executor create() {
+      return fakeClock.getScheduledExecutorService();
+    }
+
+    @Override
+    public void close(Executor instance) {}
+  }
+
+  @Captor private ArgumentCaptor<ResolutionResult> resultCaptor;
+  @Captor private ArgumentCaptor<Status> errorCaptor;
+  @Mock private ServiceConfigParser serviceConfigParser;
+  @Mock private NameResolver.Listener2 mockListener;
+
+  private GrpclbNameResolver resolver;
+  private String hostName;
+
+  @Before
+  public void setUp() {
+    GrpclbNameResolver.setEnableTxt(true);
+    NameResolver.Args args =
+        NameResolver.Args.newBuilder()
+            .setDefaultPort(DEFAULT_PORT)
+            .setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR)
+            .setSynchronizationContext(syncContext)
+            .setServiceConfigParser(serviceConfigParser)
+            .setChannelLogger(mock(ChannelLogger.class))
+            .build();
+    resolver =
+        new GrpclbNameResolver(
+            null, NAME, args, fakeExecutorResource, fakeClock.getStopwatchSupplier().get(),
+            /* isAndroid */false);
+    hostName = resolver.getHost();
+    assertThat(hostName).isEqualTo(NAME);
+  }
+
+  @Test
+  public void resolve_emptyResult() {
+    resolver.setAddressResolver(new AddressResolver() {
+      @Override
+      public List<InetAddress> resolveAddress(String host) throws Exception {
+        return Collections.emptyList();
+      }
+    });
+    resolver.setResourceResolver(new ResourceResolver() {
+      @Override
+      public List<String> resolveTxt(String host) throws Exception {
+        return Collections.emptyList();
+      }
+
+      @Override
+      public List<SrvRecord> resolveSrv(String host) throws Exception {
+        return Collections.emptyList();
+      }
+    });
+
+    resolver.start(mockListener);
+    assertThat(fakeClock.runDueTasks()).isEqualTo(1);
+
+    verify(mockListener).onResult(resultCaptor.capture());
+    ResolutionResult result = resultCaptor.getValue();
+    assertThat(result.getAddresses()).isEmpty();
+    assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
+    assertThat(result.getServiceConfig()).isNull();
+  }
+
+  @Test
+  public void resolve_presentResourceResolver() throws Exception {
+    InetAddress backendAddr = InetAddress.getByAddress(new byte[] {127, 0, 0, 0});
+    InetAddress lbAddr = InetAddress.getByAddress(new byte[] {10, 1, 0, 0});
+    int lbPort = 8080;
+    String lbName = "foo.example.com.";  // original name in SRV record
+    SrvRecord srvRecord = new SrvRecord(lbName, 8080);
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
+    when(mockAddressResolver.resolveAddress(hostName))
+        .thenReturn(Collections.singletonList(backendAddr));
+    when(mockAddressResolver.resolveAddress(lbName))
+        .thenReturn(Collections.singletonList(lbAddr));
+    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
+    when(mockResourceResolver.resolveTxt(anyString()))
+        .thenReturn(
+            Collections.singletonList(
+                "grpc_config=[{\"clientLanguage\": [\"java\"], \"serviceConfig\": {}}]"));
+    when(mockResourceResolver.resolveSrv(anyString()))
+        .thenReturn(Collections.singletonList(srvRecord));
+    when(serviceConfigParser.parseServiceConfig(ArgumentMatchers.<String, Object>anyMap()))
+        .thenAnswer(new Answer<ConfigOrError>() {
+          @Override
+          public ConfigOrError answer(InvocationOnMock invocation) {
+            Object[] args = invocation.getArguments();
+            return ConfigOrError.fromConfig(args[0]);
+          }
+        });
+
+    resolver.setAddressResolver(mockAddressResolver);
+    resolver.setResourceResolver(mockResourceResolver);
+
+    resolver.start(mockListener);
+    assertThat(fakeClock.runDueTasks()).isEqualTo(1);
+    verify(mockListener).onResult(resultCaptor.capture());
+    ResolutionResult result = resultCaptor.getValue();
+    InetSocketAddress resolvedBackendAddr =
+        (InetSocketAddress) Iterables.getOnlyElement(
+            Iterables.getOnlyElement(result.getAddresses()).getAddresses());
+    assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
+    EquivalentAddressGroup resolvedBalancerAddr =
+        Iterables.getOnlyElement(result.getAttributes().get(GrpclbConstants.ATTR_LB_ADDRS));
+    assertThat(resolvedBalancerAddr.getAttributes().get(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY))
+        .isEqualTo("foo.example.com");
+    InetSocketAddress resolvedBalancerSockAddr =
+        (InetSocketAddress) Iterables.getOnlyElement(resolvedBalancerAddr.getAddresses());
+    assertThat(resolvedBalancerSockAddr.getAddress()).isEqualTo(lbAddr);
+    assertThat(resolvedBalancerSockAddr.getPort()).isEqualTo(lbPort);
+    assertThat(result.getServiceConfig().getConfig()).isNotNull();
+    verify(mockAddressResolver).resolveAddress(hostName);
+    verify(mockResourceResolver).resolveTxt("_grpc_config." + hostName);
+    verify(mockResourceResolver).resolveSrv("_grpclb._tcp." + hostName);
+  }
+
+  @Test
+  public void resolve_nullResourceResolver() throws Exception {
+    InetAddress backendAddr = InetAddress.getByAddress(new byte[] {127, 0, 0, 0});
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
+    when(mockAddressResolver.resolveAddress(anyString()))
+        .thenReturn(Collections.singletonList(backendAddr));
+    ResourceResolver resourceResolver = null;
+
+    resolver.setAddressResolver(mockAddressResolver);
+    resolver.setResourceResolver(resourceResolver);
+
+    resolver.start(mockListener);
+    assertThat(fakeClock.runDueTasks()).isEqualTo(1);
+    verify(mockListener).onResult(resultCaptor.capture());
+    ResolutionResult result = resultCaptor.getValue();
+    assertThat(result.getAddresses())
+        .containsExactly(
+            new EquivalentAddressGroup(new InetSocketAddress(backendAddr, DEFAULT_PORT)));
+    assertThat(result.getAttributes()).isEqualTo(Attributes.EMPTY);
+    assertThat(result.getServiceConfig()).isNull();
+  }
+
+  @Test
+  public void resolve_nullResourceResolver_addressFailure() throws Exception {
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
+    when(mockAddressResolver.resolveAddress(anyString())).thenThrow(new IOException("no addr"));
+    ResourceResolver resourceResolver = null;
+
+    resolver.setAddressResolver(mockAddressResolver);
+    resolver.setResourceResolver(resourceResolver);
+
+    resolver.start(mockListener);
+    assertThat(fakeClock.runDueTasks()).isEqualTo(1);
+    verify(mockListener).onError(errorCaptor.capture());
+    Status errorStatus = errorCaptor.getValue();
+    assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
+    assertThat(errorStatus.getCause()).hasMessageThat().contains("no addr");
+  }
+
+  @Test
+  public void resolve_addressFailure_stillLookUpBalancersAndServiceConfig() throws Exception {
+    InetAddress lbAddr = InetAddress.getByAddress(new byte[] {10, 1, 0, 0});
+    int lbPort = 8080;
+    String lbName = "foo.example.com.";  // original name in SRV record
+    SrvRecord srvRecord = new SrvRecord(lbName, 8080);
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
+    when(mockAddressResolver.resolveAddress(hostName))
+        .thenThrow(new UnknownHostException("I really tried"));
+    when(mockAddressResolver.resolveAddress(lbName))
+        .thenReturn(Collections.singletonList(lbAddr));
+    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
+    when(mockResourceResolver.resolveTxt(anyString())).thenReturn(Collections.<String>emptyList());
+    when(mockResourceResolver.resolveSrv(anyString()))
+        .thenReturn(Collections.singletonList(srvRecord));
+
+    resolver.setAddressResolver(mockAddressResolver);
+    resolver.setResourceResolver(mockResourceResolver);
+
+    resolver.start(mockListener);
+    assertThat(fakeClock.runDueTasks()).isEqualTo(1);
+    verify(mockListener).onResult(resultCaptor.capture());
+    ResolutionResult result = resultCaptor.getValue();
+    assertThat(result.getAddresses()).isEmpty();
+    EquivalentAddressGroup resolvedBalancerAddr =
+        Iterables.getOnlyElement(result.getAttributes().get(GrpclbConstants.ATTR_LB_ADDRS));
+    assertThat(resolvedBalancerAddr.getAttributes().get(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY))
+        .isEqualTo("foo.example.com");
+    InetSocketAddress resolvedBalancerSockAddr =
+        (InetSocketAddress) Iterables.getOnlyElement(resolvedBalancerAddr.getAddresses());
+    assertThat(resolvedBalancerSockAddr.getAddress()).isEqualTo(lbAddr);
+    assertThat(resolvedBalancerSockAddr.getPort()).isEqualTo(lbPort);
+    assertThat(result.getServiceConfig()).isNull();
+    verify(mockAddressResolver).resolveAddress(hostName);
+    verify(mockResourceResolver).resolveTxt("_grpc_config." + hostName);
+    verify(mockResourceResolver).resolveSrv("_grpclb._tcp." + hostName);
+  }
+
+  @Test
+  public void resolveAll_balancerLookupFails_stillLookUpServiceConfig() throws Exception {
+    InetAddress backendAddr = InetAddress.getByAddress(new byte[] {127, 0, 0, 0});
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
+    when(mockAddressResolver.resolveAddress(hostName))
+        .thenReturn(Collections.singletonList(backendAddr));
+    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
+    when(mockResourceResolver.resolveTxt(anyString()))
+        .thenReturn(Collections.<String>emptyList());
+    when(mockResourceResolver.resolveSrv(anyString()))
+        .thenThrow(new Exception("something like javax.naming.NamingException"));
+
+    resolver.setAddressResolver(mockAddressResolver);
+    resolver.setResourceResolver(mockResourceResolver);
+
+    resolver.start(mockListener);
+    assertThat(fakeClock.runDueTasks()).isEqualTo(1);
+    verify(mockListener).onResult(resultCaptor.capture());
+    ResolutionResult result = resultCaptor.getValue();
+
+    InetSocketAddress resolvedBackendAddr =
+        (InetSocketAddress) Iterables.getOnlyElement(
+            Iterables.getOnlyElement(result.getAddresses()).getAddresses());
+    assertThat(resolvedBackendAddr.getAddress()).isEqualTo(backendAddr);
+    assertThat(result.getAttributes().get(GrpclbConstants.ATTR_LB_ADDRS)).isNull();
+    verify(mockAddressResolver).resolveAddress(hostName);
+    verify(mockResourceResolver).resolveTxt("_grpc_config." + hostName);
+    verify(mockResourceResolver).resolveSrv("_grpclb._tcp." + hostName);
+  }
+
+  @Test
+  public void resolve_addressAndBalancersLookupFail_neverLookupServiceConfig() throws Exception {
+    AddressResolver mockAddressResolver = mock(AddressResolver.class);
+    when(mockAddressResolver.resolveAddress(anyString()))
+        .thenThrow(new UnknownHostException("I really tried"));
+    ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
+    lenient().when(mockResourceResolver.resolveTxt(anyString()))
+        .thenThrow(new Exception("something like javax.naming.NamingException"));
+    when(mockResourceResolver.resolveSrv(anyString()))
+        .thenThrow(new Exception("something like javax.naming.NamingException"));
+
+    resolver.setAddressResolver(mockAddressResolver);
+    resolver.setResourceResolver(mockResourceResolver);
+
+    resolver.start(mockListener);
+    assertThat(fakeClock.runDueTasks()).isEqualTo(1);
+    verify(mockListener).onError(errorCaptor.capture());
+    Status errorStatus = errorCaptor.getValue();
+    assertThat(errorStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
+    verify(mockAddressResolver).resolveAddress(hostName);
+    verify(mockResourceResolver, never()).resolveTxt("_grpc_config." + hostName);
+    verify(mockResourceResolver).resolveSrv("_grpclb._tcp." + hostName);
+  }
+}

grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java

@@ -17,34 +17,92 @@
 package io.grpc.grpclb;
 
 import static com.google.common.truth.Truth.assertThat;
-import static io.grpc.internal.BaseDnsNameResolverProvider.ENABLE_GRPCLB_PROPERTY_NAME;
+import static org.junit.Assert.fail;
-import static org.junit.Assume.assumeTrue;
+import static org.mockito.Mockito.mock;
 
+import io.grpc.ChannelLogger;
+import io.grpc.NameResolver;
+import io.grpc.NameResolver.ServiceConfigParser;
+import io.grpc.SynchronizationContext;
 import io.grpc.internal.DnsNameResolverProvider;
+import io.grpc.internal.GrpcUtil;
+import java.net.URI;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
+/** Unit tests for {@link SecretGrpclbNameResolverProvider}. */
 @RunWith(JUnit4.class)
 public class SecretGrpclbNameResolverProviderTest {
 
+  private final SynchronizationContext syncContext = new SynchronizationContext(
+      new Thread.UncaughtExceptionHandler() {
+        @Override
+        public void uncaughtException(Thread t, Throwable e) {
+          throw new AssertionError(e);
+        }
+      });
+  private final NameResolver.Args args = NameResolver.Args.newBuilder()
+      .setDefaultPort(8080)
+      .setProxyDetector(GrpcUtil.DEFAULT_PROXY_DETECTOR)
+      .setSynchronizationContext(syncContext)
+      .setServiceConfigParser(mock(ServiceConfigParser.class))
+      .setChannelLogger(mock(ChannelLogger.class))
+      .build();
+
+  private SecretGrpclbNameResolverProvider.Provider provider =
+      new SecretGrpclbNameResolverProvider.Provider();
+
+  @Test
+  public void isAvailable() {
+    assertThat(provider.isAvailable()).isTrue();
+  }
+
   @Test
   public void priority_shouldBeHigherThanDefaultDnsNameResolver() {
     DnsNameResolverProvider defaultDnsNameResolver = new DnsNameResolverProvider();
-    SecretGrpclbNameResolverProvider.Provider grpclbDnsNameResolver =
-        new SecretGrpclbNameResolverProvider.Provider();
 
-    assertThat(defaultDnsNameResolver.priority())
+    assertThat(provider.priority()).isGreaterThan(defaultDnsNameResolver.priority());
-        .isLessThan(grpclbDnsNameResolver.priority());
   }
 
   @Test
-  public void isSrvEnabled_trueByDefault() {
+  public void newNameResolver() {
-    assumeTrue(System.getProperty(ENABLE_GRPCLB_PROPERTY_NAME) == null);
+    assertThat(provider.newNameResolver(URI.create("dns:///localhost:443"), args))
-
+        .isInstanceOf(GrpclbNameResolver.class);
-    SecretGrpclbNameResolverProvider.Provider grpclbDnsNameResolver =
+    assertThat(provider.newNameResolver(URI.create("notdns:///localhost:443"), args)).isNull();
-        new SecretGrpclbNameResolverProvider.Provider();
-
-    assertThat(grpclbDnsNameResolver.isSrvEnabled()).isTrue();
   }
-}
+
+  @Test
+  public void invalidDnsName() throws Exception {
+    testInvalidUri(new URI("dns", null, "/[invalid]", null));
+  }
+
+  @Test
+  public void validIpv6() throws Exception {
+    testValidUri(new URI("dns", null, "/[::1]", null));
+  }
+
+  @Test
+  public void validDnsNameWithoutPort() throws Exception {
+    testValidUri(new URI("dns", null, "/foo.googleapis.com", null));
+  }
+
+  @Test
+  public void validDnsNameWithPort() throws Exception {
+    testValidUri(new URI("dns", null, "/foo.googleapis.com:456", null));
+  }
+
+  private void testInvalidUri(URI uri) {
+    try {
+      provider.newNameResolver(uri, args);
+      fail("Should have failed");
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+  }
+
+  private void testValidUri(URI uri) {
+    GrpclbNameResolver resolver = provider.newNameResolver(uri, args);
+    assertThat(resolver).isNotNull();
+  }
+}