From b92864cb28df27ec21307ca75e4e6a5e8131d9b9 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Fri, 10 Mar 2023 13:31:04 -0600 Subject: [PATCH 1/2] JaegerRemoteSampler only uses upstream grpc implementation if ManagedChannel is set --- .../sampler/DefaultGrpcServiceBuilder.java | 164 ----------------- .../trace/jaeger/sampler/GrpcService.java | 26 +-- .../jaeger/sampler/GrpcServiceBuilder.java | 36 ---- .../trace/jaeger/sampler/GrpcServiceUtil.java | 51 ------ .../jaeger/sampler/JaegerRemoteSampler.java | 7 +- .../sampler/JaegerRemoteSamplerBuilder.java | 85 ++++++--- .../jaeger/sampler/OkHttpGrpcService.java | 20 +- .../sampler/OkHttpGrpcServiceBuilder.java | 172 ------------------ .../SamplingStrategyResponseUnMarshaler.java | 3 +- .../trace/jaeger/sampler/UnMarshaler.java | 19 -- ...cService.java => UpstreamGrpcService.java} | 27 +-- .../sampler/JaegerRemoteSamplerTest.java | 72 +++++++- .../sampler/OkHttpGrpcServiceBuilderTest.java | 111 ----------- .../DefaultGrpcServiceBuilderTest.java | 119 ------------ .../JaegerRemoteSamplerGrpcNettyTest.java | 97 +++------- 15 files changed, 181 insertions(+), 828 deletions(-) delete mode 100644 sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilder.java delete mode 100644 sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcServiceBuilder.java delete mode 100644 sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcServiceUtil.java delete mode 100644 sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcServiceBuilder.java delete mode 100644 sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/UnMarshaler.java rename sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/{DefaultGrpcService.java => UpstreamGrpcService.java} (74%) delete mode 100644 sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcServiceBuilderTest.java delete mode 100644 sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilderTest.java diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilder.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilder.java deleted file mode 100644 index e0d39cfa23b..00000000000 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilder.java +++ /dev/null @@ -1,164 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.sdk.extension.trace.jaeger.sampler; - -import static io.grpc.Metadata.ASCII_STRING_MARSHALLER; -import static io.opentelemetry.api.internal.Utils.checkArgument; -import static io.opentelemetry.exporter.internal.grpc.ManagedChannelUtil.toServiceConfig; -import static java.util.Objects.requireNonNull; - -import io.grpc.Codec; -import io.grpc.ManagedChannel; -import io.grpc.ManagedChannelBuilder; -import io.grpc.Metadata; -import io.grpc.stub.MetadataUtils; -import io.opentelemetry.exporter.internal.ExporterBuilderUtil; -import io.opentelemetry.exporter.internal.TlsConfigHelper; -import io.opentelemetry.exporter.internal.grpc.ManagedChannelUtil; -import io.opentelemetry.exporter.internal.grpc.MarshalerServiceStub; -import io.opentelemetry.exporter.internal.marshal.Marshaler; -import io.opentelemetry.exporter.internal.retry.RetryPolicy; -import java.net.URI; -import java.time.Duration; -import java.util.concurrent.TimeUnit; -import java.util.function.Function; -import javax.annotation.Nullable; - -final class DefaultGrpcServiceBuilder - implements GrpcServiceBuilder { - - private final String type; - private final Function> stubFactory; - private final String grpcServiceName; - - @Nullable private ManagedChannel channel; - private long timeoutNanos; - private URI endpoint; - private boolean compressionEnabled = false; - @Nullable private Metadata metadata; - @Nullable private RetryPolicy retryPolicy; - private final TlsConfigHelper tlsConfigHelper = new TlsConfigHelper(); - - // Visible for testing - DefaultGrpcServiceBuilder( - String type, - Function> stubFactory, - long defaultTimeoutSecs, - URI defaultEndpoint, - String grpcServiceName) { - this.type = type; - this.stubFactory = stubFactory; - this.grpcServiceName = grpcServiceName; - timeoutNanos = TimeUnit.SECONDS.toNanos(defaultTimeoutSecs); - endpoint = defaultEndpoint; - } - - @Override - public DefaultGrpcServiceBuilder setChannel(ManagedChannel channel) { - requireNonNull(channel, "channel"); - this.channel = channel; - return this; - } - - @Override - public DefaultGrpcServiceBuilder setTimeout(long timeout, TimeUnit unit) { - requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); - timeoutNanos = unit.toNanos(timeout); - return this; - } - - @Override - public DefaultGrpcServiceBuilder setTimeout(Duration timeout) { - requireNonNull(timeout, "timeout"); - checkArgument(!timeout.isNegative(), "timeout must be non-negative"); - return setTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS); - } - - @Override - public DefaultGrpcServiceBuilder setEndpoint(String endpoint) { - requireNonNull(endpoint, "endpoint"); - this.endpoint = ExporterBuilderUtil.validateEndpoint(endpoint); - return this; - } - - @Override - public DefaultGrpcServiceBuilder setCompression(String compressionMethod) { - requireNonNull(compressionMethod, "compressionMethod"); - checkArgument( - compressionMethod.equals("gzip") || compressionMethod.equals("none"), - "Unsupported compression method. Supported compression methods include: gzip, none."); - this.compressionEnabled = true; - return this; - } - - @Override - public DefaultGrpcServiceBuilder setTrustedCertificates( - byte[] trustedCertificatesPem) { - requireNonNull(trustedCertificatesPem, "trustedCertificatesPem"); - tlsConfigHelper.createTrustManager(trustedCertificatesPem); - return this; - } - - @Override - public GrpcServiceBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem); - return this; - } - - @Override - public DefaultGrpcServiceBuilder addHeader(String key, String value) { - requireNonNull(key, "key"); - requireNonNull(value, "value"); - if (metadata == null) { - metadata = new Metadata(); - } - metadata.put(Metadata.Key.of(key, ASCII_STRING_MARSHALLER), value); - return this; - } - - @Override - public DefaultGrpcServiceBuilder addRetryPolicy(RetryPolicy retryPolicy) { - requireNonNull(retryPolicy, "retryPolicy"); - this.retryPolicy = retryPolicy; - return this; - } - - @Override - public GrpcService build() { - ManagedChannel channel = this.channel; - if (channel == null) { - ManagedChannelBuilder managedChannelBuilder = - ManagedChannelBuilder.forTarget(endpoint.getAuthority()); - - if (endpoint.getScheme().equals("https")) { - managedChannelBuilder.useTransportSecurity(); - } else { - managedChannelBuilder.usePlaintext(); - } - - if (metadata != null) { - managedChannelBuilder.intercept(MetadataUtils.newAttachHeadersInterceptor(metadata)); - } - - tlsConfigHelper.configureWithKeyManager( - (tm, km) -> - ManagedChannelUtil.setClientKeysAndTrustedCertificatesPem( - managedChannelBuilder, tm, km)); - - if (retryPolicy != null) { - managedChannelBuilder.defaultServiceConfig(toServiceConfig(grpcServiceName, retryPolicy)); - } - - channel = managedChannelBuilder.build(); - } - - Codec codec = compressionEnabled ? new Codec.Gzip() : Codec.Identity.NONE; - MarshalerServiceStub stub = - stubFactory.apply(channel).withCompression(codec.getMessageEncoding()); - return new DefaultGrpcService<>(type, channel, stub, timeoutNanos); - } -} diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcService.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcService.java index 95ab116a445..b40df754ea2 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcService.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcService.java @@ -5,37 +5,17 @@ package io.opentelemetry.sdk.extension.trace.jaeger.sampler; -import io.grpc.ManagedChannel; -import io.opentelemetry.exporter.internal.grpc.GrpcExporterBuilder; -import io.opentelemetry.exporter.internal.grpc.MarshalerServiceStub; import io.opentelemetry.exporter.internal.marshal.Marshaler; import io.opentelemetry.sdk.common.CompletableResultCode; -import java.net.URI; -import java.util.function.Function; -import java.util.function.Supplier; -interface GrpcService { - - /** Returns a new {@link GrpcExporterBuilder}. */ - static - GrpcServiceBuilder builder( - String type, - long defaultTimeoutSecs, - URI defaultEndpoint, - Supplier< - Function>> - stubFactory, - String grpcServiceName, - String grpcEndpointPath) { - return GrpcServiceUtil.serviceBuilder( - type, defaultTimeoutSecs, defaultEndpoint, stubFactory, grpcServiceName, grpcEndpointPath); - } +interface GrpcService { /** * Exports the {@code exportRequest} which is a request {@link Marshaler} for {@code numItems} * items. */ - ResUnMarshalerT execute(ReqMarshalerT request, ResUnMarshalerT response); + SamplingStrategyResponseUnMarshaler execute( + SamplingStrategyParametersMarshaler request, SamplingStrategyResponseUnMarshaler response); /** Shuts the exporter down. */ CompletableResultCode shutdown(); diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcServiceBuilder.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcServiceBuilder.java deleted file mode 100644 index a977c1d2e71..00000000000 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcServiceBuilder.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.sdk.extension.trace.jaeger.sampler; - -import io.grpc.ManagedChannel; -import io.opentelemetry.exporter.internal.marshal.Marshaler; -import io.opentelemetry.exporter.internal.retry.RetryPolicy; -import java.time.Duration; -import java.util.concurrent.TimeUnit; - -interface GrpcServiceBuilder { - GrpcServiceBuilder setChannel(ManagedChannel channel); - - GrpcServiceBuilder setTimeout(long timeout, TimeUnit unit); - - GrpcServiceBuilder setTimeout(Duration timeout); - - GrpcServiceBuilder setEndpoint(String endpoint); - - GrpcServiceBuilder setCompression(String compressionMethod); - - GrpcServiceBuilder setTrustedCertificates( - byte[] trustedCertificatesPem); - - GrpcServiceBuilder setClientTls( - byte[] privateKeyPem, byte[] certificatePem); - - GrpcServiceBuilder addHeader(String key, String value); - - GrpcServiceBuilder addRetryPolicy(RetryPolicy retryPolicy); - - GrpcService build(); -} diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcServiceUtil.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcServiceUtil.java deleted file mode 100644 index 4d7fe4f0139..00000000000 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/GrpcServiceUtil.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.sdk.extension.trace.jaeger.sampler; - -import io.grpc.ManagedChannel; -import io.opentelemetry.exporter.internal.grpc.MarshalerServiceStub; -import io.opentelemetry.exporter.internal.marshal.Marshaler; -import java.net.URI; -import java.util.function.Function; -import java.util.function.Supplier; - -final class GrpcServiceUtil { - - private static final boolean USE_OKHTTP; - - static { - boolean useOkhttp = true; - // Use the OkHttp exporter unless grpc-stub is on the classpath. - try { - Class.forName("io.grpc.stub.AbstractStub"); - useOkhttp = false; - } catch (ClassNotFoundException e) { - // Fall through - } - USE_OKHTTP = useOkhttp; - } - - static - GrpcServiceBuilder serviceBuilder( - String type, - long defaultTimeoutSecs, - URI defaultEndpoint, - Supplier< - Function>> - stubFactory, - String grpcServiceName, - String grpcEndpointPath) { - if (USE_OKHTTP) { - return new OkHttpGrpcServiceBuilder<>( - type, grpcEndpointPath, defaultTimeoutSecs, defaultEndpoint); - } else { - return new DefaultGrpcServiceBuilder<>( - type, stubFactory.get(), defaultTimeoutSecs, defaultEndpoint, grpcServiceName); - } - } - - private GrpcServiceUtil() {} -} diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java index 05f4c5e3f5d..e787f35924a 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java @@ -36,13 +36,10 @@ public final class JaegerRemoteSampler implements Sampler, Closeable { private volatile Sampler sampler; - private final GrpcService< - SamplingStrategyParametersMarshaler, SamplingStrategyResponseUnMarshaler> - delegate; + private final GrpcService delegate; JaegerRemoteSampler( - GrpcService - delegate, + GrpcService delegate, @Nullable String serviceName, int pollingIntervalMs, Sampler initialSampler) { diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java index d1756c40151..6d48c90a3b2 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java @@ -9,11 +9,19 @@ import io.grpc.ManagedChannel; import io.opentelemetry.api.internal.Utils; +import io.opentelemetry.exporter.internal.ExporterBuilderUtil; +import io.opentelemetry.exporter.internal.TlsConfigHelper; +import io.opentelemetry.exporter.internal.okhttp.OkHttpUtil; import io.opentelemetry.sdk.trace.samplers.Sampler; import java.net.URI; import java.time.Duration; +import java.util.Arrays; +import java.util.Collections; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; +import okhttp3.Headers; +import okhttp3.OkHttpClient; +import okhttp3.Protocol; /** A builder for {@link JaegerRemoteSampler}. */ public final class JaegerRemoteSamplerBuilder { @@ -27,15 +35,17 @@ public final class JaegerRemoteSamplerBuilder { private static final int DEFAULT_POLLING_INTERVAL_MILLIS = 60000; private static final Sampler INITIAL_SAMPLER = Sampler.parentBased(Sampler.traceIdRatioBased(0.001)); + private static final long DEFAULT_TIMEOUT_SECS = 10; - @Nullable private String serviceName; + private URI endpoint = DEFAULT_ENDPOINT; private Sampler initialSampler = INITIAL_SAMPLER; private int pollingIntervalMillis = DEFAULT_POLLING_INTERVAL_MILLIS; - private static final long DEFAULT_TIMEOUT_SECS = 10; + private final TlsConfigHelper tlsConfigHelper = new TlsConfigHelper(); - private final GrpcServiceBuilder< - SamplingStrategyParametersMarshaler, SamplingStrategyResponseUnMarshaler> - delegate; + @Nullable private String serviceName; + + // Use Object type since gRPC may not be on the classpath. + @Nullable private Object grpcChannel; /** * Sets the service name to be used by this exporter. Required. @@ -54,14 +64,14 @@ public JaegerRemoteSamplerBuilder setServiceName(String serviceName) { */ public JaegerRemoteSamplerBuilder setEndpoint(String endpoint) { requireNonNull(endpoint, "endpoint"); - delegate.setEndpoint(endpoint); + this.endpoint = ExporterBuilderUtil.validateEndpoint(endpoint); return this; } /** Sets trusted certificate. */ public JaegerRemoteSamplerBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { requireNonNull(trustedCertificatesPem, "trustedCertificatesPem"); - delegate.setTrustedCertificates(trustedCertificatesPem); + tlsConfigHelper.createTrustManager(trustedCertificatesPem); return this; } @@ -72,7 +82,9 @@ public JaegerRemoteSamplerBuilder setTrustedCertificates(byte[] trustedCertifica * @since 1.24.0 */ public JaegerRemoteSamplerBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - delegate.setClientTls(privateKeyPem, certificatePem); + requireNonNull(privateKeyPem, "privateKeyPem"); + requireNonNull(certificatePem, "certificatePem"); + tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem); return this; } @@ -116,7 +128,7 @@ public JaegerRemoteSamplerBuilder setInitialSampler(Sampler initialSampler) { @Deprecated public JaegerRemoteSamplerBuilder setChannel(ManagedChannel channel) { requireNonNull(channel, "channel"); - delegate.setChannel(channel); + this.grpcChannel = channel; return this; } @@ -126,24 +138,47 @@ public JaegerRemoteSamplerBuilder setChannel(ManagedChannel channel) { * @return the remote sampler instance. */ public JaegerRemoteSampler build() { - return new JaegerRemoteSampler( - delegate.build(), serviceName, pollingIntervalMillis, initialSampler); - } + if (grpcChannel != null) { + return new JaegerRemoteSampler( + UpstreamGrpcExporterFactory.buildWithChannel((ManagedChannel) grpcChannel), + serviceName, + pollingIntervalMillis, + initialSampler); + } - JaegerRemoteSamplerBuilder() { - delegate = - GrpcService.builder( - "remoteSampling", - DEFAULT_TIMEOUT_SECS, - DEFAULT_ENDPOINT, - () -> MarshallerRemoteSamplerServiceGrpc::newFutureStub, - GRPC_SERVICE_NAME, - GRPC_ENDPOINT_PATH); + OkHttpClient.Builder clientBuilder = + new OkHttpClient.Builder().dispatcher(OkHttpUtil.newDispatcher()); + + clientBuilder.callTimeout(Duration.ofNanos(TimeUnit.SECONDS.toNanos(DEFAULT_TIMEOUT_SECS))); + + tlsConfigHelper.configureWithSocketFactory(clientBuilder::sslSocketFactory); + + String endpoint = this.endpoint.resolve(GRPC_ENDPOINT_PATH).toString(); + if (endpoint.startsWith("http://")) { + clientBuilder.protocols(Collections.singletonList(Protocol.H2_PRIOR_KNOWLEDGE)); + } else { + clientBuilder.protocols(Arrays.asList(Protocol.HTTP_2, Protocol.HTTP_1_1)); + } + + Headers.Builder headers = new Headers.Builder(); + headers.add("te", "trailers"); + + return new JaegerRemoteSampler( + new OkHttpGrpcService("remoteSampling", clientBuilder.build(), endpoint, headers.build()), + serviceName, + pollingIntervalMillis, + initialSampler); } - // Visible for testing - GrpcServiceBuilder - getDelegate() { - return delegate; + // Use an inner class to ensure GrpcExporterBuilder does not have classloading dependencies on + // upstream gRPC. + private static class UpstreamGrpcExporterFactory { + private static GrpcService buildWithChannel(ManagedChannel channel) { + return new UpstreamGrpcService( + "remoteSampling", + channel, + MarshallerRemoteSamplerServiceGrpc.newFutureStub(channel), + TimeUnit.SECONDS.toNanos(DEFAULT_TIMEOUT_SECS)); + } } } diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcService.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcService.java index 8d91b48807f..9b32aeefb26 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcService.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcService.java @@ -7,7 +7,6 @@ import io.opentelemetry.exporter.internal.grpc.GrpcRequestBody; import io.opentelemetry.exporter.internal.grpc.GrpcStatusUtil; -import io.opentelemetry.exporter.internal.marshal.Marshaler; import io.opentelemetry.exporter.internal.retry.RetryUtil; import io.opentelemetry.sdk.common.CompletableResultCode; import java.io.ByteArrayInputStream; @@ -28,8 +27,7 @@ import okio.GzipSource; import okio.Okio; -final class OkHttpGrpcService - implements GrpcService { +final class OkHttpGrpcService implements GrpcService { private static final String GRPC_STATUS = "grpc-status"; private static final String GRPC_MESSAGE = "grpc-message"; @@ -40,28 +38,22 @@ final class OkHttpGrpcService - implements GrpcServiceBuilder { - - private final String type; - private final String grpcEndpointPath; - - private long timeoutNanos; - private URI endpoint; - private boolean compressionEnabled = false; - private final Headers.Builder headers = new Headers.Builder(); - @Nullable private byte[] trustedCertificatesPem; - @Nullable private byte[] privateKeyPem; - @Nullable private byte[] certificatePem; - @Nullable private RetryPolicy retryPolicy; - - OkHttpGrpcServiceBuilder( - String type, String grpcEndpointPath, long defaultTimeoutSecs, URI defaultEndpoint) { - this.type = type; - this.grpcEndpointPath = grpcEndpointPath; - timeoutNanos = TimeUnit.SECONDS.toNanos(defaultTimeoutSecs); - endpoint = defaultEndpoint; - } - - @Override - public GrpcServiceBuilder setChannel(ManagedChannel channel) { - throw new UnsupportedOperationException("Only available on DefaultGrpcService"); - } - - @Override - public OkHttpGrpcServiceBuilder setTimeout( - long timeout, TimeUnit unit) { - requireNonNull(unit, "unit"); - checkArgument(timeout >= 0, "timeout must be non-negative"); - timeoutNanos = unit.toNanos(timeout); - return this; - } - - @Override - public OkHttpGrpcServiceBuilder setTimeout(Duration timeout) { - requireNonNull(timeout, "timeout"); - checkArgument(!timeout.isNegative(), "timeout must be non-negative"); - return setTimeout(timeout.toNanos(), TimeUnit.NANOSECONDS); - } - - @Override - public OkHttpGrpcServiceBuilder setEndpoint(String endpoint) { - requireNonNull(endpoint, "endpoint"); - this.endpoint = ExporterBuilderUtil.validateEndpoint(endpoint); - return this; - } - - @Override - public OkHttpGrpcServiceBuilder setCompression( - String compressionMethod) { - requireNonNull(compressionMethod, "compressionMethod"); - checkArgument( - compressionMethod.equals("gzip") || compressionMethod.equals("none"), - "Unsupported compression method. Supported compression methods include: gzip, none."); - this.compressionEnabled = true; - return this; - } - - @Override - public OkHttpGrpcServiceBuilder setTrustedCertificates( - byte[] trustedCertificatesPem) { - requireNonNull(trustedCertificatesPem, "trustedCertificatesPem"); - this.trustedCertificatesPem = trustedCertificatesPem; - return this; - } - - @Override - public GrpcServiceBuilder setClientTls( - byte[] privateKeyPem, byte[] certificatePem) { - requireNonNull(privateKeyPem, "privateKeyPem"); - requireNonNull(certificatePem, "certificatePem"); - this.privateKeyPem = privateKeyPem; - this.certificatePem = certificatePem; - return this; - } - - @Override - public OkHttpGrpcServiceBuilder addHeader( - String key, String value) { - requireNonNull(key, "key"); - requireNonNull(value, "value"); - headers.add(key, value); - return this; - } - - @Override - public OkHttpGrpcServiceBuilder addRetryPolicy( - RetryPolicy retryPolicy) { - requireNonNull(retryPolicy, "retryPolicy"); - this.retryPolicy = retryPolicy; - return this; - } - - @Override - public GrpcService build() { - OkHttpClient.Builder clientBuilder = - new OkHttpClient.Builder().dispatcher(OkHttpUtil.newDispatcher()); - - clientBuilder.callTimeout(Duration.ofNanos(timeoutNanos)); - - if (trustedCertificatesPem != null) { - try { - X509TrustManager trustManager = TlsUtil.trustManager(trustedCertificatesPem); - X509KeyManager keyManager = null; - if (privateKeyPem != null && certificatePem != null) { - keyManager = TlsUtil.keyManager(privateKeyPem, certificatePem); - } - clientBuilder.sslSocketFactory( - TlsUtil.sslSocketFactory(keyManager, trustManager), trustManager); - } catch (SSLException e) { - throw new IllegalStateException( - "Could not set trusted certificates, are they valid X.509 in PEM format?", e); - } - } - - String endpoint = this.endpoint.resolve(grpcEndpointPath).toString(); - if (endpoint.startsWith("http://")) { - clientBuilder.protocols(Collections.singletonList(Protocol.H2_PRIOR_KNOWLEDGE)); - } else { - clientBuilder.protocols(Arrays.asList(Protocol.HTTP_2, Protocol.HTTP_1_1)); - } - - headers.add("te", "trailers"); - if (compressionEnabled) { - headers.add("grpc-encoding", "gzip"); - } - - if (retryPolicy != null) { - clientBuilder.addInterceptor( - new RetryInterceptor(retryPolicy, OkHttpGrpcExporter::isRetryable)); - } - - return new OkHttpGrpcService<>( - type, clientBuilder.build(), endpoint, headers.build(), compressionEnabled); - } -} diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/SamplingStrategyResponseUnMarshaler.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/SamplingStrategyResponseUnMarshaler.java index 454b1f916cc..0c06df5c16a 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/SamplingStrategyResponseUnMarshaler.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/SamplingStrategyResponseUnMarshaler.java @@ -9,7 +9,7 @@ import java.io.IOException; import javax.annotation.Nullable; -class SamplingStrategyResponseUnMarshaler extends UnMarshaler { +class SamplingStrategyResponseUnMarshaler { @Nullable private SamplingStrategyResponse samplingStrategyResponse; @@ -18,7 +18,6 @@ public SamplingStrategyResponse get() { return samplingStrategyResponse; } - @Override public void read(byte[] payload) throws IOException { SamplingStrategyResponse.Builder responseBuilder = new SamplingStrategyResponse.Builder(); try { diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/UnMarshaler.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/UnMarshaler.java deleted file mode 100644 index 89b3c2496d4..00000000000 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/UnMarshaler.java +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.sdk.extension.trace.jaeger.sampler; - -import java.io.IOException; - -/** - * UnMarshaler from protobuf wire format to SDK data type. - * - *

This class is internal and is hence not for public use. Its APIs are unstable and can change - * at any time. - */ -abstract class UnMarshaler { - - abstract void read(byte[] payload) throws IOException; -} diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcService.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/UpstreamGrpcService.java similarity index 74% rename from sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcService.java rename to sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/UpstreamGrpcService.java index 1a75796a14f..32b19e4c0bc 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcService.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/UpstreamGrpcService.java @@ -10,27 +10,29 @@ import io.grpc.Status; import io.opentelemetry.exporter.internal.grpc.ManagedChannelUtil; import io.opentelemetry.exporter.internal.grpc.MarshalerServiceStub; -import io.opentelemetry.exporter.internal.marshal.Marshaler; import io.opentelemetry.sdk.common.CompletableResultCode; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; -final class DefaultGrpcService - implements GrpcService { +final class UpstreamGrpcService implements GrpcService { - private static final Logger logger = Logger.getLogger(DefaultGrpcService.class.getName()); + private static final Logger logger = Logger.getLogger(UpstreamGrpcService.class.getName()); private final String type; private final ManagedChannel managedChannel; - private final MarshalerServiceStub stub; + private final MarshalerServiceStub< + SamplingStrategyParametersMarshaler, SamplingStrategyResponseUnMarshaler, ?> + stub; private final long timeoutNanos; - /** Creates a new {@link DefaultGrpcService}. */ - DefaultGrpcService( + /** Creates a new {@link UpstreamGrpcService}. */ + UpstreamGrpcService( String type, ManagedChannel channel, - MarshalerServiceStub stub, + MarshalerServiceStub< + SamplingStrategyParametersMarshaler, SamplingStrategyResponseUnMarshaler, ?> + stub, long timeoutNanos) { this.type = type; this.managedChannel = channel; @@ -39,9 +41,12 @@ final class DefaultGrpcService stub = this.stub; + public SamplingStrategyResponseUnMarshaler execute( + SamplingStrategyParametersMarshaler exportRequest, + SamplingStrategyResponseUnMarshaler responseUnmarshaller) { + MarshalerServiceStub< + SamplingStrategyParametersMarshaler, SamplingStrategyResponseUnMarshaler, ?> + stub = this.stub; if (timeoutNanos > 0) { stub = stub.withDeadlineAfter(timeoutNanos, TimeUnit.NANOSECONDS); } diff --git a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerTest.java b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerTest.java index 0eb50298e57..ce0a9a6d523 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerTest.java +++ b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerTest.java @@ -6,6 +6,7 @@ package io.opentelemetry.sdk.extension.trace.jaeger.sampler; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Named.named; @@ -25,7 +26,6 @@ import io.opentelemetry.sdk.extension.trace.jaeger.proto.api_v2.Sampling.SamplingStrategyType; import io.opentelemetry.sdk.trace.samplers.Sampler; import java.io.IOException; -import java.lang.reflect.Field; import java.nio.file.Files; import java.time.Duration; import java.util.concurrent.CompletableFuture; @@ -134,6 +134,7 @@ void connectionWorks() { .setPollingInterval(1, TimeUnit.SECONDS) .setServiceName(SERVICE_NAME) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -151,6 +152,7 @@ void tlsConnectionWorks() throws IOException { .setTrustedCertificates(Files.readAllBytes(certificate.certificateFile().toPath())) .setServiceName(SERVICE_NAME) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -171,6 +173,7 @@ void clientTlsConnectionWorks(byte[] privateKey) throws IOException { privateKey, Files.readAllBytes(clientCertificate.certificateFile().toPath())) .setServiceName(SERVICE_NAME) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -197,6 +200,8 @@ void description() { .setPollingInterval(1, TimeUnit.SECONDS) .setServiceName(SERVICE_NAME) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); + assertThat(sampler.getDescription()) .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); @@ -213,6 +218,8 @@ void initialSampler() { .setServiceName(SERVICE_NAME) .setInitialSampler(Sampler.alwaysOn()) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); + assertThat(sampler.getDescription()).startsWith("JaegerRemoteSampler{AlwaysOnSampler}"); } } @@ -225,6 +232,7 @@ void pollingInterval() { .setServiceName(SERVICE_NAME) .setPollingInterval(1, TimeUnit.MILLISECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); // wait until the sampling strategy is retrieved before exiting test method await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -239,6 +247,7 @@ void pollingInterval_duration() { .setServiceName(SERVICE_NAME) .setPollingInterval(Duration.ofMillis(1)) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); // wait until the sampling strategy is retrieved before exiting test method await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -283,6 +292,7 @@ void perOperationSampling() { // Make sure only polls once. .setPollingInterval(500, TimeUnit.SECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); await() .untilAsserted( @@ -306,6 +316,7 @@ void internal_error_server_response() { .setServiceName(SERVICE_NAME) .setPollingInterval(50, TimeUnit.MILLISECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); assertThat(sampler.getDescription()) .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); @@ -331,6 +342,7 @@ void unavailable_error_server_response() { .setServiceName(SERVICE_NAME) .setPollingInterval(50, TimeUnit.MILLISECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); assertThat(sampler.getDescription()) .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); @@ -355,6 +367,7 @@ void unimplemented_error_server_response() { .setServiceName(SERVICE_NAME) .setPollingInterval(50, TimeUnit.MILLISECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(OkHttpGrpcService.class); assertThat(sampler.getDescription()) .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); @@ -368,14 +381,52 @@ void unimplemented_error_server_response() { } } + @Test + void builder_ValidConfig() { + assertThatCode(() -> JaegerRemoteSampler.builder().setEndpoint("http://localhost:4317")) + .doesNotThrowAnyException(); + assertThatCode(() -> JaegerRemoteSampler.builder().setEndpoint("http://localhost")) + .doesNotThrowAnyException(); + assertThatCode(() -> JaegerRemoteSampler.builder().setEndpoint("https://localhost")) + .doesNotThrowAnyException(); + assertThatCode(() -> JaegerRemoteSampler.builder().setEndpoint("http://foo:bar@localhost")) + .doesNotThrowAnyException(); + + assertThatCode( + () -> + JaegerRemoteSampler.builder() + .setTrustedCertificates( + Files.readAllBytes(certificate.certificateFile().toPath()))) + .doesNotThrowAnyException(); + + assertThatCode( + () -> + JaegerRemoteSampler.builder() + .setClientTls( + Files.readAllBytes(clientCertificate.privateKeyFile().toPath()), + Files.readAllBytes(clientCertificate.certificateFile().toPath()))) + .doesNotThrowAnyException(); + } + @Test void invalidArguments() { assertThatThrownBy(() -> JaegerRemoteSampler.builder().setServiceName(null)) .isInstanceOf(NullPointerException.class) .hasMessage("serviceName"); + assertThatThrownBy(() -> JaegerRemoteSampler.builder().setEndpoint(null)) .isInstanceOf(NullPointerException.class) .hasMessage("endpoint"); + assertThatThrownBy(() -> JaegerRemoteSampler.builder().setEndpoint("😺://localhost")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid endpoint, must be a URL: 😺://localhost"); + assertThatThrownBy(() -> JaegerRemoteSampler.builder().setEndpoint("localhost")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid endpoint, must start with http:// or https://: localhost"); + assertThatThrownBy(() -> JaegerRemoteSampler.builder().setEndpoint("gopher://localhost")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid endpoint, must start with http:// or https://: gopher://localhost"); + assertThatThrownBy( () -> JaegerRemoteSampler.builder().setPollingInterval(-1, TimeUnit.MILLISECONDS)) .isInstanceOf(IllegalArgumentException.class) @@ -390,12 +441,17 @@ void invalidArguments() { assertThatThrownBy(() -> JaegerRemoteSampler.builder().setPollingInterval(null)) .isInstanceOf(NullPointerException.class) .hasMessage("interval"); - } - @Test - void usingOkHttp() { - assertThat(JaegerRemoteSampler.builder().getDelegate()) - .isInstanceOf(OkHttpGrpcServiceBuilder.class); + assertThatThrownBy(() -> JaegerRemoteSampler.builder().setTrustedCertificates(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("trustedCertificatesPem"); + + assertThatThrownBy(() -> JaegerRemoteSampler.builder().setClientTls(null, new byte[] {})) + .isInstanceOf(NullPointerException.class) + .hasMessage("privateKeyPem"); + assertThatThrownBy(() -> JaegerRemoteSampler.builder().setClientTls(new byte[] {}, null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("certificatePem"); } static ThrowingRunnable samplerIsType( @@ -404,9 +460,7 @@ static ThrowingRunnable samplerIsType( assertThat(sampler.getSampler().getClass().getName()) .isEqualTo("io.opentelemetry.sdk.trace.samplers.ParentBasedSampler"); - Field field = sampler.getSampler().getClass().getDeclaredField("root"); - field.setAccessible(true); - assertThat(field.get(sampler.getSampler()).getClass()).isEqualTo(expected); + assertThat(sampler.getSampler()).extracting("root").isInstanceOf(expected); }; } } diff --git a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcServiceBuilderTest.java b/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcServiceBuilderTest.java deleted file mode 100644 index eab10fd1a16..00000000000 --- a/sdk-extensions/jaeger-remote-sampler/src/test/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcServiceBuilderTest.java +++ /dev/null @@ -1,111 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.sdk.extension.trace.jaeger.sampler; - -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -import io.opentelemetry.exporter.internal.retry.RetryPolicy; -import java.net.URI; -import java.nio.charset.StandardCharsets; -import java.time.Duration; -import java.util.concurrent.TimeUnit; -import org.junit.jupiter.api.Test; - -class OkHttpGrpcServiceBuilderTest { - - private static OkHttpGrpcServiceBuilder< - SamplingStrategyParametersMarshaler, SamplingStrategyResponseUnMarshaler> - exporterBuilder() { - return new OkHttpGrpcServiceBuilder<>("some", "some", 1, URI.create("htt://localhost:8080")); - } - - @Test - @SuppressWarnings("PreferJavaTimeOverload") - void validConfig() { - assertThatCode(() -> exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS)) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(0))) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setTimeout(10, TimeUnit.MILLISECONDS)) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(10))) - .doesNotThrowAnyException(); - - assertThatCode(() -> exporterBuilder().setEndpoint("http://localhost:4317")) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setEndpoint("http://localhost")) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setEndpoint("https://localhost")) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setEndpoint("http://foo:bar@localhost")) - .doesNotThrowAnyException(); - - assertThatCode(() -> exporterBuilder().setCompression("gzip")).doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setCompression("none")).doesNotThrowAnyException(); - - assertThatCode(() -> exporterBuilder().addHeader("foo", "bar").addHeader("baz", "qux")) - .doesNotThrowAnyException(); - - assertThatCode( - () -> - exporterBuilder().setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8))) - .doesNotThrowAnyException(); - - assertThatCode(() -> exporterBuilder().addRetryPolicy(RetryPolicy.getDefault())) - .doesNotThrowAnyException(); - } - - @Test - @SuppressWarnings({"PreferJavaTimeOverload", "NullAway"}) - public void invalidConfig() { - assertThatThrownBy(() -> exporterBuilder().setTimeout(-1, TimeUnit.MILLISECONDS)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("timeout must be non-negative"); - assertThatThrownBy(() -> exporterBuilder().setTimeout(1, null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("unit"); - assertThatThrownBy(() -> exporterBuilder().setTimeout(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("timeout"); - - assertThatThrownBy(() -> exporterBuilder().addRetryPolicy(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("retryPolicy"); - - assertThatThrownBy(() -> exporterBuilder().addHeader(null, "val")) - .isInstanceOf(NullPointerException.class) - .hasMessage("key"); - assertThatThrownBy(() -> exporterBuilder().addHeader("key", null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("value"); - - assertThatThrownBy(() -> exporterBuilder().setTrustedCertificates(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("trustedCertificatesPem"); - - assertThatThrownBy(() -> exporterBuilder().setEndpoint(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("endpoint"); - assertThatThrownBy(() -> exporterBuilder().setEndpoint("😺://localhost")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid endpoint, must be a URL: 😺://localhost"); - assertThatThrownBy(() -> exporterBuilder().setEndpoint("localhost")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid endpoint, must start with http:// or https://: localhost"); - assertThatThrownBy(() -> exporterBuilder().setEndpoint("gopher://localhost")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid endpoint, must start with http:// or https://: gopher://localhost"); - - assertThatThrownBy(() -> exporterBuilder().setCompression(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("compressionMethod"); - assertThatThrownBy(() -> exporterBuilder().setCompression("foo")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Unsupported compression method. Supported compression methods include: gzip, none."); - } -} diff --git a/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilderTest.java b/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilderTest.java deleted file mode 100644 index efe3bc193b6..00000000000 --- a/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilderTest.java +++ /dev/null @@ -1,119 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.sdk.extension.trace.jaeger.sampler; - -import static org.assertj.core.api.Assertions.assertThatCode; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -import com.linecorp.armeria.testing.junit5.server.SelfSignedCertificateExtension; -import io.opentelemetry.exporter.internal.retry.RetryPolicy; -import java.net.URI; -import java.time.Duration; -import java.util.concurrent.TimeUnit; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -class DefaultGrpcServiceBuilderTest { - - @RegisterExtension - static final SelfSignedCertificateExtension serverTls = new SelfSignedCertificateExtension(); - - private static DefaultGrpcServiceBuilder< - SamplingStrategyParametersMarshaler, SamplingStrategyResponseUnMarshaler> - exporterBuilder() { - return new DefaultGrpcServiceBuilder<>( - "some", null, 10, URI.create("htt://localhost:8080"), "some"); - } - - @Test - @SuppressWarnings("PreferJavaTimeOverload") - void validConfig() { - assertThatCode(() -> exporterBuilder().setTimeout(0, TimeUnit.MILLISECONDS)) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(0))) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setTimeout(10, TimeUnit.MILLISECONDS)) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setTimeout(Duration.ofMillis(10))) - .doesNotThrowAnyException(); - - assertThatCode(() -> exporterBuilder().setEndpoint("http://localhost:4317")) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setEndpoint("http://localhost")) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setEndpoint("https://localhost")) - .doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setEndpoint("http://foo:bar@localhost")) - .doesNotThrowAnyException(); - - assertThatCode(() -> exporterBuilder().setCompression("gzip")).doesNotThrowAnyException(); - assertThatCode(() -> exporterBuilder().setCompression("none")).doesNotThrowAnyException(); - - assertThatCode(() -> exporterBuilder().addHeader("foo", "bar").addHeader("baz", "qux")) - .doesNotThrowAnyException(); - - assertThatCode( - () -> exporterBuilder().setTrustedCertificates(serverTls.certificate().getEncoded())) - .doesNotThrowAnyException(); - - assertThatCode(() -> exporterBuilder().addRetryPolicy(RetryPolicy.getDefault())) - .doesNotThrowAnyException(); - } - - @Test - @SuppressWarnings({"PreferJavaTimeOverload", "NullAway"}) - public void invalidConfig() { - assertThatThrownBy(() -> exporterBuilder().setTimeout(-1, TimeUnit.MILLISECONDS)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("timeout must be non-negative"); - assertThatThrownBy(() -> exporterBuilder().setTimeout(1, null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("unit"); - assertThatThrownBy(() -> exporterBuilder().setTimeout(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("timeout"); - - assertThatThrownBy(() -> exporterBuilder().addRetryPolicy(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("retryPolicy"); - - assertThatThrownBy(() -> exporterBuilder().addHeader(null, "val")) - .isInstanceOf(NullPointerException.class) - .hasMessage("key"); - assertThatThrownBy(() -> exporterBuilder().addHeader("key", null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("value"); - - assertThatThrownBy(() -> exporterBuilder().setTrustedCertificates(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("trustedCertificatesPem"); - - assertThatThrownBy(() -> exporterBuilder().setEndpoint(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("endpoint"); - assertThatThrownBy(() -> exporterBuilder().setEndpoint("😺://localhost")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid endpoint, must be a URL: 😺://localhost"); - assertThatThrownBy(() -> exporterBuilder().setEndpoint("localhost")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid endpoint, must start with http:// or https://: localhost"); - assertThatThrownBy(() -> exporterBuilder().setEndpoint("gopher://localhost")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Invalid endpoint, must start with http:// or https://: gopher://localhost"); - - assertThatThrownBy(() -> exporterBuilder().setCompression(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("compressionMethod"); - assertThatThrownBy(() -> exporterBuilder().setCompression("foo")) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - "Unsupported compression method. Supported compression methods include: gzip, none."); - - assertThatThrownBy(() -> exporterBuilder().setChannel(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("channel"); - } -} diff --git a/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerGrpcNettyTest.java b/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerGrpcNettyTest.java index d0b5f31ec5e..a83d03f3514 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerGrpcNettyTest.java +++ b/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerGrpcNettyTest.java @@ -6,7 +6,6 @@ package io.opentelemetry.sdk.extension.trace.jaeger.sampler; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Awaitility.await; import com.linecorp.armeria.common.grpc.protocol.ArmeriaStatusException; @@ -16,14 +15,13 @@ import com.linecorp.armeria.testing.junit5.server.SelfSignedCertificateExtension; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.github.netmikey.logunit.api.LogCapturer; +import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.extension.trace.jaeger.proto.api_v2.Sampling; import io.opentelemetry.sdk.extension.trace.jaeger.proto.api_v2.Sampling.RateLimitingSamplingStrategy; import io.opentelemetry.sdk.extension.trace.jaeger.proto.api_v2.Sampling.SamplingStrategyType; import io.opentelemetry.sdk.trace.samplers.Sampler; -import java.io.IOException; -import java.lang.reflect.Field; -import java.nio.file.Files; import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; @@ -38,7 +36,8 @@ import org.slf4j.event.Level; import org.slf4j.event.LoggingEvent; -@SuppressLogger(DefaultGrpcService.class) +@SuppressLogger(UpstreamGrpcService.class) +@SuppressWarnings("deprecation") // Testing deprecated code class JaegerRemoteSamplerGrpcNettyTest { private static final String SERVICE_NAME = "my-service"; @@ -55,7 +54,7 @@ private static void addGrpcError(int code, @Nullable String message) { } @RegisterExtension - LogCapturer logs = LogCapturer.create().captureForType(DefaultGrpcService.class, Level.TRACE); + LogCapturer logs = LogCapturer.create().captureForType(UpstreamGrpcService.class, Level.TRACE); @Order(1) @RegisterExtension @@ -96,8 +95,6 @@ protected CompletionStage handleMessage( } }); sb.http(0); - sb.https(0); - sb.tls(certificate.certificateFile(), certificate.privateKeyFile()); } }; @@ -107,31 +104,20 @@ public void before() { responses.clear(); } - @Test - void connectionWorks() { - try (JaegerRemoteSampler sampler = - JaegerRemoteSampler.builder() - .setEndpoint(server.httpUri().toString()) - .setPollingInterval(1, TimeUnit.SECONDS) - .setServiceName(SERVICE_NAME) - .build()) { - - await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); - - // verify - assertThat(sampler.getDescription()).contains("RateLimitingSampler{999.00}"); - } + private static ManagedChannel managedChannel() { + return ManagedChannelBuilder.forTarget(server.httpUri().getAuthority()).usePlaintext().build(); } @Test - void tlsConnectionWorks() throws IOException { + void connectionWorks() { try (JaegerRemoteSampler sampler = JaegerRemoteSampler.builder() - .setEndpoint(server.httpsUri().toString()) + .setChannel(managedChannel()) + .setEndpoint(server.httpUri().toString()) .setPollingInterval(1, TimeUnit.SECONDS) - .setTrustedCertificates(Files.readAllBytes(certificate.certificateFile().toPath())) .setServiceName(SERVICE_NAME) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(UpstreamGrpcService.class); await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -144,10 +130,12 @@ void tlsConnectionWorks() throws IOException { void description() { try (JaegerRemoteSampler sampler = JaegerRemoteSampler.builder() - .setEndpoint(server.httpUri().toString()) + .setChannel(managedChannel()) .setPollingInterval(1, TimeUnit.SECONDS) .setServiceName(SERVICE_NAME) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(UpstreamGrpcService.class); + assertThat(sampler.getDescription()) .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); @@ -160,10 +148,12 @@ void description() { void initialSampler() { try (JaegerRemoteSampler sampler = JaegerRemoteSampler.builder() - .setEndpoint("http://example.com") + .setChannel(managedChannel()) .setServiceName(SERVICE_NAME) .setInitialSampler(Sampler.alwaysOn()) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(UpstreamGrpcService.class); + assertThat(sampler.getDescription()).startsWith("JaegerRemoteSampler{AlwaysOnSampler}"); } } @@ -172,10 +162,11 @@ void initialSampler() { void pollingInterval() { try (JaegerRemoteSampler sampler = JaegerRemoteSampler.builder() - .setEndpoint(server.httpUri().toString()) + .setChannel(managedChannel()) .setServiceName(SERVICE_NAME) .setPollingInterval(1, TimeUnit.MILLISECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(UpstreamGrpcService.class); // wait until the sampling strategy is retrieved before exiting test method await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -186,10 +177,11 @@ void pollingInterval() { void pollingInterval_duration() { try (JaegerRemoteSampler sampler = JaegerRemoteSampler.builder() - .setEndpoint(server.httpUri().toString()) + .setChannel(managedChannel()) .setServiceName(SERVICE_NAME) .setPollingInterval(Duration.ofMillis(1)) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(UpstreamGrpcService.class); // wait until the sampling strategy is retrieved before exiting test method await().untilAsserted(samplerIsType(sampler, RateLimitingSampler.class)); @@ -229,11 +221,12 @@ void perOperationSampling() { try (JaegerRemoteSampler sampler = JaegerRemoteSampler.builder() - .setEndpoint(server.httpUri().toString()) + .setChannel(managedChannel()) .setServiceName(SERVICE_NAME) // Make sure only polls once. .setPollingInterval(500, TimeUnit.SECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(UpstreamGrpcService.class); await() .untilAsserted( @@ -252,10 +245,11 @@ void internal_error_server_response() { try (JaegerRemoteSampler sampler = JaegerRemoteSampler.builder() - .setEndpoint(server.httpUri().toString()) + .setChannel(managedChannel()) .setServiceName(SERVICE_NAME) .setPollingInterval(50, TimeUnit.MILLISECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(UpstreamGrpcService.class); assertThat(sampler.getDescription()) .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); @@ -276,10 +270,11 @@ void unavailable_error_server_response() { try (JaegerRemoteSampler sampler = JaegerRemoteSampler.builder() - .setEndpoint(server.httpUri().toString()) + .setChannel(managedChannel()) .setServiceName(SERVICE_NAME) .setPollingInterval(50, TimeUnit.MILLISECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(UpstreamGrpcService.class); assertThat(sampler.getDescription()) .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); @@ -299,10 +294,11 @@ void unimplemented_error_server_response() { try (JaegerRemoteSampler sampler = JaegerRemoteSampler.builder() - .setEndpoint(server.httpUri().toString()) + .setChannel(managedChannel()) .setServiceName(SERVICE_NAME) .setPollingInterval(50, TimeUnit.MILLISECONDS) .build()) { + assertThat(sampler).extracting("delegate").isInstanceOf(UpstreamGrpcService.class); assertThat(sampler.getDescription()) .startsWith("JaegerRemoteSampler{ParentBased{root:TraceIdRatioBased{0.001000}"); @@ -316,45 +312,12 @@ void unimplemented_error_server_response() { } } - @Test - void invalidArguments() { - assertThatThrownBy(() -> JaegerRemoteSampler.builder().setServiceName(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("serviceName"); - assertThatThrownBy(() -> JaegerRemoteSampler.builder().setEndpoint(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("endpoint"); - assertThatThrownBy( - () -> JaegerRemoteSampler.builder().setPollingInterval(-1, TimeUnit.MILLISECONDS)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("polling interval must be positive"); - assertThatThrownBy(() -> JaegerRemoteSampler.builder().setPollingInterval(1, null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("unit"); - assertThatThrownBy( - () -> JaegerRemoteSampler.builder().setPollingInterval(Duration.ofMillis(-1))) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("polling interval must be positive"); - assertThatThrownBy(() -> JaegerRemoteSampler.builder().setPollingInterval(null)) - .isInstanceOf(NullPointerException.class) - .hasMessage("interval"); - } - - @Test - void usingGrpc() { - assertThat(JaegerRemoteSampler.builder().getDelegate()) - .isInstanceOf(DefaultGrpcServiceBuilder.class); - } - static ThrowingRunnable samplerIsType( JaegerRemoteSampler sampler, Class expected) { return () -> { assertThat(sampler.getSampler().getClass().getName()) .isEqualTo("io.opentelemetry.sdk.trace.samplers.ParentBasedSampler"); - - Field field = sampler.getSampler().getClass().getDeclaredField("root"); - field.setAccessible(true); - assertThat(field.get(sampler.getSampler()).getClass()).isEqualTo(expected); + assertThat(sampler.getSampler()).extracting("root").isInstanceOf(expected); }; } } From 3258b61a20ce92d2e1f2f01f7517081d8357d995 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 16 Mar 2023 15:43:44 -0500 Subject: [PATCH 2/2] Restore builder constructor --- .../trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java index 6d48c90a3b2..d1c74895267 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSamplerBuilder.java @@ -47,6 +47,8 @@ public final class JaegerRemoteSamplerBuilder { // Use Object type since gRPC may not be on the classpath. @Nullable private Object grpcChannel; + JaegerRemoteSamplerBuilder() {} + /** * Sets the service name to be used by this exporter. Required. *