Skip to content

Commit 2f3cdb9

Browse files
kuba-wuanuraaga
kuba-wu
and
anuraaga
authored
SDK arch test - ensuring that API implmenetation returns only API types (open-telemetry#3146)
* SDK arch test - ensuring that API implmenetation returns only API types * added metrics verification * updated tests to match change in SDK / meter return type * typo corrected Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com> * typo corrected Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>
1 parent 1dbb9b4 commit 2f3cdb9

18 files changed

Lines changed: 260 additions & 108 deletions

‎all/src/test/java/io/opentelemetry/all/SdkDesignTest.java‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.all;
7+
8+
import com.tngtech.archunit.base.DescribedPredicate;
9+
import com.tngtech.archunit.base.Optional;
10+
import com.tngtech.archunit.base.PackageMatcher;
11+
import com.tngtech.archunit.core.domain.JavaClass;
12+
import com.tngtech.archunit.core.domain.JavaClassList;
13+
import com.tngtech.archunit.core.domain.JavaClasses;
14+
import com.tngtech.archunit.core.domain.JavaMethod;
15+
import com.tngtech.archunit.core.importer.ClassFileImporter;
16+
import com.tngtech.archunit.lang.syntax.ArchRuleDefinition;
17+
import com.tngtech.archunit.lang.syntax.elements.MethodsShouldConjunction;
18+
import java.util.ArrayList;
19+
import java.util.Arrays;
20+
import java.util.List;
21+
import org.junit.jupiter.api.Test;
22+
23+
public class SdkDesignTest {
24+
25+
private static final JavaClasses SDK_OTEL_CLASSES =
26+
new ClassFileImporter().importPackages("io.opentelemetry.sdk");
27+
28+
/**
29+
* Ensures that all SDK methods that: - are defined in classes that extend or implement API model
30+
* and are public (to exclude protected builders) - are public (avoids issues with protected
31+
* methods returning classes unavailable to test's CL) - override or implement parent method
32+
* return only API, Context or generic Java type.
33+
*/
34+
@Test
35+
void sdkImplementationOfApiClassesShouldReturnApiTypeOnly() {
36+
MethodsShouldConjunction covariantReturnRule =
37+
ArchRuleDefinition.methods()
38+
.that()
39+
.areDeclaredInClassesThat()
40+
.areAssignableTo(inPackage("io.opentelemetry.api.."))
41+
.and()
42+
.areDeclaredInClassesThat()
43+
.arePublic()
44+
.and()
45+
.arePublic()
46+
.and(implementOrOverride())
47+
.should()
48+
.haveRawReturnType(
49+
inPackage("io.opentelemetry.api..", "io.opentelemetry.context..", "java.."))
50+
.orShould()
51+
.haveRawReturnType("void");
52+
53+
covariantReturnRule.check(SDK_OTEL_CLASSES);
54+
}
55+
56+
static DescribedPredicate<? super JavaMethod> implementOrOverride() {
57+
return new DescribedPredicate<>("implement or override a method") {
58+
@Override
59+
public boolean apply(JavaMethod input) {
60+
JavaClassList params = input.getRawParameterTypes();
61+
Class<?>[] paramsType = new Class<?>[params.size()];
62+
for (int i = 0, n = params.size(); i < n; i++) {
63+
paramsType[i] = params.get(i).reflect();
64+
}
65+
String name = input.getName();
66+
67+
List<JavaClass> parents = new ArrayList<>(input.getOwner().getAllRawSuperclasses());
68+
parents.addAll(input.getOwner().getAllInterfaces());
69+
70+
for (JavaClass parent : parents) {
71+
Optional<JavaMethod> found = parent.tryGetMethod(name, paramsType);
72+
if (found.isPresent()) {
73+
return true;
74+
}
75+
}
76+
return false;
77+
}
78+
};
79+
}
80+
81+
static DescribedPredicate<? super JavaClass> inPackage(String... requiredPackages) {
82+
return new DescribedPredicate<>("are in " + Arrays.toString(requiredPackages)) {
83+
@Override
84+
public boolean apply(JavaClass member) {
85+
for (String requiredPackage : requiredPackages) {
86+
if (PackageMatcher.of(requiredPackage).matches(member.getPackageName())) {
87+
return true;
88+
}
89+
}
90+
return false;
91+
}
92+
};
93+
}
94+
}

‎sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/SdkMeterProvider.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package io.opentelemetry.sdk.metrics;
77

88
import io.opentelemetry.api.OpenTelemetry;
9+
import io.opentelemetry.api.metrics.Meter;
910
import io.opentelemetry.api.metrics.MeterProvider;
1011
import io.opentelemetry.sdk.common.Clock;
1112
import io.opentelemetry.sdk.internal.ComponentRegistry;
@@ -51,12 +52,12 @@ public final class SdkMeterProvider implements MeterProvider, MetricProducer {
5152
}
5253

5354
@Override
54-
public SdkMeter get(String instrumentationName) {
55+
public Meter get(String instrumentationName) {
5556
return get(instrumentationName, null);
5657
}
5758

5859
@Override
59-
public SdkMeter get(String instrumentationName, @Nullable String instrumentationVersion) {
60+
public Meter get(String instrumentationName, @Nullable String instrumentationVersion) {
6061
// Per the spec, both null and empty are "invalid" and a "default" should be used.
6162
if (instrumentationName == null || instrumentationName.isEmpty()) {
6263
LOGGER.fine("Meter requested without instrumentation name.");

‎sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/BatchRecorderSdkTest.java‎

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@
1111

1212
import io.opentelemetry.api.common.Attributes;
1313
import io.opentelemetry.api.metrics.BatchRecorder;
14+
import io.opentelemetry.api.metrics.DoubleCounter;
15+
import io.opentelemetry.api.metrics.DoubleUpDownCounter;
16+
import io.opentelemetry.api.metrics.DoubleValueRecorder;
17+
import io.opentelemetry.api.metrics.LongCounter;
18+
import io.opentelemetry.api.metrics.LongUpDownCounter;
19+
import io.opentelemetry.api.metrics.LongValueRecorder;
20+
import io.opentelemetry.api.metrics.Meter;
1421
import io.opentelemetry.api.metrics.common.Labels;
1522
import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
1623
import io.opentelemetry.sdk.internal.TestClock;
@@ -38,7 +45,7 @@ class BatchRecorderSdkTest {
3845
private final TestClock testClock = TestClock.create();
3946
private final SdkMeterProvider sdkMeterProvider =
4047
SdkMeterProvider.builder().setClock(testClock).setResource(RESOURCE).build();
41-
private final SdkMeter sdkMeter = sdkMeterProvider.get(getClass().getName());
48+
private final Meter sdkMeter = sdkMeterProvider.get(getClass().getName());
4249

4350
@Test
4451
void batchRecorder_badLabelSet() {
@@ -49,15 +56,15 @@ void batchRecorder_badLabelSet() {
4956

5057
@Test
5158
void batchRecorder() {
52-
DoubleCounterSdk doubleCounter = sdkMeter.doubleCounterBuilder("testDoubleCounter").build();
53-
LongCounterSdk longCounter = sdkMeter.longCounterBuilder("testLongCounter").build();
54-
DoubleUpDownCounterSdk doubleUpDownCounter =
59+
DoubleCounter doubleCounter = sdkMeter.doubleCounterBuilder("testDoubleCounter").build();
60+
LongCounter longCounter = sdkMeter.longCounterBuilder("testLongCounter").build();
61+
DoubleUpDownCounter doubleUpDownCounter =
5562
sdkMeter.doubleUpDownCounterBuilder("testDoubleUpDownCounter").build();
56-
LongUpDownCounterSdk longUpDownCounter =
63+
LongUpDownCounter longUpDownCounter =
5764
sdkMeter.longUpDownCounterBuilder("testLongUpDownCounter").build();
58-
DoubleValueRecorderSdk doubleValueRecorder =
65+
DoubleValueRecorder doubleValueRecorder =
5966
sdkMeter.doubleValueRecorderBuilder("testDoubleValueRecorder").build();
60-
LongValueRecorderSdk longValueRecorder =
67+
LongValueRecorder longValueRecorder =
6168
sdkMeter.longValueRecorderBuilder("testLongValueRecorder").build();
6269
Labels labelSet = Labels.of("key", "value");
6370

@@ -102,15 +109,15 @@ void batchRecorder() {
102109
}
103110

104111
private void assertBatchRecordings(
105-
DoubleCounterSdk doubleCounter,
106-
LongCounterSdk longCounter,
107-
DoubleUpDownCounterSdk doubleUpDownCounter,
108-
LongUpDownCounterSdk longUpDownCounter,
109-
DoubleValueRecorderSdk doubleValueRecorder,
110-
LongValueRecorderSdk longValueRecorder,
112+
DoubleCounter doubleCounter,
113+
LongCounter longCounter,
114+
DoubleUpDownCounter doubleUpDownCounter,
115+
LongUpDownCounter longUpDownCounter,
116+
DoubleValueRecorder doubleValueRecorder,
117+
LongValueRecorder longValueRecorder,
111118
Labels labelSet,
112119
boolean shouldHaveDeltas) {
113-
assertThat(doubleCounter.collectAll(testClock.now()))
120+
assertThat(((AbstractInstrument) doubleCounter).collectAll(testClock.now()))
114121
.containsExactly(
115122
MetricData.createDoubleSum(
116123
RESOURCE,
@@ -124,7 +131,7 @@ private void assertBatchRecordings(
124131
Collections.singletonList(
125132
DoublePointData.create(
126133
testClock.now(), testClock.now(), labelSet, 24.2d)))));
127-
assertThat(longCounter.collectAll(testClock.now()))
134+
assertThat(((AbstractInstrument) longCounter).collectAll(testClock.now()))
128135
.containsExactly(
129136
MetricData.createLongSum(
130137
RESOURCE,
@@ -137,7 +144,7 @@ private void assertBatchRecordings(
137144
AggregationTemporality.CUMULATIVE,
138145
Collections.singletonList(
139146
LongPointData.create(testClock.now(), testClock.now(), labelSet, 12)))));
140-
assertThat(doubleUpDownCounter.collectAll(testClock.now()))
147+
assertThat(((AbstractInstrument) doubleUpDownCounter).collectAll(testClock.now()))
141148
.containsExactly(
142149
MetricData.createDoubleSum(
143150
RESOURCE,
@@ -151,7 +158,7 @@ private void assertBatchRecordings(
151158
Collections.singletonList(
152159
DoublePointData.create(
153160
testClock.now(), testClock.now(), labelSet, -12.1d)))));
154-
assertThat(longUpDownCounter.collectAll(testClock.now()))
161+
assertThat(((AbstractInstrument) longUpDownCounter).collectAll(testClock.now()))
155162
.containsExactly(
156163
MetricData.createLongSum(
157164
RESOURCE,
@@ -166,7 +173,7 @@ private void assertBatchRecordings(
166173
LongPointData.create(testClock.now(), testClock.now(), labelSet, -12)))));
167174

168175
if (shouldHaveDeltas) {
169-
assertThat(doubleValueRecorder.collectAll(testClock.now()))
176+
assertThat(((AbstractInstrument) doubleValueRecorder).collectAll(testClock.now()))
170177
.containsExactly(
171178
MetricData.createDoubleSummary(
172179
RESOURCE,
@@ -186,11 +193,11 @@ private void assertBatchRecordings(
186193
ValueAtPercentile.create(0.0, 13.1),
187194
ValueAtPercentile.create(100.0, 13.1)))))));
188195
} else {
189-
assertThat(doubleValueRecorder.collectAll(testClock.now())).isEmpty();
196+
assertThat(((AbstractInstrument) doubleValueRecorder).collectAll(testClock.now())).isEmpty();
190197
}
191198

192199
if (shouldHaveDeltas) {
193-
assertThat(longValueRecorder.collectAll(testClock.now()))
200+
assertThat(((AbstractInstrument) longValueRecorder).collectAll(testClock.now()))
194201
.containsExactly(
195202
MetricData.createDoubleSummary(
196203
RESOURCE,
@@ -210,7 +217,7 @@ private void assertBatchRecordings(
210217
ValueAtPercentile.create(0.0, 13),
211218
ValueAtPercentile.create(100.0, 13)))))));
212219
} else {
213-
assertThat(longValueRecorder.collectAll(testClock.now())).isEmpty();
220+
assertThat(((AbstractInstrument) longValueRecorder).collectAll(testClock.now())).isEmpty();
214221
}
215222
}
216223
}

‎sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/DoubleCounterSdkTest.java‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import io.opentelemetry.api.common.Attributes;
1313
import io.opentelemetry.api.metrics.BoundDoubleCounter;
1414
import io.opentelemetry.api.metrics.DoubleCounter;
15+
import io.opentelemetry.api.metrics.Meter;
1516
import io.opentelemetry.api.metrics.common.Labels;
1617
import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
1718
import io.opentelemetry.sdk.internal.TestClock;
@@ -35,7 +36,7 @@ class DoubleCounterSdkTest {
3536
private final TestClock testClock = TestClock.create();
3637
private final SdkMeterProvider sdkMeterProvider =
3738
SdkMeterProvider.builder().setClock(testClock).setResource(RESOURCE).build();
38-
private final SdkMeter sdkMeter = sdkMeterProvider.get(getClass().getName());
39+
private final Meter sdkMeter = sdkMeterProvider.get(getClass().getName());
3940

4041
@Test
4142
void add_PreventNullLabels() {
@@ -53,7 +54,7 @@ void bound_PreventNullLabels() {
5354

5455
@Test
5556
void collectMetrics_NoRecords() {
56-
DoubleCounterSdk doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
57+
DoubleCounter doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
5758
BoundDoubleCounter bound = doubleCounter.bind(Labels.of("foo", "bar"));
5859
try {
5960
assertThat(sdkMeterProvider.collectAllMetrics()).isEmpty();
@@ -64,7 +65,7 @@ void collectMetrics_NoRecords() {
6465

6566
@Test
6667
void collectMetrics_WithEmptyLabel() {
67-
DoubleCounterSdk doubleCounter =
68+
DoubleCounter doubleCounter =
6869
sdkMeter
6970
.doubleCounterBuilder("testCounter")
7071
.setDescription("description")
@@ -97,7 +98,7 @@ void collectMetrics_WithEmptyLabel() {
9798
@Test
9899
void collectMetrics_WithMultipleCollects() {
99100
long startTime = testClock.now();
100-
DoubleCounterSdk doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
101+
DoubleCounter doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
101102
BoundDoubleCounter bound = doubleCounter.bind(Labels.of("K", "V"));
102103
try {
103104
// Do some records using bounds and direct calls and bindings.
@@ -152,26 +153,28 @@ void collectMetrics_WithMultipleCollects() {
152153

153154
@Test
154155
void doubleCounterAdd_Monotonicity() {
155-
DoubleCounterSdk doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
156+
DoubleCounter doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
156157

157158
assertThatThrownBy(() -> doubleCounter.add(-45.77d, Labels.empty()))
158159
.isInstanceOf(IllegalArgumentException.class);
159160
}
160161

161162
@Test
162163
void boundDoubleCounterAdd_Monotonicity() {
163-
DoubleCounterSdk doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
164+
DoubleCounter doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
164165

165166
assertThatThrownBy(() -> doubleCounter.bind(Labels.empty()).add(-9.3))
166167
.isInstanceOf(IllegalArgumentException.class);
167168
}
168169

169170
@Test
170171
void stressTest() {
171-
final DoubleCounterSdk doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
172+
final DoubleCounter doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
172173

173174
StressTestRunner.Builder stressTestBuilder =
174-
StressTestRunner.builder().setInstrument(doubleCounter).setCollectionIntervalMs(100);
175+
StressTestRunner.builder()
176+
.setInstrument((DoubleCounterSdk) doubleCounter)
177+
.setCollectionIntervalMs(100);
175178

176179
for (int i = 0; i < 4; i++) {
177180
stressTestBuilder.addOperation(
@@ -203,10 +206,12 @@ void stressTest() {
203206
void stressTest_WithDifferentLabelSet() {
204207
final String[] keys = {"Key_1", "Key_2", "Key_3", "Key_4"};
205208
final String[] values = {"Value_1", "Value_2", "Value_3", "Value_4"};
206-
final DoubleCounterSdk doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
209+
final DoubleCounter doubleCounter = sdkMeter.doubleCounterBuilder("testCounter").build();
207210

208211
StressTestRunner.Builder stressTestBuilder =
209-
StressTestRunner.builder().setInstrument(doubleCounter).setCollectionIntervalMs(100);
212+
StressTestRunner.builder()
213+
.setInstrument((DoubleCounterSdk) doubleCounter)
214+
.setCollectionIntervalMs(100);
210215

211216
for (int i = 0; i < 4; i++) {
212217
stressTestBuilder.addOperation(

‎sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/DoubleSumObserverSdkTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import static org.assertj.core.api.Assertions.assertThat;
1010

1111
import io.opentelemetry.api.common.Attributes;
12+
import io.opentelemetry.api.metrics.Meter;
1213
import io.opentelemetry.api.metrics.common.Labels;
1314
import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
1415
import io.opentelemetry.sdk.internal.TestClock;
@@ -30,7 +31,7 @@ class DoubleSumObserverSdkTest {
3031
private final TestClock testClock = TestClock.create();
3132
private final SdkMeterProvider sdkMeterProvider =
3233
SdkMeterProvider.builder().setClock(testClock).setResource(RESOURCE).build();
33-
private final SdkMeter sdkMeter = sdkMeterProvider.get(getClass().getName());
34+
private final Meter sdkMeter = sdkMeterProvider.get(getClass().getName());
3435

3536
@Test
3637
void collectMetrics_NoCallback() {

0 commit comments

Comments
 (0)