From 6a2b1aeabebe823373952cd6a8c39e33eb131da5 Mon Sep 17 00:00:00 2001 From: kshyanashree <109167932+kshyanashree@users.noreply.github.com> Date: Wed, 7 Sep 2022 11:43:42 -0700 Subject: [PATCH] Add profiler task for calling a credential helper. (#16237) This might help debug future performance issues related caused by slow credential helpers. Closes #16226. PiperOrigin-RevId: 472683449 Change-Id: Ib1f51a51763ccfa0e18be980fd4b4d77954a9814 Co-authored-by: Tiago Quelhas --- .../lib/authandtls/credentialhelper/BUILD | 1 + .../credentialhelper/CredentialHelper.java | 92 ++++++++++--------- .../build/lib/profiler/ProfilerTask.java | 1 + 3 files changed, 52 insertions(+), 42 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD index 20500030d42356..62a58d7a82eea4 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD +++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/BUILD @@ -15,6 +15,7 @@ java_library( srcs = glob(["*.java"]), deps = [ "//src/main/java/com/google/devtools/build/lib/events", + "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/vfs", "//third_party:auth", diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java index db9b43b1aef925..2219a595741fa4 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/credentialhelper/CredentialHelper.java @@ -14,12 +14,15 @@ package com.google.devtools.build.lib.authandtls.credentialhelper; +import static com.google.devtools.build.lib.profiler.ProfilerTask.CREDENTIAL_HELPER; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.io.CharStreams; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.shell.Subprocess; import com.google.devtools.build.lib.shell.SubprocessBuilder; import com.google.devtools.build.lib.vfs.Path; @@ -68,57 +71,62 @@ public GetCredentialsResponse getCredentials(CredentialHelperEnvironment environ Preconditions.checkNotNull(environment); Preconditions.checkNotNull(uri); - Subprocess process = spawnSubprocess(environment, "get"); - try (Reader stdout = new InputStreamReader(process.getInputStream(), UTF_8); - Reader stderr = new InputStreamReader(process.getErrorStream(), UTF_8)) { - try (Writer stdin = new OutputStreamWriter(process.getOutputStream(), UTF_8)) { - GSON.toJson(GetCredentialsRequest.newBuilder().setUri(uri).build(), stdin); - } + Profiler prof = Profiler.instance(); - process.waitFor(); - if (process.timedout()) { - throw new CredentialHelperException( - String.format( - Locale.US, - "Failed to get credentials for '%s' from helper '%s': process timed out", - uri, - path)); - } - if (process.exitValue() != 0) { - throw new CredentialHelperException( - String.format( - Locale.US, - "Failed to get credentials for '%s' from helper '%s': process exited with code %d." - + " stderr: %s", - uri, - path, - process.exitValue(), - CharStreams.toString(stderr))); - } + try (SilentCloseable c = prof.profile(CREDENTIAL_HELPER, "calling credential helper")) { + Subprocess process = spawnSubprocess(environment, "get"); + try (Reader stdout = new InputStreamReader(process.getInputStream(), UTF_8); + Reader stderr = new InputStreamReader(process.getErrorStream(), UTF_8)) { + try (Writer stdin = new OutputStreamWriter(process.getOutputStream(), UTF_8)) { + GSON.toJson(GetCredentialsRequest.newBuilder().setUri(uri).build(), stdin); + } + + process.waitFor(); - try { - GetCredentialsResponse response = GSON.fromJson(stdout, GetCredentialsResponse.class); - if (response == null) { + if (process.timedout()) { throw new CredentialHelperException( String.format( Locale.US, - "Failed to get credentials for '%s' from helper '%s': process exited without" - + " output. stderr: %s", + "Failed to get credentials for '%s' from helper '%s': process timed out", + uri, + path)); + } + if (process.exitValue() != 0) { + throw new CredentialHelperException( + String.format( + Locale.US, + "Failed to get credentials for '%s' from helper '%s': process exited with code" + + " %d. stderr: %s", uri, path, + process.exitValue(), CharStreams.toString(stderr))); } - return response; - } catch (JsonSyntaxException e) { - throw new CredentialHelperException( - String.format( - Locale.US, - "Failed to get credentials for '%s' from helper '%s': error parsing output. stderr:" - + " %s", - uri, - path, - CharStreams.toString(stderr)), - e); + + try { + GetCredentialsResponse response = GSON.fromJson(stdout, GetCredentialsResponse.class); + if (response == null) { + throw new CredentialHelperException( + String.format( + Locale.US, + "Failed to get credentials for '%s' from helper '%s': process exited without" + + " output. stderr: %s", + uri, + path, + CharStreams.toString(stderr))); + } + return response; + } catch (JsonSyntaxException e) { + throw new CredentialHelperException( + String.format( + Locale.US, + "Failed to get credentials for '%s' from helper '%s': error parsing output." + + " stderr: %s", + uri, + path, + CharStreams.toString(stderr)), + e); + } } } } diff --git a/src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java b/src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java index 1dc82313d19da1..1c7785b930b84d 100644 --- a/src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java +++ b/src/main/java/com/google/devtools/build/lib/profiler/ProfilerTask.java @@ -84,6 +84,7 @@ public enum ProfilerTask { WORKER_BORROW("borrowing a worker"), WORKER_WORKING("waiting for response from worker"), WORKER_COPYING_OUTPUTS("copying outputs from worker"), + CREDENTIAL_HELPER("calling credential helper"), UNKNOWN("Unknown event"); private static class Threshold {