From cfbec1714398d7e963ff50bb79b850b63f451ebb Mon Sep 17 00:00:00 2001 From: Tom Gianos Date: Thu, 13 May 2021 20:34:24 -0700 Subject: [PATCH] Tracing Improvements - Flag new job creation with `genie.job.new=true` tag - Tag job id at initial job save layer - Standardize tags further - Move command and cluster tagging into job resolution - Remove unnecessary init span in agent now that we've further refined how this data is going to be processed downstream --- .../genie/agent/cli/GenieAgentRunner.java | 25 ++---- .../agent/cli/GenieAgentRunnerSpec.groovy | 83 +++++++++++-------- .../internal/tracing/TracingConstants.java | 40 +++++++++ ...PersistenceServiceIntegrationTestBase.java | 6 +- .../impl/jpa/JpaPersistenceServiceImpl.java | 37 ++++++--- .../services/impl/JobLaunchServiceImpl.java | 5 -- .../services/impl/JobResolverServiceImpl.java | 70 +++++++++++++--- .../data/DataAutoConfiguration.java | 11 ++- .../services/ServicesAutoConfiguration.java | 19 +++-- .../impl/JobLaunchServiceImplSpec.groovy | 17 +--- .../impl/JobResolverServiceImplSpec.groovy | 66 +++++++++------ ...ersistenceServiceImplApplicationsTest.java | 4 +- ...JpaPersistenceServiceImplClustersTest.java | 4 +- ...JpaPersistenceServiceImplCommandsTest.java | 4 +- .../JpaPersistenceServiceImplJobsTest.java | 16 ++-- 15 files changed, 259 insertions(+), 148 deletions(-) diff --git a/genie-agent/src/main/java/com/netflix/genie/agent/cli/GenieAgentRunner.java b/genie-agent/src/main/java/com/netflix/genie/agent/cli/GenieAgentRunner.java index 5a84bacf611..38f3f082a37 100644 --- a/genie-agent/src/main/java/com/netflix/genie/agent/cli/GenieAgentRunner.java +++ b/genie-agent/src/main/java/com/netflix/genie/agent/cli/GenieAgentRunner.java @@ -46,9 +46,7 @@ @Slf4j public class GenieAgentRunner implements CommandLineRunner, ExitCodeGenerator { - static final String INIT_SPAN_NAME = "genie-agent-init"; static final String RUN_SPAN_NAME = "genie-agent-run"; - static final String COMMAND_NAME_TAG = TracingConstants.AGENT_TAG_BASE + ".command.name"; private final ArgumentParser argumentParser; private final CommandFactory commandFactory; @@ -122,7 +120,7 @@ private void internalRun(final String[] args) { } else if (!availableCommands.contains(commandName)) { throw new IllegalArgumentException("Invalid command -- commands available: " + availableCommandsString); } - this.tagAdapter.tag(span, COMMAND_NAME_TAG, commandName); + this.tagAdapter.tag(span, TracingConstants.AGENT_CLI_COMMAND_NAME_TAG, commandName); ConsoleLog.getLogger().info("Preparing agent to execute command: '{}'", commandName); @@ -147,19 +145,14 @@ public int getExitCode() { private ScopedSpan initializeTracing() { // Attempt to extract any existing trace information from the environment final Optional existingTraceContext = this.tracePropagator.extract(System.getenv()); - final ScopedSpan initSpan = existingTraceContext.isPresent() - ? this.tracer.startScopedSpanWithParent(INIT_SPAN_NAME, existingTraceContext.get()) - : this.tracer.startScopedSpan(INIT_SPAN_NAME); + final ScopedSpan runSpan = existingTraceContext.isPresent() + ? this.tracer.startScopedSpanWithParent(RUN_SPAN_NAME, existingTraceContext.get()) + : this.tracer.startScopedSpan(RUN_SPAN_NAME); // Quickly create and report an initial span - final TraceContext initContext = initSpan.context(); - try { - final String traceId = initContext.traceIdString(); - log.info("Trace ID: {}", traceId); - ConsoleLog.getLogger().info("Trace ID: {}", traceId); - } finally { - initSpan.finish(); - } - // Create a new span that will represent the potentially long running action the agent will execute - return this.tracer.startScopedSpanWithParent(RUN_SPAN_NAME, initContext); + final TraceContext initContext = runSpan.context(); + final String traceId = initContext.traceIdString(); + log.info("Trace ID: {}", traceId); + ConsoleLog.getLogger().info("Trace ID: {}", traceId); + return runSpan; } } diff --git a/genie-agent/src/test/groovy/com/netflix/genie/agent/cli/GenieAgentRunnerSpec.groovy b/genie-agent/src/test/groovy/com/netflix/genie/agent/cli/GenieAgentRunnerSpec.groovy index 279c761ff2a..04f12f96c97 100644 --- a/genie-agent/src/test/groovy/com/netflix/genie/agent/cli/GenieAgentRunnerSpec.groovy +++ b/genie-agent/src/test/groovy/com/netflix/genie/agent/cli/GenieAgentRunnerSpec.groovy @@ -21,6 +21,7 @@ import brave.ScopedSpan import brave.Tracer import brave.propagation.TraceContext import com.beust.jcommander.ParameterException +import com.netflix.genie.common.internal.tracing.TracingConstants import com.netflix.genie.common.internal.tracing.brave.BraveTagAdapter import com.netflix.genie.common.internal.tracing.brave.BraveTracePropagator import com.netflix.genie.common.internal.tracing.brave.BraveTracingCleanup @@ -40,7 +41,6 @@ class GenieAgentRunnerSpec extends Specification { BraveTracePropagator tracePropagator BraveTracingCleanup traceCleaner BraveTagAdapter tagAdapter - ScopedSpan initSpan ScopedSpan runSpan TraceContext initContext @@ -54,7 +54,6 @@ class GenieAgentRunnerSpec extends Specification { this.traceCleaner = Mock(BraveTracingCleanup) this.tagAdapter = Mock(BraveTagAdapter) this.args = new String[0] - this.initSpan = Mock(ScopedSpan) this.runSpan = Mock(ScopedSpan) this.runner = new GenieAgentRunner( this.argsParser, @@ -77,14 +76,16 @@ class GenieAgentRunnerSpec extends Specification { then: 1 * this.tracePropagator.extract(_ as Map) >> Optional.empty() 0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext) - 1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan - 1 * this.initSpan.context() >> this.initContext - 1 * this.initSpan.finish() - 1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan + 1 * this.tracer.startScopedSpan(GenieAgentRunner.RUN_SPAN_NAME) >> this.runSpan + 1 * this.runSpan.context() >> this.initContext 1 * this.runSpan.finish() 1 * this.traceCleaner.cleanup() 1 * this.tracer.currentSpanCustomizer() >> this.runSpan - 1 * this.tagAdapter.tag(this.runSpan, GenieAgentRunner.COMMAND_NAME_TAG, TestCommands.ExampleCommand1.NAME) + 1 * this.tagAdapter.tag( + this.runSpan, + TracingConstants.AGENT_CLI_COMMAND_NAME_TAG, + TestCommands.ExampleCommand1.NAME + ) 1 * this.argsParser.parse(this.args) 1 * this.argsParser.getSelectedCommand() >> TestCommands.ExampleCommand1.NAME 1 * this.argsParser.getCommandNames() >> TestCommands.allCommandNames() @@ -104,15 +105,17 @@ class GenieAgentRunnerSpec extends Specification { then: 1 * this.tracePropagator.extract(_ as Map) >> Optional.empty() 0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext) - 1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan - 1 * this.initSpan.context() >> this.initContext - 1 * this.initSpan.finish() - 1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan + 1 * this.tracer.startScopedSpan(GenieAgentRunner.RUN_SPAN_NAME) >> this.runSpan + 1 * this.runSpan.context() >> this.initContext 1 * this.runSpan.error(_ as Throwable) 1 * this.runSpan.finish() 1 * this.traceCleaner.cleanup() 1 * this.tracer.currentSpanCustomizer() >> this.runSpan - 0 * this.tagAdapter.tag(this.runSpan, GenieAgentRunner.COMMAND_NAME_TAG, TestCommands.ExampleCommand1.NAME) + 0 * this.tagAdapter.tag( + this.runSpan, + TracingConstants.AGENT_CLI_COMMAND_NAME_TAG, + TestCommands.ExampleCommand1.NAME + ) 1 * this.argsParser.parse(this.args) >> { throw exception } 0 * this.argsParser.getSelectedCommand() >> TestCommands.ExampleCommand1.NAME 0 * this.argsParser.getCommandNames() >> TestCommands.allCommandNames() @@ -130,15 +133,17 @@ class GenieAgentRunnerSpec extends Specification { then: 1 * this.tracePropagator.extract(_ as Map) >> Optional.empty() 0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext) - 1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan - 1 * this.initSpan.context() >> this.initContext - 1 * this.initSpan.finish() - 1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan - 1 * this.runSpan.error(_ as Throwable) + 1 * this.tracer.startScopedSpan(GenieAgentRunner.RUN_SPAN_NAME) >> this.runSpan + 1 * this.runSpan.context() >> this.initContext 1 * this.runSpan.finish() + 1 * this.runSpan.error(_ as Throwable) 1 * this.traceCleaner.cleanup() 1 * this.tracer.currentSpanCustomizer() >> this.runSpan - 0 * this.tagAdapter.tag(this.runSpan, GenieAgentRunner.COMMAND_NAME_TAG, TestCommands.ExampleCommand1.NAME) + 0 * this.tagAdapter.tag( + this.runSpan, + TracingConstants.AGENT_CLI_COMMAND_NAME_TAG, + TestCommands.ExampleCommand1.NAME + ) 1 * this.argsParser.parse(this.args) 1 * this.argsParser.getSelectedCommand() >> null 1 * this.argsParser.getCommandNames() >> TestCommands.allCommandNames() @@ -156,15 +161,17 @@ class GenieAgentRunnerSpec extends Specification { then: 1 * this.tracePropagator.extract(_ as Map) >> Optional.empty() 0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext) - 1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan - 1 * this.initSpan.context() >> this.initContext - 1 * this.initSpan.finish() - 1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan - 1 * this.runSpan.error(_ as Throwable) + 1 * this.tracer.startScopedSpan(GenieAgentRunner.RUN_SPAN_NAME) >> this.runSpan + 1 * this.runSpan.context() >> this.initContext 1 * this.runSpan.finish() + 1 * this.runSpan.error(_ as Throwable) 1 * this.traceCleaner.cleanup() 1 * this.tracer.currentSpanCustomizer() >> this.runSpan - 0 * this.tagAdapter.tag(this.runSpan, GenieAgentRunner.COMMAND_NAME_TAG, "foo") + 0 * this.tagAdapter.tag( + this.runSpan, + TracingConstants.AGENT_CLI_COMMAND_NAME_TAG, + "foo" + ) 1 * this.argsParser.parse(this.args) 1 * this.argsParser.getSelectedCommand() >> "foo" 1 * this.argsParser.getCommandNames() >> TestCommands.allCommandNames() @@ -182,15 +189,17 @@ class GenieAgentRunnerSpec extends Specification { then: 1 * this.tracePropagator.extract(_ as Map) >> Optional.empty() 0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext) - 1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan - 1 * this.initSpan.context() >> this.initContext - 1 * this.initSpan.finish() - 1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan - 1 * this.runSpan.error(_ as Throwable) + 1 * this.tracer.startScopedSpan(GenieAgentRunner.RUN_SPAN_NAME) >> this.runSpan + 1 * this.runSpan.context() >> this.initContext 1 * this.runSpan.finish() + 1 * this.runSpan.error(_ as Throwable) 1 * this.traceCleaner.cleanup() 1 * this.tracer.currentSpanCustomizer() >> this.runSpan - 1 * this.tagAdapter.tag(this.runSpan, GenieAgentRunner.COMMAND_NAME_TAG, TestCommands.ExampleCommand1.NAME) + 1 * this.tagAdapter.tag( + this.runSpan, + TracingConstants.AGENT_CLI_COMMAND_NAME_TAG, + TestCommands.ExampleCommand1.NAME + ) 1 * this.argsParser.parse(this.args) 1 * this.argsParser.getSelectedCommand() >> TestCommands.ExampleCommand1.NAME 1 * this.argsParser.getCommandNames() >> TestCommands.allCommandNames() @@ -208,15 +217,17 @@ class GenieAgentRunnerSpec extends Specification { then: 1 * this.tracePropagator.extract(_ as Map) >> Optional.empty() 0 * this.tracer.startScopedSpanWithParent(_ as String, _ as TraceContext) - 1 * this.tracer.startScopedSpan(GenieAgentRunner.INIT_SPAN_NAME) >> this.initSpan - 1 * this.initSpan.context() >> this.initContext - 1 * this.initSpan.finish() - 1 * this.tracer.startScopedSpanWithParent(GenieAgentRunner.RUN_SPAN_NAME, this.initContext) >> this.runSpan - 1 * this.runSpan.error(_ as Throwable) + 1 * this.tracer.startScopedSpan(GenieAgentRunner.RUN_SPAN_NAME) >> this.runSpan + 1 * this.runSpan.context() >> this.initContext 1 * this.runSpan.finish() + 1 * this.runSpan.error(_ as Throwable) 1 * this.traceCleaner.cleanup() 1 * this.tracer.currentSpanCustomizer() >> this.runSpan - 1 * this.tagAdapter.tag(this.runSpan, GenieAgentRunner.COMMAND_NAME_TAG, TestCommands.ExampleCommand1.NAME) + 1 * this.tagAdapter.tag( + this.runSpan, + TracingConstants.AGENT_CLI_COMMAND_NAME_TAG, + TestCommands.ExampleCommand1.NAME + ) 1 * argsParser.parse(args) 1 * argsParser.getSelectedCommand() >> TestCommands.ExampleCommand1.NAME 1 * argsParser.getCommandNames() >> TestCommands.allCommandNames() diff --git a/genie-common-internal/src/main/java/com/netflix/genie/common/internal/tracing/TracingConstants.java b/genie-common-internal/src/main/java/com/netflix/genie/common/internal/tracing/TracingConstants.java index d3f06f01380..d6a91f07b08 100644 --- a/genie-common-internal/src/main/java/com/netflix/genie/common/internal/tracing/TracingConstants.java +++ b/genie-common-internal/src/main/java/com/netflix/genie/common/internal/tracing/TracingConstants.java @@ -35,6 +35,11 @@ public final class TracingConstants { */ public static final String AGENT_TAG_BASE = GLOBAL_TAG_BASE + ".agent"; + /** + * The command that was entered on the CLI for the agent to execute. + */ + public static final String AGENT_CLI_COMMAND_NAME_TAG = AGENT_TAG_BASE + "cli.command.name"; + /** * The root for all tags related to spans occurring in the Genie server. */ @@ -50,11 +55,46 @@ public final class TracingConstants { */ public static final String JOB_ID_TAG = JOB_TAG_BASE + ".id"; + /** + * The tag to represent that this span contains a new job submission. + */ + public static final String NEW_JOB_TAG = JOB_TAG_BASE + ".new"; + /** * The tag for the job name. */ public static final String JOB_NAME_TAG = JOB_TAG_BASE + ".name"; + /** + * The tag for the job user. + */ + public static final String JOB_USER_TAG = JOB_TAG_BASE + ".user"; + + /** + * The tag for the job command id. + */ + public static final String JOB_CLUSTER_ID_TAG = JOB_TAG_BASE + ".cluster.id"; + + /** + * The tag for the job command id. + */ + public static final String JOB_CLUSTER_NAME_TAG = JOB_TAG_BASE + ".cluster.name"; + + /** + * The tag for the job command id. + */ + public static final String JOB_COMMAND_ID_TAG = JOB_TAG_BASE + ".command.id"; + + /** + * The tag for the job command id. + */ + public static final String JOB_COMMAND_NAME_TAG = JOB_TAG_BASE + ".command.name"; + + /** + * Convenience constant for representing a flag tag with a value of {@literal true}. + */ + public static final String TRUE_VALUE = "true"; + private TracingConstants() { } } diff --git a/genie-web/src/integTest/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceIntegrationTestBase.java b/genie-web/src/integTest/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceIntegrationTestBase.java index b89d16e70bc..9fff039eb85 100644 --- a/genie-web/src/integTest/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceIntegrationTestBase.java +++ b/genie-web/src/integTest/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceIntegrationTestBase.java @@ -18,6 +18,7 @@ package com.netflix.genie.web.data.services.impl.jpa; import com.github.springtestdbunit.TransactionDbUnitTestExecutionListener; +import com.netflix.genie.common.internal.spring.autoconfigure.CommonTracingAutoConfiguration; import com.netflix.genie.web.data.observers.PersistedJobStatusObserver; import com.netflix.genie.web.data.services.impl.jpa.repositories.JpaApplicationRepository; import com.netflix.genie.web.data.services.impl.jpa.repositories.JpaClusterRepository; @@ -34,6 +35,7 @@ import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; import org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager; import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.annotation.Import; import org.springframework.test.context.TestExecutionListeners; import org.springframework.test.context.support.DependencyInjectionTestExecutionListener; @@ -54,7 +56,9 @@ @Import( { DataAutoConfiguration.class, - ValidationAutoConfiguration.class + ValidationAutoConfiguration.class, + TraceAutoConfiguration.class, + CommonTracingAutoConfiguration.class } ) @MockBean( diff --git a/genie-web/src/main/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImpl.java b/genie-web/src/main/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImpl.java index b7e52b8ff88..04d33c3cbe7 100644 --- a/genie-web/src/main/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImpl.java +++ b/genie-web/src/main/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImpl.java @@ -17,6 +17,8 @@ */ package com.netflix.genie.web.data.services.impl.jpa; +import brave.SpanCustomizer; +import brave.Tracer; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.NullNode; import com.google.common.annotations.VisibleForTesting; @@ -62,6 +64,9 @@ import com.netflix.genie.common.internal.exceptions.unchecked.GenieInvalidStatusException; import com.netflix.genie.common.internal.exceptions.unchecked.GenieJobAlreadyClaimedException; import com.netflix.genie.common.internal.exceptions.unchecked.GenieRuntimeException; +import com.netflix.genie.common.internal.tracing.TracingConstants; +import com.netflix.genie.common.internal.tracing.brave.BraveTagAdapter; +import com.netflix.genie.common.internal.tracing.brave.BraveTracingComponents; import com.netflix.genie.web.data.services.PersistenceService; import com.netflix.genie.web.data.services.impl.jpa.converters.EntityV3DtoConverters; import com.netflix.genie.web.data.services.impl.jpa.converters.EntityV4DtoConverters; @@ -129,7 +134,6 @@ import javax.validation.constraints.Size; import java.net.URI; import java.time.Instant; -import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -201,15 +205,20 @@ public class JpaPersistenceServiceImpl implements PersistenceService { private final JpaJobRepository jobRepository; private final JpaTagRepository tagRepository; + private final Tracer tracer; + private final BraveTagAdapter tagAdapter; + /** * Constructor. * - * @param entityManager The {@link EntityManager} to use - * @param jpaRepositories All the repositories in the Genie application + * @param entityManager The {@link EntityManager} to use + * @param jpaRepositories All the repositories in the Genie application + * @param tracingComponents All the Brave related tracing components needed to add metadata to Spans */ public JpaPersistenceServiceImpl( final EntityManager entityManager, - final JpaRepositories jpaRepositories + final JpaRepositories jpaRepositories, + final BraveTracingComponents tracingComponents ) { this.entityManager = entityManager; this.applicationRepository = jpaRepositories.getApplicationRepository(); @@ -219,6 +228,9 @@ public JpaPersistenceServiceImpl( this.fileRepository = jpaRepositories.getFileRepository(); this.jobRepository = jpaRepositories.getJobRepository(); this.tagRepository = jpaRepositories.getTagRepository(); + + this.tracer = tracingComponents.getTracer(); + this.tagAdapter = tracingComponents.getTagAdapter(); } //region Application APIs @@ -1540,9 +1552,7 @@ public long deleteJobsCreatedBefore( */ @Override @Nonnull - public String saveJobSubmission( - @Valid final JobSubmission jobSubmission - ) throws IdAlreadyExistsException { + public String saveJobSubmission(@Valid final JobSubmission jobSubmission) throws IdAlreadyExistsException { log.debug("[saveJobSubmission] Attempting to save job submission {}", jobSubmission); // TODO: Metrics final JobEntity jobEntity = new JobEntity(); @@ -1585,6 +1595,9 @@ public String saveJobSubmission( jobSubmission, id ); + final SpanCustomizer spanCustomizer = this.addJobIdTag(id); + // This is a new job so add flag representing that fact + this.tagAdapter.tag(spanCustomizer, TracingConstants.NEW_JOB_TAG, TracingConstants.TRUE_VALUE); return id; } catch (final DataIntegrityViolationException e) { throw new IdAlreadyExistsException( @@ -2720,10 +2733,6 @@ private void setExecutionResources( job.setApplications(applications); } - private int toTimeoutUsed(final Instant started, final Instant timeout) { - return (int) started.until(timeout, ChronoUnit.SECONDS); - } - private Optional getEntityOrNullForFindJobs( final JpaBaseRepository repository, final String id, @@ -2742,5 +2751,11 @@ private Optional getEntityOrNullForFindJobs( return optionalEntity; } + + private SpanCustomizer addJobIdTag(final String jobId) { + final SpanCustomizer spanCustomizer = this.tracer.currentSpanCustomizer(); + this.tagAdapter.tag(spanCustomizer, TracingConstants.JOB_ID_TAG, jobId); + return spanCustomizer; + } //endregion } diff --git a/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobLaunchServiceImpl.java b/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobLaunchServiceImpl.java index 04448f04804..9aa3a9daefc 100644 --- a/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobLaunchServiceImpl.java +++ b/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobLaunchServiceImpl.java @@ -26,8 +26,6 @@ import com.netflix.genie.common.external.dtos.v4.JobStatus; import com.netflix.genie.common.internal.exceptions.checked.GenieJobResolutionException; import com.netflix.genie.common.internal.exceptions.unchecked.GenieInvalidStatusException; -import com.netflix.genie.common.internal.tracing.TracingConstants; -import com.netflix.genie.common.internal.tracing.brave.BraveTagAdapter; import com.netflix.genie.common.internal.tracing.brave.BraveTracingComponents; import com.netflix.genie.web.agent.launchers.AgentLauncher; import com.netflix.genie.web.data.services.DataServices; @@ -84,7 +82,6 @@ public class JobLaunchServiceImpl implements JobLaunchService { private final JobResolverService jobResolverService; private final AgentLauncherSelector agentLauncherSelector; private final Tracer tracer; - private final BraveTagAdapter tagAdapter; private final MeterRegistry registry; /** @@ -107,7 +104,6 @@ public JobLaunchServiceImpl( this.jobResolverService = jobResolverService; this.agentLauncherSelector = agentLauncherSelector; this.tracer = tracingComponents.getTracer(); - this.tagAdapter = tracingComponents.getTagAdapter(); this.registry = registry; } @@ -139,7 +135,6 @@ public String launchJob( */ final String jobId = this.persistenceService.saveJobSubmission(jobSubmission); span.annotate(SAVED_JOB_SUBMISSION_ANNOTATION); - this.tagAdapter.tag(span, TracingConstants.JOB_ID_TAG, jobId); final ResolvedJob resolvedJob; try { diff --git a/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobResolverServiceImpl.java b/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobResolverServiceImpl.java index 91316a85d19..c16afa62f5e 100644 --- a/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobResolverServiceImpl.java +++ b/genie-web/src/main/java/com/netflix/genie/web/services/impl/JobResolverServiceImpl.java @@ -17,6 +17,8 @@ */ package com.netflix.genie.web.services.impl; +import brave.SpanCustomizer; +import brave.Tracer; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -28,12 +30,16 @@ import com.netflix.genie.common.external.dtos.v4.Command; import com.netflix.genie.common.external.dtos.v4.Criterion; import com.netflix.genie.common.external.dtos.v4.JobEnvironment; +import com.netflix.genie.common.external.dtos.v4.JobMetadata; import com.netflix.genie.common.external.dtos.v4.JobRequest; import com.netflix.genie.common.external.dtos.v4.JobSpecification; import com.netflix.genie.common.external.dtos.v4.JobStatus; import com.netflix.genie.common.internal.exceptions.checked.GenieJobResolutionException; import com.netflix.genie.common.internal.exceptions.unchecked.GenieJobResolutionRuntimeException; import com.netflix.genie.common.internal.jobs.JobConstants; +import com.netflix.genie.common.internal.tracing.TracingConstants; +import com.netflix.genie.common.internal.tracing.brave.BraveTagAdapter; +import com.netflix.genie.common.internal.tracing.brave.BraveTracingComponents; import com.netflix.genie.web.data.services.DataServices; import com.netflix.genie.web.data.services.PersistenceService; import com.netflix.genie.web.dtos.ResolvedJob; @@ -147,6 +153,8 @@ public class JobResolverServiceImpl implements JobResolverService { // TODO: Switch to path private final File defaultJobDirectory; private final String defaultArchiveLocation; + private final Tracer tracer; + private final BraveTagAdapter tagAdapter; //endregion //region Public APIs @@ -154,12 +162,13 @@ public class JobResolverServiceImpl implements JobResolverService { /** * Constructor. * - * @param dataServices The {@link DataServices} encapsulation instance to use - * @param clusterSelectors The {@link ClusterSelector} implementations to use - * @param commandSelector The {@link CommandSelector} implementation to use - * @param registry The {@link MeterRegistry }metrics repository to use - * @param jobsProperties The properties for running a job set by the user - * @param environment The Spring application {@link Environment} for dynamic property resolution + * @param dataServices The {@link DataServices} encapsulation instance to use + * @param clusterSelectors The {@link ClusterSelector} implementations to use + * @param commandSelector The {@link CommandSelector} implementation to use + * @param registry The {@link MeterRegistry }metrics repository to use + * @param jobsProperties The properties for running a job set by the user + * @param environment The Spring application {@link Environment} for dynamic property resolution + * @param tracingComponents The {@link BraveTracingComponents} instance to use */ public JobResolverServiceImpl( final DataServices dataServices, @@ -167,7 +176,8 @@ public JobResolverServiceImpl( final CommandSelector commandSelector, // TODO: For now this is a single value but maybe support List final MeterRegistry registry, final JobsProperties jobsProperties, - final Environment environment + final Environment environment, + final BraveTracingComponents tracingComponents ) { this.persistenceService = dataServices.getPersistenceService(); this.clusterSelectors = clusterSelectors; @@ -183,6 +193,10 @@ public JobResolverServiceImpl( // Metrics this.registry = registry; + + // tracing + this.tracer = tracingComponents.getTracer(); + this.tagAdapter = tracingComponents.getTagAdapter(); } /** @@ -208,7 +222,12 @@ public ResolvedJob resolveJob( // TODO: Possible improvement to combine this query with a few others to save DB trips but for now... final boolean apiJob = this.persistenceService.isApiJob(id); - final JobResolutionContext context = new JobResolutionContext(id, jobRequest, apiJob); + final JobResolutionContext context = new JobResolutionContext( + id, + jobRequest, + apiJob, + this.tracer.currentSpanCustomizer() + ); final ResolvedJob resolvedJob = this.resolve(context); @@ -256,7 +275,12 @@ public ResolvedJob resolveJob( jobRequest ); - final JobResolutionContext context = new JobResolutionContext(id, jobRequest, apiJob); + final JobResolutionContext context = new JobResolutionContext( + id, + jobRequest, + apiJob, + this.tracer.currentSpanCustomizer() + ); final ResolvedJob resolvedJob = this.resolve(context); MetricsUtils.addSuccessTags(tags); @@ -279,6 +303,7 @@ public ResolvedJob resolveJob( private ResolvedJob resolve( final JobResolutionContext context ) throws GenieJobResolutionException, GenieJobResolutionRuntimeException { + this.tagSpanWithJobMetadata(context); this.resolveCommand(context); this.resolveCluster(context); this.resolveApplications(context); @@ -383,8 +408,13 @@ private void resolveCommand(final JobResolutionContext context) throws GenieJobR ); MetricsUtils.addSuccessTags(tags); - tags.add(Tag.of(MetricsConstants.TagKeys.COMMAND_ID, command.getId())); - tags.add(Tag.of(MetricsConstants.TagKeys.COMMAND_NAME, command.getMetadata().getName())); + final String commandId = command.getId(); + final String commandName = command.getMetadata().getName(); + tags.add(Tag.of(MetricsConstants.TagKeys.COMMAND_ID, commandId)); + tags.add(Tag.of(MetricsConstants.TagKeys.COMMAND_NAME, commandName)); + final SpanCustomizer spanCustomizer = context.getSpanCustomizer(); + this.tagAdapter.tag(spanCustomizer, TracingConstants.JOB_COMMAND_ID_TAG, commandId); + this.tagAdapter.tag(spanCustomizer, TracingConstants.JOB_COMMAND_NAME_TAG, commandName); context.setCommand(command); } catch (final GenieJobResolutionException e) { // No candidates or selector choose none @@ -506,8 +536,13 @@ private void resolveCluster(final JobResolutionContext context) throws GenieJobR context.setCluster(cluster); MetricsUtils.addSuccessTags(tags); - tags.add(Tag.of(MetricsConstants.TagKeys.CLUSTER_ID, cluster.getId())); - tags.add(Tag.of(MetricsConstants.TagKeys.CLUSTER_NAME, cluster.getMetadata().getName())); + final String clusterId = cluster.getId(); + final String clusterName = cluster.getMetadata().getName(); + tags.add(Tag.of(MetricsConstants.TagKeys.CLUSTER_ID, clusterId)); + tags.add(Tag.of(MetricsConstants.TagKeys.CLUSTER_NAME, clusterName)); + final SpanCustomizer spanCustomizer = context.getSpanCustomizer(); + this.tagAdapter.tag(spanCustomizer, TracingConstants.JOB_CLUSTER_ID_TAG, clusterId); + this.tagAdapter.tag(spanCustomizer, TracingConstants.JOB_CLUSTER_NAME_TAG, clusterName); } catch (final GenieJobResolutionException e) { tags.add(NO_CLUSTER_RESOLVED_ID); tags.add(NO_CLUSTER_RESOLVED_NAME); @@ -885,6 +920,14 @@ private String getProxyObjectClassName(final Object possibleProxyObject) { } return className; } + + private void tagSpanWithJobMetadata(final JobResolutionContext context) { + final SpanCustomizer spanCustomizer = this.tracer.currentSpanCustomizer(); + this.tagAdapter.tag(spanCustomizer, TracingConstants.JOB_ID_TAG, context.getJobId()); + final JobMetadata jobMetadata = context.getJobRequest().getMetadata(); + this.tagAdapter.tag(spanCustomizer, TracingConstants.JOB_NAME_TAG, jobMetadata.getName()); + this.tagAdapter.tag(spanCustomizer, TracingConstants.JOB_USER_TAG, jobMetadata.getUser()); + } //endregion //region Helper Classes @@ -903,6 +946,7 @@ static class JobResolutionContext { private final String jobId; private final JobRequest jobRequest; private final boolean apiJob; + private final SpanCustomizer spanCustomizer; private Command command; private Cluster cluster; diff --git a/genie-web/src/main/java/com/netflix/genie/web/spring/autoconfigure/data/DataAutoConfiguration.java b/genie-web/src/main/java/com/netflix/genie/web/spring/autoconfigure/data/DataAutoConfiguration.java index aa51cf968a4..e495f987aa8 100644 --- a/genie-web/src/main/java/com/netflix/genie/web/spring/autoconfigure/data/DataAutoConfiguration.java +++ b/genie-web/src/main/java/com/netflix/genie/web/spring/autoconfigure/data/DataAutoConfiguration.java @@ -17,6 +17,7 @@ */ package com.netflix.genie.web.spring.autoconfigure.data; +import com.netflix.genie.common.internal.tracing.brave.BraveTracingComponents; import com.netflix.genie.web.data.services.DataServices; import com.netflix.genie.web.data.services.PersistenceService; import com.netflix.genie.web.data.services.impl.jpa.JpaPersistenceServiceImpl; @@ -96,8 +97,9 @@ public JpaRepositories genieJpaRepositories( /** * Provide a default implementation of {@link PersistenceService} if no other has been defined. * - * @param entityManager The {@link EntityManager} for this application - * @param jpaRepositories The {@link JpaRepositories} for Genie + * @param entityManager The {@link EntityManager} for this application + * @param jpaRepositories The {@link JpaRepositories} for Genie + * @param tracingComponents The {@link BraveTracingComponents} instance to use * @return A {@link JpaPersistenceServiceImpl} instance which implements {@link PersistenceService} backed by * JPA and a relational database */ @@ -105,8 +107,9 @@ public JpaRepositories genieJpaRepositories( @ConditionalOnMissingBean(PersistenceService.class) public JpaPersistenceServiceImpl geniePersistenceService( final EntityManager entityManager, - final JpaRepositories jpaRepositories + final JpaRepositories jpaRepositories, + final BraveTracingComponents tracingComponents ) { - return new JpaPersistenceServiceImpl(entityManager, jpaRepositories); + return new JpaPersistenceServiceImpl(entityManager, jpaRepositories, tracingComponents); } } diff --git a/genie-web/src/main/java/com/netflix/genie/web/spring/autoconfigure/services/ServicesAutoConfiguration.java b/genie-web/src/main/java/com/netflix/genie/web/spring/autoconfigure/services/ServicesAutoConfiguration.java index db454530d57..b7d67c28087 100644 --- a/genie-web/src/main/java/com/netflix/genie/web/spring/autoconfigure/services/ServicesAutoConfiguration.java +++ b/genie-web/src/main/java/com/netflix/genie/web/spring/autoconfigure/services/ServicesAutoConfiguration.java @@ -142,12 +142,13 @@ public AttachmentService attachmentService( /** * Get an implementation of {@link JobResolverService} if one hasn't already been defined. * - * @param dataServices The {@link DataServices} encapsulation instance to use - * @param clusterSelectors The {@link ClusterSelector} implementations to use - * @param commandSelector The {@link CommandSelector} implementation to use - * @param registry The metrics repository to use - * @param jobsProperties The properties for running a job set by the user - * @param environment The Spring application {@link Environment} for dynamic property resolution + * @param dataServices The {@link DataServices} encapsulation instance to use + * @param clusterSelectors The {@link ClusterSelector} implementations to use + * @param commandSelector The {@link CommandSelector} implementation to use + * @param registry The metrics repository to use + * @param jobsProperties The properties for running a job set by the user + * @param environment The Spring application {@link Environment} for dynamic property resolution + * @param tracingComponents The {@link BraveTracingComponents} to use * @return A {@link JobResolverServiceImpl} instance */ @Bean @@ -158,7 +159,8 @@ public JobResolverServiceImpl jobResolverService( final CommandSelector commandSelector, final MeterRegistry registry, final JobsProperties jobsProperties, - final Environment environment + final Environment environment, + final BraveTracingComponents tracingComponents ) { return new JobResolverServiceImpl( dataServices, @@ -166,7 +168,8 @@ public JobResolverServiceImpl jobResolverService( commandSelector, registry, jobsProperties, - environment + environment, + tracingComponents ); } diff --git a/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/JobLaunchServiceImplSpec.groovy b/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/JobLaunchServiceImplSpec.groovy index ee7f1b23725..77f4a89482a 100644 --- a/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/JobLaunchServiceImplSpec.groovy +++ b/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/JobLaunchServiceImplSpec.groovy @@ -28,7 +28,6 @@ import com.netflix.genie.common.external.dtos.v4.JobStatus import com.netflix.genie.common.internal.exceptions.checked.GenieJobResolutionException import com.netflix.genie.common.internal.exceptions.unchecked.GenieInvalidStatusException import com.netflix.genie.common.internal.exceptions.unchecked.GenieJobResolutionRuntimeException -import com.netflix.genie.common.internal.tracing.TracingConstants import com.netflix.genie.common.internal.tracing.brave.BraveTagAdapter import com.netflix.genie.common.internal.tracing.brave.BraveTracePropagator import com.netflix.genie.common.internal.tracing.brave.BraveTracingCleanup @@ -62,7 +61,6 @@ class JobLaunchServiceImplSpec extends Specification { JobResolverService jobResolverService AgentLauncherSelector agentLauncherSelector Tracer tracer - BraveTagAdapter tagAdapter SpanCustomizer span JobLaunchServiceImpl service @@ -71,7 +69,6 @@ class JobLaunchServiceImplSpec extends Specification { this.jobResolverService = Mock(JobResolverService) this.agentLauncherSelector = Mock(AgentLauncherSelector) this.tracer = Mock(Tracer) - this.tagAdapter = Mock(BraveTagAdapter) this.span = Mock(SpanCustomizer) def dataServices = Mock(DataServices) { getPersistenceService() >> this.persistenceService @@ -84,7 +81,7 @@ class JobLaunchServiceImplSpec extends Specification { this.tracer, Mock(BraveTracePropagator), Mock(BraveTracingCleanup), - this.tagAdapter + Mock(BraveTagAdapter) ), new SimpleMeterRegistry() ) @@ -94,7 +91,6 @@ class JobLaunchServiceImplSpec extends Specification { def "Successful launch (requestedLauncherExt: #requestedLauncherExt launcherExt: #launcherExt)"() { def agentLauncher = Mock(AgentLauncher) def jobId = UUID.randomUUID().toString() - def jobName = UUID.randomUUID().toString() def resolvedJob = Mock(ResolvedJob) def jobSubmission = Mock(JobSubmission) def jobRequest = Mock(JobRequest) @@ -108,7 +104,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.tracer.currentSpanCustomizer() >> this.span 1 * this.span.annotate(JobLaunchServiceImpl.BEGIN_LAUNCH_JOB_ANNOTATION) 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> jobId - 1 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 1 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 1 * this.jobResolverService.resolveJob(jobId) >> resolvedJob 1 * this.span.annotate(JobLaunchServiceImpl.RESOLVED_JOB_ANNOTATION) @@ -163,7 +158,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> { throw new IllegalStateException("fail") } - 0 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 0 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 0 * this.jobResolverService.resolveJob(_ as String) 0 * this.persistenceService.updateJobStatus(jobId, JobStatus.RESOLVED, JobStatus.ACCEPTED, _ as String) @@ -180,7 +174,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> { throw new IdAlreadyExistsException("try again") } - 0 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 0 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 0 * this.jobResolverService.resolveJob(_ as String) 0 * this.persistenceService.updateJobStatus(jobId, JobStatus.RESOLVED, JobStatus.ACCEPTED, _ as String) @@ -196,7 +189,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.tracer.currentSpanCustomizer() >> this.span 1 * this.span.annotate(JobLaunchServiceImpl.BEGIN_LAUNCH_JOB_ANNOTATION) 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> jobId - 1 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 1 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 1 * this.jobResolverService.resolveJob(jobId) >> { throw new GenieJobResolutionException("fail") @@ -222,7 +214,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.tracer.currentSpanCustomizer() >> this.span 1 * this.span.annotate(JobLaunchServiceImpl.BEGIN_LAUNCH_JOB_ANNOTATION) 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> jobId - 1 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 1 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 1 * this.jobResolverService.resolveJob(jobId) >> { throw new GenieJobResolutionRuntimeException("fail") @@ -248,7 +239,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.tracer.currentSpanCustomizer() >> this.span 1 * this.span.annotate(JobLaunchServiceImpl.BEGIN_LAUNCH_JOB_ANNOTATION) 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> jobId - 1 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 1 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 1 * this.jobResolverService.resolveJob(jobId) >> resolvedJob 1 * this.span.annotate(JobLaunchServiceImpl.RESOLVED_JOB_ANNOTATION) @@ -267,7 +257,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.tracer.currentSpanCustomizer() >> this.span 1 * this.span.annotate(JobLaunchServiceImpl.BEGIN_LAUNCH_JOB_ANNOTATION) 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> jobId - 1 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 1 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 1 * this.jobResolverService.resolveJob(jobId) >> resolvedJob 1 * this.span.annotate(JobLaunchServiceImpl.RESOLVED_JOB_ANNOTATION) @@ -292,7 +281,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.tracer.currentSpanCustomizer() >> this.span 1 * this.span.annotate(JobLaunchServiceImpl.BEGIN_LAUNCH_JOB_ANNOTATION) 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> jobId - 1 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 1 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 1 * this.jobResolverService.resolveJob(jobId) >> resolvedJob 1 * this.span.annotate(JobLaunchServiceImpl.RESOLVED_JOB_ANNOTATION) @@ -316,7 +304,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.tracer.currentSpanCustomizer() >> this.span 1 * this.span.annotate(JobLaunchServiceImpl.BEGIN_LAUNCH_JOB_ANNOTATION) 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> jobId - 1 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 1 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 1 * this.jobResolverService.resolveJob(jobId) >> resolvedJob 1 * this.span.annotate(JobLaunchServiceImpl.RESOLVED_JOB_ANNOTATION) @@ -342,7 +329,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.tracer.currentSpanCustomizer() >> this.span 1 * this.span.annotate(JobLaunchServiceImpl.BEGIN_LAUNCH_JOB_ANNOTATION) 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> jobId - 1 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 1 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 1 * this.jobResolverService.resolveJob(jobId) >> resolvedJob 1 * this.span.annotate(JobLaunchServiceImpl.RESOLVED_JOB_ANNOTATION) @@ -369,7 +355,6 @@ class JobLaunchServiceImplSpec extends Specification { 1 * this.tracer.currentSpanCustomizer() >> this.span 1 * this.span.annotate(JobLaunchServiceImpl.BEGIN_LAUNCH_JOB_ANNOTATION) 1 * this.persistenceService.saveJobSubmission(jobSubmission) >> jobId - 1 * this.tagAdapter.tag(this.span, TracingConstants.JOB_ID_TAG, jobId) 1 * this.span.annotate(JobLaunchServiceImpl.SAVED_JOB_SUBMISSION_ANNOTATION) 1 * this.jobResolverService.resolveJob(jobId) >> resolvedJob 1 * this.span.annotate(JobLaunchServiceImpl.RESOLVED_JOB_ANNOTATION) diff --git a/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/JobResolverServiceImplSpec.groovy b/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/JobResolverServiceImplSpec.groovy index 8b84f3326f8..326be0a7584 100644 --- a/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/JobResolverServiceImplSpec.groovy +++ b/genie-web/src/test/groovy/com/netflix/genie/web/services/impl/JobResolverServiceImplSpec.groovy @@ -17,6 +17,8 @@ */ package com.netflix.genie.web.services.impl +import brave.SpanCustomizer +import brave.Tracer import com.google.common.collect.ImmutableSet import com.google.common.collect.Iterables import com.google.common.collect.Lists @@ -41,6 +43,8 @@ import com.netflix.genie.common.external.dtos.v4.JobStatus import com.netflix.genie.common.internal.exceptions.checked.GenieJobResolutionException import com.netflix.genie.common.internal.exceptions.unchecked.GenieJobResolutionRuntimeException import com.netflix.genie.common.internal.jobs.JobConstants +import com.netflix.genie.common.internal.tracing.brave.BraveTagAdapter +import com.netflix.genie.common.internal.tracing.brave.BraveTracingComponents import com.netflix.genie.web.data.services.DataServices import com.netflix.genie.web.data.services.PersistenceService import com.netflix.genie.web.dtos.ResolvedJob @@ -69,12 +73,14 @@ import java.util.stream.Stream @SuppressWarnings("GroovyAccessibility") class JobResolverServiceImplSpec extends Specification { - PersistenceService persistenceService - ClusterSelector clusterSelector - CommandSelector commandSelector - JobsProperties jobsProperties + private PersistenceService persistenceService + private ClusterSelector clusterSelector + private CommandSelector commandSelector + private JobsProperties jobsProperties + private Tracer tracer + private BraveTagAdapter tagAdapter - JobResolverServiceImpl service + private JobResolverServiceImpl service def setup() { this.persistenceService = Mock(PersistenceService) @@ -84,13 +90,22 @@ class JobResolverServiceImplSpec extends Specification { def dataServices = Mock(DataServices) { getPersistenceService() >> this.persistenceService } + this.tagAdapter = Mock(BraveTagAdapter) + this.tracer = Mock(Tracer) { + currentSpanCustomizer() >> Mock(SpanCustomizer) + } + def tracingComponents = Mock(BraveTracingComponents) { + getTagAdapter() >> this.tagAdapter + getTracer() >> this.tracer + } this.service = new JobResolverServiceImpl( dataServices, Lists.newArrayList(this.clusterSelector), this.commandSelector, new SimpleMeterRegistry(), this.jobsProperties, - Mock(Environment) + Mock(Environment), + tracingComponents ) } @@ -380,12 +395,10 @@ class JobResolverServiceImplSpec extends Specification { def commandName = UUID.randomUUID().toString() def commandTags = Sets.newHashSet(UUID.randomUUID().toString(), UUID.randomUUID().toString()) def clusterCriteria = Lists.newArrayList( - new Criterion - .Builder() + new Criterion.Builder() .withTags(Sets.newHashSet(UUID.randomUUID().toString())) .build(), - new Criterion - .Builder() + new Criterion.Builder() .withTags(Sets.newHashSet(UUID.randomUUID().toString(), UUID.randomUUID().toString())) .build() ) @@ -396,13 +409,12 @@ class JobResolverServiceImplSpec extends Specification { null, null, null, - new JobMetadata - .Builder(jobName, user) + new JobMetadata.Builder(jobName, user) .withTags(Sets.newHashSet(UUID.randomUUID().toString(), UUID.randomUUID().toString())) .withGrouping(grouping) .withGroupingInstance(groupingInstance) .build(), - new ExecutionResourceCriteria(clusterCriteria, commandCriterion, null), + new ExecutionResourceCriteria(clusterCriteria as List, commandCriterion, null), null, null ) @@ -434,7 +446,7 @@ class JobResolverServiceImplSpec extends Specification { 100L, null ) - def context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true) + def context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true, Mock(SpanCustomizer)) context.setCluster(cluster) context.setCommand(command) context.setJobMemory(this.jobsProperties.getMemory().getDefaultJobMemory()) @@ -596,7 +608,7 @@ class JobResolverServiceImplSpec extends Specification { allClusters.addAll(command2Clusters) when: "No commands are found in the database which match the users criterion" - def context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true) + def context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true, Mock(SpanCustomizer)) this.service.resolveCommand(context) then: "An exception is thrown" @@ -606,7 +618,7 @@ class JobResolverServiceImplSpec extends Specification { thrown(GenieJobResolutionException) when: "Only a single command is found which matches the criterion but it has no clusters" - context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, false) + context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, false, Mock(SpanCustomizer)) this.service.resolveCommand(context) then: "The selector is not invoked as no command is selected" @@ -616,7 +628,7 @@ class JobResolverServiceImplSpec extends Specification { thrown(GenieJobResolutionException) when: "Only a single command is found but it is filtered out by in memory cluster matching" - context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true) + context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true, Mock(SpanCustomizer)) this.service.resolveCommand(context) then: "The selector is not invoked as no command is selected" @@ -628,7 +640,7 @@ class JobResolverServiceImplSpec extends Specification { thrown(GenieJobResolutionException) when: "Only a single command is found" - context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true) + context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true, Mock(SpanCustomizer)) this.service.resolveCommand(context) def resolvedCommand = context.getCommand().orElseThrow({ new IllegalStateException() }) @@ -653,7 +665,7 @@ class JobResolverServiceImplSpec extends Specification { resolvedCommand == command1 when: "Many commands are found which match the criterion but nothing is selected by the selectors" - context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, false) + context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, false, Mock(SpanCustomizer)) this.service.resolveCommand(context) then: "An exception is thrown" @@ -679,7 +691,7 @@ class JobResolverServiceImplSpec extends Specification { thrown(GenieJobResolutionException) when: "The selectors throw an exception" - context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true) + context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true, Mock(SpanCustomizer)) this.service.resolveCommand(context) then: "It is propagated" @@ -702,7 +714,7 @@ class JobResolverServiceImplSpec extends Specification { thrown(GenieJobResolutionRuntimeException) when: "The selectors select a command" - context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true) + context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, true, Mock(SpanCustomizer)) this.service.resolveCommand(context) resolvedCommand = context.getCommand().orElseThrow({ new IllegalStateException() }) @@ -1027,9 +1039,10 @@ class JobResolverServiceImplSpec extends Specification { (Mock(Command)): Sets.newHashSet(Mock(Cluster), Mock(Cluster)) ] def cpu = 5 + def spanCustomizer = Mock(SpanCustomizer) when: - def context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, apiJob) + def context = new JobResolverServiceImpl.JobResolutionContext(jobId, jobRequest, apiJob, spanCustomizer) then: context.getJobId() == jobId @@ -1045,6 +1058,7 @@ class JobResolverServiceImplSpec extends Specification { !context.getJobDirectory().isPresent() !context.getCommandClusters().isPresent() !context.getCpu().isPresent() + context.getSpanCustomizer() == spanCustomizer when: context.build() @@ -1193,12 +1207,10 @@ class JobResolverServiceImplSpec extends Specification { @Nullable Integer requestedCpu ) { def clusterCriteria = Lists.newArrayList( - new Criterion - .Builder() + new Criterion.Builder() .withTags(Sets.newHashSet(UUID.randomUUID().toString())) .build(), - new Criterion - .Builder() + new Criterion.Builder() .withTags(Sets.newHashSet(UUID.randomUUID().toString(), UUID.randomUUID().toString())) .build() ) @@ -1208,7 +1220,7 @@ class JobResolverServiceImplSpec extends Specification { null, commandArgs, new JobMetadata.Builder(UUID.randomUUID().toString(), UUID.randomUUID().toString()).build(), - new ExecutionResourceCriteria(clusterCriteria, commandCriterion, null), + new ExecutionResourceCriteria(clusterCriteria as List, commandCriterion, null), new JobEnvironmentRequest.Builder() .withRequestedJobMemory(requestedMemory) .withRequestedJobCpu(requestedCpu) diff --git a/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplApplicationsTest.java b/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplApplicationsTest.java index 172f67581d6..5fded141110 100644 --- a/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplApplicationsTest.java +++ b/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplApplicationsTest.java @@ -24,6 +24,7 @@ import com.netflix.genie.common.external.dtos.v4.ApplicationRequest; import com.netflix.genie.common.external.dtos.v4.ApplicationStatus; import com.netflix.genie.common.external.dtos.v4.ExecutionEnvironment; +import com.netflix.genie.common.internal.tracing.brave.BraveTracingComponents; import com.netflix.genie.web.data.services.impl.jpa.entities.ApplicationEntity; import com.netflix.genie.web.data.services.impl.jpa.entities.CommandEntity; import com.netflix.genie.web.data.services.impl.jpa.repositories.JpaApplicationRepository; @@ -64,7 +65,8 @@ void setup() { Mockito.when(jpaRepositories.getApplicationRepository()).thenReturn(this.jpaApplicationRepository); this.persistenceService = new JpaPersistenceServiceImpl( Mockito.mock(EntityManager.class), - jpaRepositories + jpaRepositories, + Mockito.mock(BraveTracingComponents.class) ); } diff --git a/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplClustersTest.java b/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplClustersTest.java index b509e7f71be..1133dec236c 100644 --- a/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplClustersTest.java +++ b/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplClustersTest.java @@ -23,6 +23,7 @@ import com.netflix.genie.common.external.dtos.v4.ClusterRequest; import com.netflix.genie.common.external.dtos.v4.ClusterStatus; import com.netflix.genie.common.external.dtos.v4.ExecutionEnvironment; +import com.netflix.genie.common.internal.tracing.brave.BraveTracingComponents; import com.netflix.genie.web.data.services.impl.jpa.entities.ClusterEntity; import com.netflix.genie.web.data.services.impl.jpa.entities.FileEntity; import com.netflix.genie.web.data.services.impl.jpa.repositories.JpaClusterRepository; @@ -69,7 +70,8 @@ void setup() { Mockito.when(jpaRepositories.getFileRepository()).thenReturn(this.jpaFileRepository); this.service = new JpaPersistenceServiceImpl( Mockito.mock(EntityManager.class), - jpaRepositories + jpaRepositories, + Mockito.mock(BraveTracingComponents.class) ); } diff --git a/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplCommandsTest.java b/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplCommandsTest.java index 27da2a4a3a4..3e6ae3256f6 100644 --- a/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplCommandsTest.java +++ b/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplCommandsTest.java @@ -24,6 +24,7 @@ import com.netflix.genie.common.external.dtos.v4.CommandRequest; import com.netflix.genie.common.external.dtos.v4.CommandStatus; import com.netflix.genie.common.external.dtos.v4.ExecutionEnvironment; +import com.netflix.genie.common.internal.tracing.brave.BraveTracingComponents; import com.netflix.genie.web.data.services.impl.jpa.entities.CommandEntity; import com.netflix.genie.web.data.services.impl.jpa.repositories.JpaApplicationRepository; import com.netflix.genie.web.data.services.impl.jpa.repositories.JpaCommandRepository; @@ -75,7 +76,8 @@ void setup() { Mockito.when(jpaRepositories.getCriterionRepository()).thenReturn(Mockito.mock(JpaCriterionRepository.class)); this.service = new JpaPersistenceServiceImpl( Mockito.mock(EntityManager.class), - jpaRepositories + jpaRepositories, + Mockito.mock(BraveTracingComponents.class) ); } diff --git a/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplJobsTest.java b/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplJobsTest.java index 855f53b759c..56bef2791b7 100644 --- a/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplJobsTest.java +++ b/genie-web/src/test/java/com/netflix/genie/web/data/services/impl/jpa/JpaPersistenceServiceImplJobsTest.java @@ -31,6 +31,7 @@ import com.netflix.genie.common.internal.exceptions.checked.GenieCheckedException; import com.netflix.genie.common.internal.exceptions.unchecked.GenieInvalidStatusException; import com.netflix.genie.common.internal.exceptions.unchecked.GenieJobAlreadyClaimedException; +import com.netflix.genie.common.internal.tracing.brave.BraveTracingComponents; import com.netflix.genie.web.data.services.impl.jpa.entities.ClusterEntity; import com.netflix.genie.web.data.services.impl.jpa.entities.CommandEntity; import com.netflix.genie.web.data.services.impl.jpa.entities.JobEntity; @@ -78,8 +79,6 @@ class JpaPersistenceServiceImplJobsTest { private JpaApplicationRepository applicationRepository; private JpaClusterRepository clusterRepository; private JpaCommandRepository commandRepository; - private JpaFileRepository fileRepository; - private JpaTagRepository tagRepository; private JpaPersistenceServiceImpl persistenceService; @@ -89,20 +88,21 @@ void setup() { this.applicationRepository = Mockito.mock(JpaApplicationRepository.class); this.clusterRepository = Mockito.mock(JpaClusterRepository.class); this.commandRepository = Mockito.mock(JpaCommandRepository.class); - this.tagRepository = Mockito.mock(JpaTagRepository.class); - this.fileRepository = Mockito.mock(JpaFileRepository.class); + final JpaTagRepository tagRepository = Mockito.mock(JpaTagRepository.class); + final JpaFileRepository fileRepository = Mockito.mock(JpaFileRepository.class); final JpaRepositories jpaRepositories = Mockito.mock(JpaRepositories.class); Mockito.when(jpaRepositories.getApplicationRepository()).thenReturn(this.applicationRepository); Mockito.when(jpaRepositories.getClusterRepository()).thenReturn(this.clusterRepository); Mockito.when(jpaRepositories.getCommandRepository()).thenReturn(this.commandRepository); Mockito.when(jpaRepositories.getJobRepository()).thenReturn(this.jobRepository); - Mockito.when(jpaRepositories.getFileRepository()).thenReturn(this.fileRepository); - Mockito.when(jpaRepositories.getTagRepository()).thenReturn(this.tagRepository); + Mockito.when(jpaRepositories.getFileRepository()).thenReturn(fileRepository); + Mockito.when(jpaRepositories.getTagRepository()).thenReturn(tagRepository); this.persistenceService = new JpaPersistenceServiceImpl( Mockito.mock(EntityManager.class), - jpaRepositories + jpaRepositories, + Mockito.mock(BraveTracingComponents.class) ); } @@ -517,7 +517,7 @@ void cantGetJobHostIfNoJobExecution() { } @Test - void canGetJobHost() throws GenieCheckedException { + void canGetJobHost() { final String jobId = UUID.randomUUID().toString(); final String hostName = UUID.randomUUID().toString(); Mockito