Skip to content

Commit

Permalink
Added changes from the review
Browse files Browse the repository at this point in the history
  • Loading branch information
@marcingrzejszczak
marcingrzejszczak committed Apr 6, 2023
1 parent 415d22c commit 0b8b935
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 59 deletions.
1 change: 0 additions & 1 deletion build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ subprojects {
description = 'Facade over tracing concepts'

repositories {
mavenLocal()
mavenCentral()
maven {
// TODO remove before releasing GA
Expand Down
8 changes: 4 additions & 4 deletions ...g/src/main/java/io/micrometer/tracing/annotation/ImperativeMethodInvocationProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*/
package io.micrometer.tracing.annotation;

import io.micrometer.common.annotation.TagValueExpressionResolver;
import io.micrometer.common.annotation.TagValueResolver;
import io.micrometer.common.annotation.ValueExpressionResolver;
import io.micrometer.common.annotation.ValueResolver;
import io.micrometer.common.util.StringUtils;
import io.micrometer.tracing.Span;
import io.micrometer.tracing.Tracer;
Expand All @@ -43,8 +43,8 @@ public class ImperativeMethodInvocationProcessor extends AbstractMethodInvocatio
* resolver provider
*/
public ImperativeMethodInvocationProcessor(NewSpanParser newSpanParser, Tracer tracer,
Function<Class<? extends TagValueResolver>, ? extends TagValueResolver> resolverProvider,
Function<Class<? extends TagValueExpressionResolver>, ? extends TagValueExpressionResolver> expressionResolverProvider) {
Function<Class<? extends ValueResolver>, ? extends ValueResolver> resolverProvider,
Function<Class<? extends ValueExpressionResolver>, ? extends ValueExpressionResolver> expressionResolverProvider) {
super(newSpanParser, tracer, tracer.currentTraceContext(),
new SpanTagAnnotationHandler(resolverProvider, expressionResolverProvider));
}
Expand Down
10 changes: 5 additions & 5 deletions micrometer-tracing/src/main/java/io/micrometer/tracing/annotation/SpanTag.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@
*/
package io.micrometer.tracing.annotation;

import io.micrometer.common.annotation.NoOpTagValueResolver;
import io.micrometer.common.annotation.TagValueResolver;
import io.micrometer.common.annotation.NoOpValueResolver;
import io.micrometer.common.annotation.ValueResolver;

import java.lang.annotation.*;

/**
* There are 3 different ways to add tags to a span. All of them are controlled by the
* annotation values. Precedence is:
* <p>
* try with the {@link TagValueResolver} bean if the value of the bean wasn't set, try to
* try with the {@link ValueResolver} bean if the value of the bean wasn't set, try to
* evaluate a SPEL expression if there’s no SPEL expression just return a
* {@code toString()} value of the parameter
*
Expand Down Expand Up @@ -57,8 +57,8 @@

/**
* Use this bean to resolve the tag value. Has the highest precedence.
* @return {@link TagValueResolver} bean
* @return {@link ValueResolver} bean
*/
Class<? extends TagValueResolver> resolver() default NoOpTagValueResolver.class;
Class<? extends ValueResolver> resolver() default NoOpValueResolver.class;

}
23 changes: 11 additions & 12 deletions ...eter-tracing/src/main/java/io/micrometer/tracing/annotation/SpanTagAnnotationHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
package io.micrometer.tracing.annotation;

import io.micrometer.common.KeyValue;
import io.micrometer.common.annotation.NoOpTagValueResolver;
import io.micrometer.common.annotation.NoOpValueResolver;
import io.micrometer.common.annotation.TagAnnotationHandler;
import io.micrometer.common.annotation.TagValueExpressionResolver;
import io.micrometer.common.annotation.TagValueResolver;
import io.micrometer.common.annotation.ValueExpressionResolver;
import io.micrometer.common.annotation.ValueResolver;
import io.micrometer.common.util.StringUtils;
import io.micrometer.tracing.SpanCustomizer;

Expand All @@ -41,9 +41,8 @@
*/
class SpanTagAnnotationHandler extends TagAnnotationHandler<SpanCustomizer> {

public SpanTagAnnotationHandler(
Function<Class<? extends TagValueResolver>, ? extends TagValueResolver> resolverProvider,
Function<Class<? extends TagValueExpressionResolver>, ? extends TagValueExpressionResolver> expressionResolverProvider) {
public SpanTagAnnotationHandler(Function<Class<? extends ValueResolver>, ? extends ValueResolver> resolverProvider,
Function<Class<? extends ValueExpressionResolver>, ? extends ValueExpressionResolver> expressionResolverProvider) {
super((keyValue, spanCustomizer) -> spanCustomizer.tag(keyValue.getKey(), keyValue.getValue()),
resolverProvider, expressionResolverProvider, SpanTag.class, (annotation, o) -> {
if (!(annotation instanceof SpanTag)) {
Expand All @@ -60,15 +59,15 @@ private static String resolveTagKey(SpanTag annotation) {
}

static String resolveTagValue(SpanTag annotation, Object argument,
Function<Class<? extends TagValueResolver>, ? extends TagValueResolver> resolverProvider,
Function<Class<? extends TagValueExpressionResolver>, ? extends TagValueExpressionResolver> expressionResolverProvider) {
Function<Class<? extends ValueResolver>, ? extends ValueResolver> resolverProvider,
Function<Class<? extends ValueExpressionResolver>, ? extends ValueExpressionResolver> expressionResolverProvider) {
String value = null;
if (annotation.resolver() != NoOpTagValueResolver.class) {
TagValueResolver tagValueResolver = resolverProvider.apply(annotation.resolver());
value = tagValueResolver.resolve(argument);
if (annotation.resolver() != NoOpValueResolver.class) {
ValueResolver ValueResolver = resolverProvider.apply(annotation.resolver());
value = ValueResolver.resolve(argument);
}
else if (StringUtils.isNotBlank(annotation.expression())) {
value = expressionResolverProvider.apply(TagValueExpressionResolver.class)
value = expressionResolverProvider.apply(ValueExpressionResolver.class)
.resolve(annotation.expression(), argument);
}
else if (argument != null) {
Expand Down
6 changes: 3 additions & 3 deletions ...ter-tracing/src/test/java/io/micrometer/tracing/annotation/NoOpTagValueResolverTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@
*/
package io.micrometer.tracing.annotation;

import io.micrometer.common.annotation.NoOpTagValueResolver;
import io.micrometer.common.annotation.NoOpValueResolver;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.BDDAssertions.then;

/**
* @author Marcin Grzejszczak
*/
class NoOpTagValueResolverTests {
class NoOpValueResolverTests {

@Test
void should_return_null() throws Exception {
then(new NoOpTagValueResolver().resolve("")).isNull();
then(new NoOpValueResolver().resolve("")).isNull();
}

}
31 changes: 15 additions & 16 deletions ...ing/src/test/java/io/micrometer/tracing/annotation/NullSpanTagAnnotationHandlerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*/
package io.micrometer.tracing.annotation;

import io.micrometer.common.annotation.TagValueExpressionResolver;
import io.micrometer.common.annotation.TagValueResolver;
import io.micrometer.common.annotation.ValueExpressionResolver;
import io.micrometer.common.annotation.ValueResolver;
import io.micrometer.tracing.SpanCustomizer;
import org.junit.jupiter.api.Test;

Expand All @@ -28,9 +28,9 @@

class NullSpanTagAnnotationHandlerTests {

TagValueResolver tagValueResolver = parameter -> null;
ValueResolver ValueResolver = parameter -> null;

TagValueExpressionResolver tagValueExpressionResolver = (expression, parameter) -> "";
ValueExpressionResolver ValueExpressionResolver = (expression, parameter) -> "";

SpanCustomizer spanCustomizer = new SpanCustomizer() {
@Override
Expand All @@ -49,16 +49,16 @@ public SpanCustomizer event(String value) {
}
};

SpanTagAnnotationHandler handler = new SpanTagAnnotationHandler(aClass -> tagValueResolver,
aClass -> tagValueExpressionResolver);
SpanTagAnnotationHandler handler = new SpanTagAnnotationHandler(aClass -> ValueResolver,
aClass -> ValueExpressionResolver);

@Test
void shouldUseEmptyStringWheCustomTagValueResolverReturnsNull() throws NoSuchMethodException, SecurityException {
Method method = AnnotationMockClass.class.getMethod("getAnnotationForTagValueResolver", String.class);
void shouldUseEmptyStringWheCustomValueResolverReturnsNull() throws NoSuchMethodException, SecurityException {
Method method = AnnotationMockClass.class.getMethod("getAnnotationForValueResolver", String.class);
Annotation annotation = method.getParameterAnnotations()[0][0];
if (annotation instanceof SpanTag) {
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, "test",
aClass -> tagValueResolver, aClass -> tagValueExpressionResolver);
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, "test", aClass -> ValueResolver,
aClass -> ValueExpressionResolver);
assertThat(resolvedValue).isEqualTo("");
}
else {
Expand All @@ -71,8 +71,8 @@ void shouldUseEmptyStringWhenTagValueExpressionReturnNull() throws NoSuchMethodE
Method method = AnnotationMockClass.class.getMethod("getAnnotationForTagValueExpression", String.class);
Annotation annotation = method.getParameterAnnotations()[0][0];
if (annotation instanceof SpanTag) {
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, "test",
aClass -> tagValueResolver, aClass -> tagValueExpressionResolver);
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, "test", aClass -> ValueResolver,
aClass -> ValueExpressionResolver);

assertThat(resolvedValue).isEqualTo("");
}
Expand All @@ -86,8 +86,8 @@ void shouldUseEmptyStringWhenArgumentIsNull() throws NoSuchMethodException, Secu
Method method = AnnotationMockClass.class.getMethod("getAnnotationForArgumentToString", Long.class);
Annotation annotation = method.getParameterAnnotations()[0][0];
if (annotation instanceof SpanTag) {
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, null, aClass -> tagValueResolver,
aClass -> tagValueExpressionResolver);
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, null, aClass -> ValueResolver,
aClass -> ValueExpressionResolver);
assertThat(resolvedValue).isEqualTo("");
}
else {
Expand All @@ -98,8 +98,7 @@ void shouldUseEmptyStringWhenArgumentIsNull() throws NoSuchMethodException, Secu
protected class AnnotationMockClass {

@NewSpan
public void getAnnotationForTagValueResolver(
@SpanTag(key = "test", resolver = TagValueResolver.class) String test) {
public void getAnnotationForValueResolver(@SpanTag(key = "test", resolver = ValueResolver.class) String test) {
}

@NewSpan
Expand Down
35 changes: 17 additions & 18 deletions ...tracing/src/test/java/io/micrometer/tracing/annotation/SpanTagAnnotationHandlerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*/
package io.micrometer.tracing.annotation;

import io.micrometer.common.annotation.TagValueExpressionResolver;
import io.micrometer.common.annotation.TagValueResolver;
import io.micrometer.common.annotation.ValueExpressionResolver;
import io.micrometer.common.annotation.ValueResolver;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -34,25 +34,25 @@

class SpanTagAnnotationHandlerTests {

TagValueResolver tagValueResolver = parameter -> "Value from myCustomTagValueResolver";
ValueResolver ValueResolver = parameter -> "Value from myCustomValueResolver";

TagValueExpressionResolver tagValueExpressionResolver = new SpelTagValueExpressionResolver();
ValueExpressionResolver ValueExpressionResolver = new SpelValueExpressionResolver();

SpanTagAnnotationHandler handler;

@BeforeEach
void setup() {
this.handler = new SpanTagAnnotationHandler(aClass -> tagValueResolver, aClass -> tagValueExpressionResolver);
this.handler = new SpanTagAnnotationHandler(aClass -> ValueResolver, aClass -> ValueExpressionResolver);
}

@Test
void shouldUseCustomTagValueResolver() throws NoSuchMethodException, SecurityException {
Method method = AnnotationMockClass.class.getMethod("getAnnotationForTagValueResolver", String.class);
void shouldUseCustomValueResolver() throws NoSuchMethodException, SecurityException {
Method method = AnnotationMockClass.class.getMethod("getAnnotationForValueResolver", String.class);
Annotation annotation = method.getParameterAnnotations()[0][0];
if (annotation instanceof SpanTag) {
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, "test",
aClass -> tagValueResolver, aClass -> tagValueExpressionResolver);
assertThat(resolvedValue).isEqualTo("Value from myCustomTagValueResolver");
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, "test", aClass -> ValueResolver,
aClass -> ValueExpressionResolver);
assertThat(resolvedValue).isEqualTo("Value from myCustomValueResolver");
}
else {
fail("Annotation was not SpanTag");
Expand All @@ -64,8 +64,8 @@ void shouldUseTagValueExpression() throws NoSuchMethodException, SecurityExcepti
Method method = AnnotationMockClass.class.getMethod("getAnnotationForTagValueExpression", String.class);
Annotation annotation = method.getParameterAnnotations()[0][0];
if (annotation instanceof SpanTag) {
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, "test",
aClass -> tagValueResolver, aClass -> tagValueExpressionResolver);
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, "test", aClass -> ValueResolver,
aClass -> ValueExpressionResolver);

assertThat(resolvedValue).isEqualTo("hello characters");
}
Expand All @@ -79,8 +79,8 @@ void shouldReturnArgumentToString() throws NoSuchMethodException, SecurityExcept
Method method = AnnotationMockClass.class.getMethod("getAnnotationForArgumentToString", Long.class);
Annotation annotation = method.getParameterAnnotations()[0][0];
if (annotation instanceof SpanTag) {
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, 15, aClass -> tagValueResolver,
aClass -> tagValueExpressionResolver);
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation, 15, aClass -> ValueResolver,
aClass -> ValueExpressionResolver);
assertThat(resolvedValue).isEqualTo("15");
}
else {
Expand All @@ -91,8 +91,7 @@ void shouldReturnArgumentToString() throws NoSuchMethodException, SecurityExcept
protected class AnnotationMockClass {

@NewSpan
public void getAnnotationForTagValueResolver(
@SpanTag(key = "test", resolver = TagValueResolver.class) String test) {
public void getAnnotationForValueResolver(@SpanTag(key = "test", resolver = ValueResolver.class) String test) {
}

@NewSpan
Expand All @@ -106,9 +105,9 @@ public void getAnnotationForArgumentToString(@SpanTag("test") Long param) {

}

static class SpelTagValueExpressionResolver implements TagValueExpressionResolver {
static class SpelValueExpressionResolver implements ValueExpressionResolver {

private static final Log log = LogFactory.getLog(SpelTagValueExpressionResolver.class);
private static final Log log = LogFactory.getLog(SpelValueExpressionResolver.class);

@Override
public String resolve(String expression, Object parameter) {
Expand Down

0 comments on commit 0b8b935

Please sign in to comment.