Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: configure trace level #71

Merged
merged 4 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/main/java/com/vaadin/extension/Constants.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.vaadin.extension;

/**
* Constants used by the Vaadin observability extension, for example for
* configuration options
*/
public class Constants {
public static final String CONFIG_TRACE_LEVEL = "otel.instrumentation.vaadin.trace-level";
}
34 changes: 34 additions & 0 deletions src/main/java/com/vaadin/extension/conf/Configuration.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.vaadin.extension.conf;

import com.vaadin.extension.Constants;

import io.opentelemetry.javaagent.bootstrap.internal.InstrumentationConfig;

/**
* Provides the effective configuration for the Vaadin observability extension.
*/
public class Configuration {
public static final TraceLevel TRACE_LEVEL = determineTraceLevel();

private static TraceLevel determineTraceLevel() {
String traceLevelString = InstrumentationConfig.get().getString(
Constants.CONFIG_TRACE_LEVEL, TraceLevel.DEFAULT.name());
try {
return TraceLevel.valueOf(traceLevelString.toUpperCase());
} catch (IllegalArgumentException ignored) {
return TraceLevel.DEFAULT;
}
}

/**
* Checks whether a trace level is enabled. Can be used by instrumentations
* to check whether some detail should be added to a trace or not.
*
* @param traceLevel
* the trace level to check
* @return true if the trace level is enabled, false if not
*/
public static boolean isEnabled(TraceLevel traceLevel) {
return TRACE_LEVEL.includes(traceLevel);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
package com.vaadin.extension;
package com.vaadin.extension.conf;

import com.vaadin.extension.Constants;

import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.config.ConfigPropertySource;
Expand All @@ -7,8 +9,9 @@
import java.util.Map;

/**
* disable the automatic vaadin instrumentation Custom distributions can supply
* their own default configuration by implementing {@link ConfigPropertySource}.
* Provides default values for global and extension-specific OpenTelemetry
* configuration. The defaults can be overwritten by a configuration mechanism
* with a higher priority.
*
* <p>
* The configuration priority, from highest to lowest is:
Expand All @@ -22,7 +25,7 @@
* </ul>
*/
@AutoService(ConfigPropertySource.class)
public class Configuration implements ConfigPropertySource {
public class ConfigurationDefaults implements ConfigPropertySource {

@Override
public Map<String, String> getProperties() {
Expand All @@ -31,6 +34,8 @@ public Map<String, String> getProperties() {
properties.put("otel.instrumentation.vaadin.enabled", "false");
// Set the service name to vaadin by default.
properties.put("otel.service.name", "vaadin");
// Configure default trace level
properties.put(Constants.CONFIG_TRACE_LEVEL, TraceLevel.DEFAULT.name());
return properties;
}
}
14 changes: 14 additions & 0 deletions src/main/java/com/vaadin/extension/conf/TraceLevel.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.vaadin.extension.conf;

/**
* The detail level of traces. The global trace level can be configured in the
* {@link Configuration}, and individual instrumentations can check the trace
* level to determine whether to add something to a trace or not.
*/
public enum TraceLevel {
MINIMUM, DEFAULT, MAXIMUM,;

public boolean includes(TraceLevel level) {
return this.ordinal() >= level.ordinal();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
import static net.bytebuddy.matcher.ElementMatchers.named;

import com.vaadin.extension.InstrumentationHelper;
import com.vaadin.flow.server.communication.UidlRequestHandler;
import com.vaadin.extension.conf.Configuration;
import com.vaadin.extension.conf.TraceLevel;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
Expand Down Expand Up @@ -42,17 +43,15 @@ public void transform(TypeTransformer transformer) {
public static class SynchronizedHandleRequestAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.This UidlRequestHandler uidlRequestHandler,
@Advice.Origin("#m") String methodName,
@Advice.Local("otelSpan") Span span,
public static void onEnter(@Advice.Local("otelSpan") Span span,
@Advice.Local("otelScope") Scope scope) {
String spanName = uidlRequestHandler.getClass().getSimpleName()
+ "." + methodName;
span = InstrumentationHelper.startSpan(spanName);
if (Configuration.isEnabled(TraceLevel.DEFAULT)) {
final String spanName = "Handle Client Request";
span = InstrumentationHelper.startSpan(spanName);

Context context = currentContext().with(span);
scope = context.makeCurrent();
Context context = currentContext().with(span);
scope = context.makeCurrent();
}
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import static org.junit.jupiter.api.Assertions.assertEquals;

import com.vaadin.extension.ContextKeys;
import com.vaadin.extension.conf.Configuration;
import com.vaadin.extension.conf.TraceLevel;
import com.vaadin.extension.instrumentation.util.MockVaadinService;
import com.vaadin.extension.instrumentation.util.OpenTelemetryTestTools;
import com.vaadin.flow.component.Component;
Expand All @@ -25,6 +27,7 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.mockito.MockedStatic;
import org.mockito.Mockito;

import java.util.ArrayList;
Expand All @@ -35,6 +38,8 @@ public abstract class AbstractInstrumentationTest {
private VaadinSession mockSession;
private VaadinService mockService;
private Scope sessionScope;
private MockedStatic<Configuration> ConfigurationMock;
private TraceLevel configuredTraceLevel;

public UI getMockUI() {
return mockUI;
Expand Down Expand Up @@ -87,11 +92,27 @@ public void openSessionContext() {
.with(ContextKeys.SESSION_ID, getMockSessionId()).makeCurrent();
}

@BeforeEach
public void setupConfigurationMock() {
configuredTraceLevel = TraceLevel.DEFAULT;
ConfigurationMock = Mockito.mockStatic(Configuration.class);
ConfigurationMock.when(() -> Configuration.isEnabled(Mockito.any()))
.thenAnswer(invocation -> {
TraceLevel level = invocation.getArgument(0);
return configuredTraceLevel.includes(level);
});
Comment on lines +97 to +103
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some construct here to mock the enabled check, and allow configuring a specific trace level in tests.

Since we don't don't expose the API of our library to developers, we could also consider changing the configuration API to use getters and setters.

}

@AfterEach
public void closeSessionContext() {
sessionScope.close();
}

@AfterEach
public void closeConfigurationMock() {
ConfigurationMock.close();
}

protected RootContextScope withRootContext() {
return new RootContextScope();
}
Expand All @@ -100,6 +121,10 @@ protected Span getCapturedSpan(int index) {
return OpenTelemetryTestTools.getSpanBuilderCapture().getSpan(index);
}

protected int getCapturedSpanCount() {
return OpenTelemetryTestTools.getSpanBuilderCapture().getSpans().size();
}

protected SpanData getExportedSpan(int index) {
return OpenTelemetryTestTools.getSpanExporter().getSpan(index);
}
Expand All @@ -120,6 +145,10 @@ protected void assertSpanHasException(SpanData span, Throwable throwable) {
.get(SemanticAttributes.EXCEPTION_MESSAGE));
}

protected void configureTraceLevel(TraceLevel level) {
configuredTraceLevel = level;
}

@Tag("test-view")
protected static class TestView extends Component {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,42 @@

import static org.junit.jupiter.api.Assertions.*;

import com.vaadin.flow.server.communication.UidlRequestHandler;
import com.vaadin.extension.conf.TraceLevel;

import io.opentelemetry.sdk.trace.data.SpanData;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

class UidlRequestHandlerInstrumentationTest
extends AbstractInstrumentationTest {

@Test
public void synchronizedHandleRequest_createsSpan() {
UidlRequestHandler uidlRequestHandler = Mockito
.mock(UidlRequestHandler.class);

UidlRequestHandlerInstrumentation.SynchronizedHandleRequestAdvice
.onEnter(uidlRequestHandler, "synchronizedHandleRequest", null,
null);
.onEnter(null, null);
UidlRequestHandlerInstrumentation.SynchronizedHandleRequestAdvice
.onExit(null, getCapturedSpan(0), null);

SpanData span = getExportedSpan(0);
assertEquals("UidlRequestHandler.synchronizedHandleRequest",
span.getName());
assertEquals("Handle Client Request", span.getName());
}

@Test
public void synchronizedHandleRequest_traceLevels() {
configureTraceLevel(TraceLevel.MINIMUM);
UidlRequestHandlerInstrumentation.SynchronizedHandleRequestAdvice
.onEnter(null, null);
assertEquals(0, getCapturedSpanCount());

configureTraceLevel(TraceLevel.DEFAULT);
resetSpans();
UidlRequestHandlerInstrumentation.SynchronizedHandleRequestAdvice
.onEnter(null, null);
assertEquals(1, getCapturedSpanCount());

configureTraceLevel(TraceLevel.MAXIMUM);
resetSpans();
UidlRequestHandlerInstrumentation.SynchronizedHandleRequestAdvice
.onEnter(null, null);
assertEquals(1, getCapturedSpanCount());
}
}