Skip to content

Commit

Permalink
Introduced result type for Java.
Browse files Browse the repository at this point in the history
This type provides for creating safer APIs.
Replaced APIs in Ditto tracing with Scala's `Try` as return value.

Signed-off-by: Juergen Fickel <juergen.fickel@bosch.io>
  • Loading branch information
Juergen Fickel committed Nov 4, 2022
1 parent ae2d2b7 commit 607f9ca
Show file tree
Hide file tree
Showing 17 changed files with 1,250 additions and 50 deletions.
5 changes: 5 additions & 0 deletions bom/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,11 @@
<version>${project.version}</version>
</dependency>

<dependency>
<groupId>org.eclipse.ditto</groupId>
<artifactId>ditto-utils-result</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.ditto</groupId>
<artifactId>ditto-internal-utils-akka</artifactId>
Expand Down
5 changes: 5 additions & 0 deletions internal/utils/tracing/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@
<artifactId>ditto-base-model</artifactId>
</dependency>

<dependency>
<groupId>org.eclipse.ditto</groupId>
<artifactId>ditto-utils-result</artifactId>
</dependency>

<dependency>
<groupId>org.eclipse.ditto</groupId>
<artifactId>ditto-internal-utils-metrics</artifactId>
Expand Down
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(
tryToGetKamonHttpContextPropagationOrThrow(propagationChannelName),
getKamonHttpContextPropagationOrThrow(propagationChannelName),
tracingConfig.getTracingFilter()
)
);
Expand All @@ -165,18 +165,12 @@ public void init(final TracingConfig tracingConfig) {
}
}

private static KamonHttpContextPropagation tryToGetKamonHttpContextPropagationOrThrow(
private static KamonHttpContextPropagation getKamonHttpContextPropagationOrThrow(
final CharSequence propagationChannelName
) {
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);
}
return KamonHttpContextPropagation.newInstanceForChannelName(propagationChannelName)
.mapErr(throwable -> new DittoConfigError(throwable.getMessage(), throwable))
.orElseThrow();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,14 @@ private static TracingFilter getConfigBasedTracingFilterOrThrow(final ConfigWith
if (filterConfig.isEmpty()) {
result = AcceptAllTracingFilter.getInstance();
} else {
final var kamonTracingFilterTry = KamonTracingFilter.tryFromConfig(filterConfig);
if (kamonTracingFilterTry.isSuccess()) {
result = kamonTracingFilterTry.get();
} else {
final var failed = kamonTracingFilterTry.failed();
final var failure = failed.get();
throw new DittoConfigError(
MessageFormat.format("Failed to get {0} from config: {1}",
TracingFilter.class.getSimpleName(),
failure.getMessage()),
failure
);
}
result = KamonTracingFilter.fromConfig(filterConfig)
.mapErr(throwable -> new DittoConfigError(
MessageFormat.format("Failed to get {0} from config: {1}",
TracingFilter.class.getSimpleName(),
throwable.getMessage()),
throwable
))
.orElseThrow();
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@

import org.eclipse.ditto.base.model.common.ConditionChecker;
import org.eclipse.ditto.internal.utils.tracing.span.SpanOperationName;
import org.eclipse.ditto.utils.result.Result;

import com.typesafe.config.Config;

import kamon.util.Filter;
import scala.util.Try;

/**
* This class wraps a Kamon {@link Filter} to implement {@link TracingFilter}.
Expand All @@ -41,30 +41,36 @@ private KamonTracingFilter(final Filter kamonFilter, final Config filterConfig)
}

/**
* Tries to return a new instance of {@code KamonTracingFilter} for the specified {@code Config} argument.
* If instantiation failed the returned {@code Try} is a failure.
* Creates an instance of {@code KamonTracingFilter} for the specified {@code Config} argument.
* If instantiation failed the returned {@code Result} is an Err.
*
* @param config the configuration which provides the includes and excludes of the returned filter.
* The configuration is supposed to have the following structure:
* <pre>
* config { includes = [ "some/pattern", "regex:some[0-9]" ] excludes = [ ] }
* </pre>
* @return a {@code Try} with the new KamonTracingFilter which is based on {@code config}.
* @return a {@code Result} with the new KamonTracingFilter which is based on {@code config}, if successful.
* Otherwise, an error result.
* @throws NullPointerException if {@code config} is {@code null}.
*/
public static Try<KamonTracingFilter> tryFromConfig(final Config config) {
public static Result<KamonTracingFilter, Throwable> fromConfig(final Config config) {

// NPE should not lead to a Try failure because passing a null config
// NPE should not lead to an Err result because passing a null config
// is a programming error.
ConditionChecker.checkNotNull(config, "config");
return Try.apply(() -> new KamonTracingFilter(Filter.from(validateConfig(config)), config));
return validateConfig(config).map(Filter::from).map(filter -> new KamonTracingFilter(filter, config));
}

private static Config validateConfig(final Config config) {
private static Result<Config, Throwable> validateConfig(final Config config) {
final Result<Config, Throwable> result;
if (!config.hasPath("includes") && !config.hasPath("excludes")) {
throw new IllegalArgumentException("Configuration is missing <includes> and <excludes> paths.");
result = Result.err(
new IllegalArgumentException("Configuration is missing <includes> and <excludes> paths.")
);
} else {
result = Result.ok(config);
}
return config;
return result;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

import javax.annotation.concurrent.NotThreadSafe;

import org.eclipse.ditto.utils.result.Result;

import kamon.Kamon;
import kamon.context.Context;
import kamon.context.HttpPropagation;
Expand All @@ -43,6 +45,31 @@ private KamonHttpContextPropagation(
this.propagation = propagation;
}

public static Result<KamonHttpContextPropagation, Throwable> newInstanceForChannelName(
final CharSequence propagationChannelName
) {
checkNotNull(propagationChannelName, "propagationChannelName");
return getHttpPropagation(propagationChannelName.toString()).map(KamonHttpContextPropagation::new);
}

private static Result<Propagation<HttpPropagation.HeaderReader, HttpPropagation.HeaderWriter>, Throwable> getHttpPropagation(
final String propagationChannelName
) {
final Result<Propagation<HttpPropagation.HeaderReader, HttpPropagation.HeaderWriter>, Throwable> result;
final var contextPropagationOption = Kamon.httpPropagation(propagationChannelName);
if (contextPropagationOption.isDefined()) {
result = Result.ok(contextPropagationOption.get());
} else {
result = Result.err(new IllegalArgumentException(
MessageFormat.format(
"HTTP propagation for channel name <{0}> is undefined.",
propagationChannelName
)
));
}
return result;
}

/**
* Returns a new instance of {@code KamonHttpContextPropagation} for the specified propagation channel name
* argument.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void ofWithConfigWithFilterReturnsExpectedTracingFilter() {
)));

assertThat(underTest.getTracingFilter())
.isEqualTo(KamonTracingFilter.tryFromConfig(ConfigFactory.parseMap(filterConfigMap)).get());
.isEqualTo(KamonTracingFilter.fromConfig(ConfigFactory.parseMap(filterConfigMap)).orElseThrow());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,27 +62,27 @@ public void testHashCodeAndEquals() {
@Test
public void fromConfigWithNullConfigThrowsNullPointerException() {
Assertions.assertThatNullPointerException()
.isThrownBy(() -> KamonTracingFilter.tryFromConfig(null))
.isThrownBy(() -> KamonTracingFilter.fromConfig(null))
.withMessage("The config must not be null!")
.withNoCause();
}

@Test
public void fromConfigWithEmptyConfigThrowsIllegalArgumentException() {
final var kamonTracingFilterTry = KamonTracingFilter.tryFromConfig(ConfigFactory.empty());
final var kamonTracingFilterResult = KamonTracingFilter.fromConfig(ConfigFactory.empty());

Assertions.assertThatIllegalArgumentException()
.isThrownBy(kamonTracingFilterTry::get)
.isThrownBy(kamonTracingFilterResult::orElseThrow)
.withMessage("Configuration is missing <includes> and <excludes> paths.")
.withNoCause();
}

@Test
public void fromConfigWithConfigContainingIncludesOnlyWorksAsExpected() {
final var includedOperationNames = List.of("foo", "bar", "baz");
final var kamonTracingFilterTry =
KamonTracingFilter.tryFromConfig(ConfigFactory.parseMap(Map.of("includes", includedOperationNames)));
final var underTest = kamonTracingFilterTry.get();
final var kamonTracingFilterResult =
KamonTracingFilter.fromConfig(ConfigFactory.parseMap(Map.of("includes", includedOperationNames)));
final var underTest = kamonTracingFilterResult.orElseThrow();

includedOperationNames.stream()
.map(SpanOperationName::of)
Expand All @@ -96,12 +96,11 @@ public void fromConfigWithConfigContainingIncludesOnlyWorksAsExpected() {
public void fromConfigWithConfigContainingIncludesAndExcludesWorksAsExpected() {
final var includes = List.of("foo", "bar", "baz", "c**");
final var excludedOperationNames = List.of("bar", "baz", "chaos");
final var kamonTracingFilterTry =
KamonTracingFilter.tryFromConfig(ConfigFactory.parseMap(Map.of(
final var kamonTracingFilterResult = KamonTracingFilter.fromConfig(ConfigFactory.parseMap(Map.of(
"includes", includes,
"excludes", excludedOperationNames
)));
final var underTest = kamonTracingFilterTry.get();
final var underTest = kamonTracingFilterResult.orElseThrow();

List.of("foo", "bar", "baz", "create", "chaos", "count").forEach(
operationName -> softly.assertThat(underTest.accept(SpanOperationName.of(operationName)))
Expand All @@ -115,12 +114,11 @@ public void fromConfigWithConfigContainingIncludesAndExcludesWorksAsExpected() {
@Test
public void fromConfigWithConfigContainingKleeneStarAsIncludesAndSomeExcludesWorksAsExpected() {
final var excludes = List.of("evil", "war");
final var kamonTracingFilterTry =
KamonTracingFilter.tryFromConfig(ConfigFactory.parseMap(Map.of(
"includes", List.of("*"),
"excludes", excludes
)));
final var kamonTracingFilter = kamonTracingFilterTry.get();
final var kamonTracingFilterTry = KamonTracingFilter.fromConfig(ConfigFactory.parseMap(Map.of(
"includes", List.of("*"),
"excludes", excludes
)));
final var kamonTracingFilter = kamonTracingFilterTry.orElseThrow();

excludes.forEach(
excluded -> softly.assertThat(kamonTracingFilter.accept(SpanOperationName.of(excluded)))
Expand Down
1 change: 1 addition & 0 deletions utils/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

<modules>
<module>jsr305</module>
<module>result</module>
</modules>

<properties>
Expand Down
71 changes: 71 additions & 0 deletions utils/result/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright (c) 2022 Contributors to the Eclipse Foundation
~
~ See the NOTICE file(s) distributed with this work for additional
~ information regarding copyright ownership.
~
~ This program and the accompanying materials are made available under the
~ terms of the Eclipse Public License 2.0 which is available at
~ http://www.eclipse.org/legal/epl-2.0
~
~ SPDX-License-Identifier: EPL-2.0
-->
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>org.eclipse.ditto</groupId>
<artifactId>ditto-utils</artifactId>
<version>${revision}</version>
</parent>

<artifactId>ditto-utils-result</artifactId>
<name>Eclipse Ditto :: Utils :: Result</name>

<properties>
<javac.source>17</javac.source>
<javac.target>17</javac.target>
</properties>

<dependencies>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>org.eclipse.ditto</groupId>
<artifactId>ditto-utils-jsr305</artifactId>
<scope>provided</scope>
</dependency>

<!-- Testing -->
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>nl.jqno.equalsverifier</groupId>
<artifactId>equalsverifier</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Loading

0 comments on commit 607f9ca

Please sign in to comment.