Skip to content

Commit

Permalink
Made static factory method of KamonHttpContextPropagation less erro…
Browse files Browse the repository at this point in the history
…r-prone.

Runtime exceptions get easily neglected by mistake. Returning a `Try` instead makes it obvious that the instance create might fail.

Signed-off-by: Juergen Fickel <juergen.fickel@bosch.io>
  • Loading branch information
Juergen Fickel committed Nov 3, 2022
1 parent 7f3817c commit ae2d2b7
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public void init(final TracingConfig tracingConfig) {
final var propagationChannelName = tracingConfig.getPropagationChannel();
newStateConsumer.accept(
new TracingEnabledState(
tryToGetKamonHttpContextPropagation(propagationChannelName),
tryToGetKamonHttpContextPropagationOrThrow(propagationChannelName),
tracingConfig.getTracingFilter()
)
);
Expand All @@ -165,13 +165,17 @@ public void init(final TracingConfig tracingConfig) {
}
}

private static KamonHttpContextPropagation tryToGetKamonHttpContextPropagation(
private static KamonHttpContextPropagation tryToGetKamonHttpContextPropagationOrThrow(
final CharSequence propagationChannelName
) {
try {
return KamonHttpContextPropagation.newInstanceForChannelName(propagationChannelName);
} catch (final IllegalArgumentException e) {
throw new DittoConfigError(e.getMessage(), e);
final var kamonHttpContextPropagationTry =
KamonHttpContextPropagation.tryNewInstanceForChannelName(propagationChannelName);
if (kamonHttpContextPropagationTry.isSuccess()) {
return kamonHttpContextPropagationTry.get();
} else {
final var failed = kamonHttpContextPropagationTry.failed();
final var throwable = failed.get();
throw new DittoConfigError(throwable.getMessage(), throwable);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import kamon.context.Propagation;
import scala.Option;
import scala.jdk.javaapi.CollectionConverters;
import scala.util.Try;

/**
* This class provides the means to read {@link Context} from a Map of headers as well as propagating {@code Context}
Expand All @@ -47,13 +48,17 @@ private KamonHttpContextPropagation(
* argument.
*
* @param propagationChannelName configured name of the HTTP propagation channel.
* @return the new instance.
* @return a {@code Try} with the new instance if successful or an {@code IllegalArgumentException} if
* {@code propagationChannelName} is undefined.
* @throws NullPointerException if {@code propagationChannelName} is {@code null}.
* @throws IllegalArgumentException if {@code propagationChannelName} is undefined.
*/
public static KamonHttpContextPropagation newInstanceForChannelName(final CharSequence propagationChannelName) {
public static Try<KamonHttpContextPropagation> tryNewInstanceForChannelName(
final CharSequence propagationChannelName
) {
checkNotNull(propagationChannelName, "propagationChannelName");
return new KamonHttpContextPropagation(getHttpPropagationOrThrow(propagationChannelName.toString()));
return Try.apply(
() -> new KamonHttpContextPropagation(getHttpPropagationOrThrow(propagationChannelName.toString()))
);
}

private static Propagation<HttpPropagation.HeaderReader, HttpPropagation.HeaderWriter> getHttpPropagationOrThrow(
Expand All @@ -72,6 +77,15 @@ private static Propagation<HttpPropagation.HeaderReader, HttpPropagation.HeaderW
}
}

/**
* Returns an instance of {@code KamonHttpContextPropagation} for the default HTTP propagation channel.
*
* @return the instance.
*/
static KamonHttpContextPropagation getInstanceForDefaultHttpChannel() {
return new KamonHttpContextPropagation(Kamon.defaultHttpPropagation());
}

/**
* Reads context information from the specified map of headers.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public final class KamonHttpContextPropagationTest {
KamonTracingInitResource.KamonTracingConfig.defaultValues()
);

private static final String DEFAULT_CHANNEL_NAME = "default";
private static final String CONTEXT_TAGS_KEY = "context-tags";
private static final String TRACEPARENT_KEY = DittoHeaderDefinition.W3C_TRACEPARENT.getKey();

Expand All @@ -70,23 +69,26 @@ public static void beforeClass() {
@Test
public void newInstanceForNullChannelNameThrowsNullPointerException() {
assertThatNullPointerException()
.isThrownBy(() -> KamonHttpContextPropagation.newInstanceForChannelName(null))
.isThrownBy(() -> KamonHttpContextPropagation.tryNewInstanceForChannelName(null))
.withMessage("The propagationChannelName must not be null!")
.withNoCause();
}

@Test
public void newInstanceForUnknownChannelNameThrowsIllegalArgumentException() {
public void newInstanceForUnknownChannelNameReturnsFailedWithIllegalArgumentException() {
final var channelName = "zoeglfrex";
final var kamonHttpContextPropagationTry =
KamonHttpContextPropagation.tryNewInstanceForChannelName(channelName);

assertThatIllegalArgumentException()
.isThrownBy(() -> KamonHttpContextPropagation.newInstanceForChannelName(channelName))
.isThrownBy(kamonHttpContextPropagationTry::get)
.withMessage("HTTP propagation for channel name <%s> is undefined.", channelName)
.withNoCause();
}

@Test
public void getContextFromHeadersForNullHeadersThrowsNullPointerException() {
final var underTest = KamonHttpContextPropagation.newInstanceForChannelName(DEFAULT_CHANNEL_NAME);
final var underTest = KamonHttpContextPropagation.getInstanceForDefaultHttpChannel();

assertThatNullPointerException()
.isThrownBy(() -> underTest.getContextFromHeaders(null))
Expand All @@ -96,7 +98,7 @@ public void getContextFromHeadersForNullHeadersThrowsNullPointerException() {

@Test
public void getContextFromEmptyMapReturnsEmptyContext() {
final var underTest = KamonHttpContextPropagation.newInstanceForChannelName(DEFAULT_CHANNEL_NAME);
final var underTest = KamonHttpContextPropagation.getInstanceForDefaultHttpChannel();

final var context = underTest.getContextFromHeaders(Map.of());

Expand All @@ -111,7 +113,7 @@ public void getContextFromMapWithCustomHeadersReturnsExpectedContext() {
"marco", "polo"
);
final var spanId = spanIdFactory.generate();
final var underTest = KamonHttpContextPropagation.newInstanceForChannelName(DEFAULT_CHANNEL_NAME);
final var underTest = KamonHttpContextPropagation.getInstanceForDefaultHttpChannel();

final var context = underTest.getContextFromHeaders(
Map.of(
Expand Down Expand Up @@ -143,7 +145,7 @@ private static String getAsTraceparentHeaderValue(final Identifier traceId, fina

@Test
public void propagateContextToHeadersWithNullContextThrowsNullPointerException() {
final var underTest = KamonHttpContextPropagation.newInstanceForChannelName(DEFAULT_CHANNEL_NAME);
final var underTest = KamonHttpContextPropagation.getInstanceForDefaultHttpChannel();

assertThatNullPointerException()
.isThrownBy(() -> underTest.propagateContextToHeaders(null, Map.of()))
Expand All @@ -153,7 +155,7 @@ public void propagateContextToHeadersWithNullContextThrowsNullPointerException()

@Test
public void propagateContextToHeadersWithNullMapThrowsNullPointerException() {
final var underTest = KamonHttpContextPropagation.newInstanceForChannelName(DEFAULT_CHANNEL_NAME);
final var underTest = KamonHttpContextPropagation.getInstanceForDefaultHttpChannel();

assertThatNullPointerException()
.isThrownBy(() -> underTest.propagateContextToHeaders(Context.Empty(), null))
Expand All @@ -170,7 +172,7 @@ public void propagateContextToMapReturnsExpectedMap() {
);
final var span = Kamon.spanBuilder(testName.getMethodName()).traceId(traceId).start();
final var dittoHeaders = DittoHeaders.newBuilder().correlationId(testName.getMethodName()).build();
final var underTest = KamonHttpContextPropagation.newInstanceForChannelName(DEFAULT_CHANNEL_NAME);
final var underTest = KamonHttpContextPropagation.getInstanceForDefaultHttpChannel();

final var headersWithPropagatedContext = underTest.propagateContextToHeaders(
Context.of(Span.Key(), span, TagSet.from(contextTags)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void setup() {
underTest = PreparedKamonSpan.newInstance(
Map.of(),
SpanOperationName.of(testName.getMethodName()),
KamonHttpContextPropagation.newInstanceForChannelName("default")
KamonHttpContextPropagation.getInstanceForDefaultHttpChannel()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void setup() {
Kamon.spanBuilder(testName.getMethodName())
.asChildOf(Kamon.spanBuilder("/").traceId(traceId).start())
.start(),
KamonHttpContextPropagation.newInstanceForChannelName("default")
KamonHttpContextPropagation.getInstanceForDefaultHttpChannel()
);
}

Expand Down

0 comments on commit ae2d2b7

Please sign in to comment.