Skip to content

Commit

Permalink
Tracing Improvements
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
tgianos committed May 14, 2021
1 parent c284ffe commit cfbec17
Show file tree
Hide file tree
Showing 15 changed files with 259 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand All @@ -147,19 +145,14 @@ public int getExitCode() {
private ScopedSpan initializeTracing() {
// Attempt to extract any existing trace information from the environment
final Optional<TraceContext> 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -40,7 +41,6 @@ class GenieAgentRunnerSpec extends Specification {
BraveTracePropagator tracePropagator
BraveTracingCleanup traceCleaner
BraveTagAdapter tagAdapter
ScopedSpan initSpan
ScopedSpan runSpan
TraceContext initContext

Expand All @@ -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,
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -54,7 +56,9 @@
@Import(
{
DataAutoConfiguration.class,
ValidationAutoConfiguration.class
ValidationAutoConfiguration.class,
TraceAutoConfiguration.class,
CommonTracingAutoConfiguration.class
}
)
@MockBean(
Expand Down
Loading

0 comments on commit cfbec17

Please sign in to comment.