Open fprochazka opened 1 year ago
It might also be a good idea to start with a JUnit 5 extension (as a library instrumentation). It might be easier for you to start with non-javaagent instrumentation, and then simply use it in the javaagent. And, as an added benefit, people that don't use javaagent might get to use that as well.
I am looking for a solution to this as well and stumbled across this archived project.
It's outdated, not published to e.g. Maven Central etc., however, an interesting starting point.
@fprochazka @mateuszrzeszutek @Okeanos I opened a draft PR, could you verify please an idea?
@aschugunov very cool that someone is working on this (#11505). I have been locally playing around with this as well but never got anything I consider publishable (did so anyway now). Direct link to the PR because GitHub makes it unnecessarily difficult to find otherwise: https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/11505
From my initial look at your code and what I experimented with I have a couple of questions/remarks/suggestions:
I think there's a few design decisions that should be explained here (in the readme). To start: JUnit provides a rather large number of different interceptors/interfaces to implement in order to inject extension code, you selected InvocationInterceptor, BeforeAllCallback, BeforeEachCallback, AfterAllCallback, AfterEachCallback
. However, JUnit also offers TestInstancePreConstructCallback
and TestInstancePreDestroyCallback
, which I currently think lend themselves well to root-span creation. Somewhat related, there's also the test-suite construct which could be source for a root-span.
Other decisions that should be documented would be where/why/how to open a new span or add an event and what the actual parent span of each JUnit lifecycle is supposed to be. For instance, the Before/After pieces could reasonably be designed as span event for a particular span, e.g. test-execution or test-class instantiation.
This is particularly interesting when for example considering the @TestInstance
annotation which changes the way test classes and their tests are stacked or when dealing with repeated, dynamic or nested tests.
Concerning the attributes: those are looking good overall. I would add the @TestInstance
value, however, as well as the unique ID from the JUnit extension context.
OTEL Context propagation within the JUnit extension, i.e. retrieving the (correct) parent … I have somewhat struggled with that myself. Does the Context parentContext = Context.current();
invocation in the createInterceptorSpan
method actually work as expected? For me it usually didn't for some reason (on second thought maybe because of the way I instantiated the global OTEL instance 🤔) and I had to use the JUnit extension context store to iterate over my "stack" of possibly existing parent-spans and retrieve the correct one that way.
I am looking eagerly forward to seeing some tests that actually verify behaviour of the library with the help of the OTEL SDK testing library. The way I constructed my tests was with the help of JUnit Platform Test Kit which works fairly well (see example below). For the test design overall I opted to create a class for each kind of JUnit test ("basic", "repeated", "dynamic", "parameterized", …) to make verification easier.
I may have overlooked that but are there already pieces in place that can pick up an externally provided trace context/trace parent, e.g. created by otel-cli or even better the OTEL Maven extension? From what I can tell when using e.g. a supported HTTP client library and using it in a test to call a WireMock server the context propagation already works out of the box so reading it in the JUnit extension from outside would be awesome as well.
Are there plans for a metrics extension as well? At least for counting the number of tests and their state (aborted, disabled, successful, failed) this should be very easy using JUnit's TestWatcher
interface.
Test Example:
class BasicTracingTest {
@RegisterExtension
private static final OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create();
@Test
void verifySpans() {
EngineTestKit
.engine(JupiterEngineDescriptor.ENGINE_ID)
.configurationParameter(EXECUTE_TEST_CASE, "true") // related to the @TestCase annotation to conditionally prevent/trigger execution
.selectors(selectClass(TracingExampleTestCase.class))
.execute()
.testEvents()
.assertStatistics(stats ->
stats
.aborted(1)
.failed(1)
.skipped(1)
.started(3)
.succeeded(1)
);
otelTesting.assertTraces().hasTracesSatisfyingExactly(
trace -> {} // trace assertions
);
}
// Custom annotation to conditionally prevent executuon of the class when executing the enclosing test via e.g. the IDE
// the annotation is not strictly speaking necessary based on the class modifiers and its name
// see https://junit.org/junit5/docs/current/user-guide/#writing-tests-conditional-execution
@TestCase
@ExtendWith(OpenTelemetryTracing.class)
@TestInstance(TestInstance.Lifecycle.PER_CLASS) // may produce different results than @TestInstance(TestInstance.Lifecycle.PER_METHOD)
static class TracingExampleTestCase {
@Test
void abortedTest() {
assumeTrue("abc".contains("Z"), "abc does not contain Z");
// aborted ...
}
@Test
void failingTest() {
fail("failed on purpose");
}
@Test
@Disabled("for demonstration purposes")
void skippedTest() {
// skipped ...
}
@Test
void succeedingTest() {
assertTrue(true);
}
}
}
Hello, I would like to run my tests with OTEL java agent auto-instrumentation, where each test would be a new root span. Ideally, the before/test/after phases and test listeners should be separated into sub-spans, just as junit understands them. Thank you for considering it. I might attempt to contribute this myself, but TBH I don't wanna promise anything.