Skip to content

Commit

Permalink
implement requested changes
Browse files Browse the repository at this point in the history
  • Loading branch information
EddeCCC committed Nov 5, 2024
1 parent a318f99 commit 2b234e9
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import java.lang.reflect.Method;
import java.util.Objects;
import java.util.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import rocks.inspectit.gepard.agent.instrumentation.hook.action.MethodExecutionContext;
Expand Down Expand Up @@ -49,7 +50,7 @@ public InternalInspectitContext onEnter(
Class<?> clazz, Object thiz, Method method, Object[] instrumentedMethodArgs) {
MethodExecutionContext executionContext =
new MethodExecutionContext(clazz, method, instrumentedMethodArgs);
AutoCloseable spanScope = startSpanAction(executionContext);
AutoCloseable spanScope = startSpanAction(executionContext).orElse(null);

// Using our log4j here will not be visible in the target application...
System.out.println("HELLO GEPARD : " + configuration.getMethodName());
Expand All @@ -76,18 +77,18 @@ public void onExit(InternalInspectitContext context, Object returnValue, Throwab
* Executes the startSpan-action, if existing
*
* @param executionContext the context of the current method
* @return the scope of the started span or null, if no span was started
* @return the scope of the started span or empty, if no span was started
*/
private AutoCloseable startSpanAction(MethodExecutionContext executionContext) {
AutoCloseable spanScope = null;
private Optional<AutoCloseable> startSpanAction(MethodExecutionContext executionContext) {
Optional<AutoCloseable> maybeSpanScope = Optional.empty();
if (Objects.nonNull(spanAction)) {
try {
spanScope = spanAction.startSpan(executionContext);
maybeSpanScope = spanAction.startSpan(executionContext);
} catch (Exception e) {
log.error("Could not execute start-span-action", e);
}
}
return spanScope;
return maybeSpanScope;
}

/**
Expand All @@ -96,10 +97,10 @@ private AutoCloseable startSpanAction(MethodExecutionContext executionContext) {
* @param context the internal inspectIT context of the method
*/
private void endSpanAction(InternalInspectitContext context) {
AutoCloseable spanScope = context.getSpanScope();
if (Objects.nonNull(spanAction))
Optional<AutoCloseable> maybeSpanScope = context.getSpanScope();
if (Objects.nonNull(spanAction) && maybeSpanScope.isPresent())
try {
spanAction.endSpan(spanScope);
spanAction.endSpan(maybeSpanScope.get());
} catch (Exception e) {
log.error("Could not execute end-span-action", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.context.Context;
import java.util.Objects;
import java.util.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import rocks.inspectit.gepard.agent.instrumentation.hook.action.MethodExecutionContext;
Expand All @@ -23,9 +23,9 @@ public class SpanAction {
* be called before {@link SpanAction#endSpan}.
*
* @param executionContext the execution context of the current method
* @return the scope of the started span or null, if the current span has the same name
* @return the scope of the started span or empty, if the current span has the same name
*/
public AutoCloseable startSpan(MethodExecutionContext executionContext) {
public Optional<AutoCloseable> startSpan(MethodExecutionContext executionContext) {
String spanName = getSpanName(executionContext);
Attributes methodAttributes = SpanUtil.createMethodAttributes(executionContext);

Expand All @@ -35,25 +35,23 @@ public AutoCloseable startSpan(MethodExecutionContext executionContext) {
Span otelSpan = Span.current();
// We overwrite the OTel attributes, if they use the same key
otelSpan.setAllAttributes(methodAttributes);
return null;
return Optional.empty();
}

// Create new span
// Create new inspectIT span
Tracer tracer = OpenTelemetryAccessor.getTracer();
Span span = tracer.spanBuilder(spanName).setParent(Context.current()).startSpan();
span.setAllAttributes(methodAttributes);
return span.makeCurrent();
return Optional.of(span.makeCurrent());
}

/**
* Ends the current span and closes its scope. Should be called after {@link
* SpanAction#startSpan}. If the scope is null, we won't do anything.
* Ends the current span and closes the provided scope. Should be called after {@link
* SpanAction#startSpan}.
*
* @param spanScope the scope of the span, which should be finished
*/
public void endSpan(AutoCloseable spanScope) {
if (Objects.isNull(spanScope)) return;

Span current = Span.current();
try {
spanScope.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ public static boolean spanAlreadyExists(String spanName) {
*/
public static Attributes createMethodAttributes(MethodExecutionContext executionContext) {
Object[] arguments = executionContext.getArguments();
AttributesBuilder builder = Attributes.builder();
if (arguments.length == 0) return builder.build();
if (arguments.length == 0) return Attributes.empty();

Method method = executionContext.getMethod();
Parameter[] parameters = method.getParameters();
Expand All @@ -52,6 +51,7 @@ public static Attributes createMethodAttributes(MethodExecutionContext execution
"Number of passed method arguments does not match with number of parameter in method definition of "
+ method.getName());

AttributesBuilder builder = Attributes.builder();
for (int i = 0; i < parameters.length; i++) {
String argumentName = parameters[i].getName();
Object argumentValue = arguments[i];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
/* (C) 2024 */
package rocks.inspectit.gepard.agent.instrumentation.hook;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;

import java.lang.reflect.Method;
import java.util.Optional;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import rocks.inspectit.gepard.agent.instrumentation.hook.action.span.SpanAction;
import rocks.inspectit.gepard.agent.instrumentation.hook.action.span.exception.CouldNotCloseSpanScopeException;
import rocks.inspectit.gepard.agent.instrumentation.hook.configuration.model.MethodHookConfiguration;
import rocks.inspectit.gepard.bootstrap.context.InternalInspectitContext;

Expand All @@ -39,28 +38,29 @@ void beforeEach() {

@Test
void shouldStartSpanAndCreateContext() {
when(spanAction.startSpan(any())).thenReturn(closeable);
when(spanAction.startSpan(any())).thenReturn(Optional.of(closeable));

InternalInspectitContext context = hook.onEnter(getClass(), this, method, new Object[] {});

verify(spanAction).startSpan(any());
assertEquals(closeable, context.getSpanScope());
assertTrue(context.getSpanScope().isPresent());
assertEquals(closeable, context.getSpanScope().get());
assertEquals(hook, context.getHook());
}

@Test
void shouldNotReturnSpanScopeWhenExceptionThrown() {
doThrow(CouldNotCloseSpanScopeException.class).when(spanAction).startSpan(any());
doThrow(IllegalStateException.class).when(spanAction).startSpan(any());

InternalInspectitContext context = hook.onEnter(getClass(), this, method, new Object[] {});

verify(spanAction).startSpan(any());
assertNull(context.getSpanScope());
assertTrue(context.getSpanScope().isEmpty());
}

@Test
void shouldEndSpan() {
when(internalContext.getSpanScope()).thenReturn(closeable);
when(internalContext.getSpanScope()).thenReturn(Optional.of(closeable));

hook.onExit(internalContext, null, null);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.opentelemetry.sdk.trace.ReadableSpan;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.Optional;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -64,13 +65,13 @@ void beforeEach() {

@Test
void shouldCreateScopeAndWriteAttributesWhenNoSpanExist() throws Exception {
AutoCloseable scope = action.startSpan(executionContext);
Optional<AutoCloseable> scope = action.startSpan(executionContext);
ReadableSpan currentSpan = (ReadableSpan) Span.current();

assertNotNull(scope);
assertTrue(scope.isPresent());
assertEquals(currentSpan.getAttribute(stringKey(key)), value);

scope.close();
scope.get().close();
}

@Test
Expand All @@ -79,16 +80,29 @@ void shouldNotCreateScopeButWriteAttributesWhenSpanAlreadyExist() {
Span otelSpan = tracer.spanBuilder(spanName).startSpan();
Scope otelScope = otelSpan.makeCurrent();

AutoCloseable scope = action.startSpan(executionContext);
Optional<AutoCloseable> scope = action.startSpan(executionContext);
ReadableSpan currentSpan = (ReadableSpan) Span.current();

assertNull(scope);
assertTrue(scope.isEmpty());
assertEquals(currentSpan.getAttribute(stringKey(key)), value);

otelScope.close();
otelSpan.end();
}

@Test
void shouldCreateEmptyAttributesForNoMethodArguments() throws Exception {
executionContext = new MethodExecutionContext(SpanActionTest.class, method, new Object[] {});

Optional<AutoCloseable> scope = action.startSpan(executionContext);
ReadableSpan currentSpan = (ReadableSpan) Span.current();

assertTrue(scope.isPresent());
assertTrue(currentSpan.getAttributes().isEmpty());

scope.get().close();
}

@Test
void shouldCloseScope() throws Exception {
action.endSpan(closeable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ private void awaitUpdateMessage(String updateMessage, int amount) {

Awaitility.await()
.pollDelay(5, TimeUnit.SECONDS)
.atMost(15, TimeUnit.SECONDS)
.atMost(12, TimeUnit.SECONDS)
.until(
() -> {
String newLogs = target.getLogs();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* (C) 2024 */
package rocks.inspectit.gepard.bootstrap.context;

import java.util.Optional;
import rocks.inspectit.gepard.bootstrap.instrumentation.IMethodHook;

/**
Expand Down Expand Up @@ -31,7 +32,7 @@ public IMethodHook getHook() {
/**
* @return the current span scope
*/
public AutoCloseable getSpanScope() {
return spanScope;
public Optional<AutoCloseable> getSpanScope() {
return Optional.ofNullable(spanScope);
}
}

0 comments on commit 2b234e9

Please sign in to comment.