Skip to content

Commit

Permalink
Otel take 2 (hyperledger#4075)
Browse files Browse the repository at this point in the history
* Revert "Revert "Upgrade OpenTelemetry (hyperledger#3675)" (hyperledger#4031)"

This reverts commit 17de636.

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>

* Make sure we don't initialize the OpenTelemetry global singleton by mistake

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>

* disable global otel singleton explicitly

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>

* make sure to set GlobalOpenTelemetry at most once to avoid test failures

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>

* reset for tests

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>

* fix changelog

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>

Signed-off-by: Antoine Toulme <antoine@lunar-ocean.com>
  • Loading branch information
atoulme authored Nov 14, 2022
1 parent 3406f06 commit 5b462af
Show file tree
Hide file tree
Showing 30 changed files with 466 additions and 258 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Improve performance of block processing by parallelizing some parts during the "commit" step [#4635](https://github.com/hyperledger/besu/pull/4635)
- Upgrade RocksDB version from 7.6.0 to 7.7.3
- Added new RPC endpoints `debug_setHead` & `debug_replayBlock [4580](https://github.com/hyperledger/besu/pull/4580)
- Upgrade OpenTelemetry to version 1.19.0 [#3675](https://github.com/hyperledger/besu/pull/3675)

### Bug Fixes

Expand Down Expand Up @@ -65,6 +66,7 @@
https://hyperledger.jfrog.io/hyperledger/besu-binaries/besu/22.10.0/besu-22.10.0.tar.gz / sha256: 18590796831a6c6c2ca17ba8e6877dd2bd63c25e034f1bbc987aaa0a9c3a178e
https://hyperledger.jfrog.io/hyperledger/besu-binaries/besu/22.10.0/besu-22.10.0.zip / sha256: 8ad4927469e8e128a3a2c7a708f108393eebc82c522f66cdcac4b7d206e07f90


## 22.10.0-RC2

### Breaking Changes
Expand Down
1 change: 1 addition & 0 deletions acceptance-tests/dsl/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ dependencies {
implementation 'io.reactivex.rxjava2:rxjava'
implementation 'io.vertx:vertx-core'
implementation 'junit:junit'
implementation 'io.opentelemetry:opentelemetry-api'
implementation 'org.apache.tuweni:tuweni-bytes'
implementation 'org.apache.tuweni:tuweni-io'
implementation 'org.apache.tuweni:tuweni-units'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ public class BesuNode implements NodeConfiguration, RunnableNode, AutoCloseable
private Optional<Integer> exitCode = Optional.empty();
private Optional<PkiKeyStoreConfiguration> pkiKeyStoreConfiguration = Optional.empty();
private final boolean isStrictTxReplayProtectionEnabled;
private final Map<String, String> environment;

public BesuNode(
final String name,
Expand Down Expand Up @@ -159,7 +160,8 @@ public BesuNode(
final List<String> runCommand,
final Optional<KeyPair> keyPair,
final Optional<PkiKeyStoreConfiguration> pkiKeyStoreConfiguration,
final boolean isStrictTxReplayProtectionEnabled)
final boolean isStrictTxReplayProtectionEnabled,
final Map<String, String> environment)
throws IOException {
this.homeDirectory = dataPath.orElseGet(BesuNode::createTmpDataDirectory);
this.isStrictTxReplayProtectionEnabled = isStrictTxReplayProtectionEnabled;
Expand Down Expand Up @@ -216,6 +218,7 @@ public BesuNode(
this.isDnsEnabled = isDnsEnabled;
privacyParameters.ifPresent(this::setPrivacyParameters);
this.pkiKeyStoreConfiguration = pkiKeyStoreConfiguration;
this.environment = environment;
LOG.info("Created BesuNode {}", this);
}

Expand Down Expand Up @@ -794,4 +797,9 @@ public void verify(final Condition expected) {
public void setExitCode(final int exitValue) {
this.exitCode = Optional.of(exitValue);
}

@Override
public Map<String, String> getEnvironment() {
return environment;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,8 @@ public void startNode(final BesuNode node) {
"JAVA_OPTS",
"-Djava.security.properties="
+ "acceptance-tests/tests/build/resources/test/acceptanceTesting.security");
// add additional environment variables
processBuilder.environment().putAll(node.getEnvironment());
try {
checkState(
isNotAliveOrphan(node.getName()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.vertx.core.Vertx;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -133,6 +134,7 @@ public void startNode(final BesuNode node) {
buildPluginContext(
node, storageService, securityModuleService, commonPluginConfiguration));

GlobalOpenTelemetry.resetForTest();
final ObservableMetricsSystem metricsSystem =
MetricsSystemFactory.create(node.getMetricsConfiguration());
final List<EnodeURL> bootnodes =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import java.nio.file.Path;
import java.util.List;
import java.util.Map;
import java.util.Optional;

public class BesuNodeConfiguration {
Expand Down Expand Up @@ -65,6 +66,7 @@ public class BesuNodeConfiguration {
private final Optional<KeyPair> keyPair;
private final Optional<PkiKeyStoreConfiguration> pkiKeyStoreConfiguration;
private final boolean strictTxReplayProtectionEnabled;
private final Map<String, String> environment;

BesuNodeConfiguration(
final String name,
Expand Down Expand Up @@ -97,7 +99,8 @@ public class BesuNodeConfiguration {
final List<String> runCommand,
final Optional<KeyPair> keyPair,
final Optional<PkiKeyStoreConfiguration> pkiKeyStoreConfiguration,
final boolean strictTxReplayProtectionEnabled) {
final boolean strictTxReplayProtectionEnabled,
final Map<String, String> environment) {
this.name = name;
this.miningParameters = miningParameters;
this.jsonRpcConfiguration = jsonRpcConfiguration;
Expand Down Expand Up @@ -129,6 +132,7 @@ public class BesuNodeConfiguration {
this.keyPair = keyPair;
this.pkiKeyStoreConfiguration = pkiKeyStoreConfiguration;
this.strictTxReplayProtectionEnabled = strictTxReplayProtectionEnabled;
this.environment = environment;
}

public String getName() {
Expand Down Expand Up @@ -254,4 +258,8 @@ public Optional<PkiKeyStoreConfiguration> getPkiKeyStoreConfiguration() {
public boolean isStrictTxReplayProtectionEnabled() {
return strictTxReplayProtectionEnabled;
}

public Map<String, String> getEnvironment() {
return environment;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;

public class BesuNodeConfigurationBuilder {
Expand Down Expand Up @@ -85,6 +87,7 @@ public class BesuNodeConfigurationBuilder {
private Optional<KeyPair> keyPair = Optional.empty();
private Optional<PkiKeyStoreConfiguration> pkiKeyStoreConfiguration = Optional.empty();
private Boolean strictTxReplayProtectionEnabled = false;
private Map<String, String> environment = new HashMap<>();

public BesuNodeConfigurationBuilder() {
// Check connections more frequently during acceptance tests to cut down on
Expand Down Expand Up @@ -483,6 +486,11 @@ public BesuNodeConfigurationBuilder strictTxReplayProtectionEnabled(
return this;
}

public BesuNodeConfigurationBuilder environment(final Map<String, String> environment) {
this.environment = environment;
return this;
}

public BesuNodeConfiguration build() {
return new BesuNodeConfiguration(
name,
Expand Down Expand Up @@ -515,6 +523,7 @@ public BesuNodeConfiguration build() {
runCommand,
keyPair,
pkiKeyStoreConfiguration,
strictTxReplayProtectionEnabled);
strictTxReplayProtectionEnabled,
environment);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ public BesuNode create(final BesuNodeConfiguration config) throws IOException {
config.getRunCommand(),
config.getKeyPair(),
config.getPkiKeyStoreConfiguration(),
config.isStrictTxReplayProtectionEnabled());
config.isStrictTxReplayProtectionEnabled(),
config.getEnvironment());
}

public BesuNode createMinerNode(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.net.URI;
import java.util.List;
import java.util.Map;
import java.util.Optional;

public interface NodeConfiguration {
Expand Down Expand Up @@ -59,4 +60,6 @@ public interface NodeConfiguration {
boolean isRevertReasonEnabled();

List<String> getStaticNodes();

Map<String, String> getEnvironment();
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ public PrivacyNode(
List.of(),
Optional.empty(),
Optional.empty(),
besuConfig.isStrictTxReplayProtectionEnabled());
besuConfig.isStrictTxReplayProtectionEnabled(),
besuConfig.getEnvironment());
}

public void testEnclaveConnection(final List<PrivacyNode> otherNodes) {
Expand Down
7 changes: 4 additions & 3 deletions acceptance-tests/tests/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,16 @@ dependencies {

testImplementation 'com.github.tomakehurst:wiremock-jre8-standalone'
testImplementation 'commons-io:commons-io'
testImplementation 'io.grpc:grpc-all'
testImplementation 'io.grpc:grpc-core'
testImplementation 'io.grpc:grpc-netty'
testImplementation 'io.grpc:grpc-stub'
testImplementation 'io.jaegertracing:jaeger-client'
testImplementation 'io.jaegertracing:jaeger-proto'
testImplementation 'io.opentelemetry:opentelemetry-extension-trace-propagators'
testImplementation 'io.opentelemetry.instrumentation:opentelemetry-okhttp-3.0'
testImplementation 'io.netty:netty-all'
testImplementation 'io.opentelemetry.proto:opentelemetry-proto'
testImplementation 'io.opentelemetry:opentelemetry-api'
testImplementation 'io.opentelemetry:opentelemetry-exporter-otlp'
testImplementation 'io.opentelemetry.proto:opentelemetry-proto'
testImplementation 'io.opentelemetry:opentelemetry-sdk'
testImplementation 'io.opentelemetry:opentelemetry-sdk-trace'
testImplementation 'io.opentracing.contrib:opentracing-okhttp3'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,21 @@

import java.time.Duration;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.google.common.io.Closer;
import com.google.protobuf.ByteString;
import io.grpc.Server;
import io.grpc.Status;
import io.grpc.netty.NettyServerBuilder;
import io.grpc.stub.StreamObserver;
import io.jaegertracing.Configuration;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.extension.trace.propagation.B3Propagator;
import io.opentelemetry.instrumentation.okhttp.v3_0.OkHttpTelemetry;
import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest;
import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse;
import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc;
Expand All @@ -44,8 +50,9 @@
import io.opentelemetry.proto.metrics.v1.ResourceMetrics;
import io.opentelemetry.proto.trace.v1.ResourceSpans;
import io.opentelemetry.proto.trace.v1.Span;
import io.opentracing.Tracer;
import io.opentracing.contrib.okhttp3.TracingCallFactory;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.trace.SdkTracerProvider;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import okhttp3.Call;
import okhttp3.MediaType;
import okhttp3.OkHttpClient;
Expand Down Expand Up @@ -120,6 +127,7 @@ List<ResourceMetrics> getReceivedMetrics() {

@Before
public void setUp() throws Exception {
System.setProperty("root.log.level", "DEBUG");
Server server =
NettyServerBuilder.forPort(4317)
.addService(fakeTracesCollector)
Expand All @@ -135,12 +143,22 @@ public void setUp() throws Exception {
.port(0)
.hostsAllowlist(singletonList("*"))
.build();
Map<String, String> env = new HashMap<>();
env.put("OTEL_METRIC_EXPORT_INTERVAL", "1000");
env.put("OTEL_TRACES_SAMPLER", "always_on");
env.put("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317");
env.put("OTEL_EXPORTER_OTLP_INSECURE", "true");
env.put("OTEL_EXPORTER_OTLP_PROTOCOL", "grpc");
env.put("OTEL_BSP_SCHEDULE_DELAY", "1000");
env.put("OTEL_BSP_EXPORT_TIMEOUT", "3000");

metricsNode =
besu.create(
new BesuNodeConfigurationBuilder()
.name("metrics-node")
.jsonRpcEnabled()
.metricsConfiguration(configuration)
.environment(env)
.build());
cluster.start(metricsNode);
}
Expand Down Expand Up @@ -170,11 +188,11 @@ public void traceReporting() {
net.netVersion().verify(metricsNode);
List<ResourceSpans> spans = fakeTracesCollector.getReceivedSpans();
assertThat(spans.isEmpty()).isFalse();
Span internalSpan = spans.get(0).getInstrumentationLibrarySpans(0).getSpans(0);
Span internalSpan = spans.get(0).getScopeSpans(0).getSpans(0);
assertThat(internalSpan.getKind()).isEqualTo(Span.SpanKind.SPAN_KIND_INTERNAL);
ByteString parent = internalSpan.getParentSpanId();
assertThat(parent.isEmpty()).isFalse();
Span serverSpan = spans.get(0).getInstrumentationLibrarySpans(0).getSpans(1);
Span serverSpan = spans.get(0).getScopeSpans(0).getSpans(1);
assertThat(serverSpan.getKind()).isEqualTo(Span.SpanKind.SPAN_KIND_SERVER);
ByteString rootSpanId = serverSpan.getParentSpanId();
assertThat(rootSpanId.isEmpty()).isTrue();
Expand All @@ -184,23 +202,26 @@ public void traceReporting() {
@Test
public void traceReportingWithTraceId() {
Duration timeout = Duration.ofSeconds(1);
OkHttpClient okClient =
new OkHttpClient.Builder()
.connectTimeout(timeout)
.readTimeout(timeout)
.writeTimeout(timeout)
.build();
WaitUtils.waitFor(
30,
() -> {
// call the json RPC endpoint to generate a trace - with trace metadata of our own
Configuration config =
new Configuration("okhttp")
.withSampler(
Configuration.SamplerConfiguration.fromEnv().withType("const").withParam(1));

Tracer tracer = config.getTracer();
Call.Factory client = new TracingCallFactory(okClient, tracer);
OpenTelemetry openTelemetry =
OpenTelemetrySdk.builder()
.setPropagators(
ContextPropagators.create(
TextMapPropagator.composite(B3Propagator.injectingSingleHeader())))
.setTracerProvider(
SdkTracerProvider.builder().setSampler(Sampler.alwaysOn()).build())
.build();
Call.Factory client =
OkHttpTelemetry.builder(openTelemetry)
.build()
.newCallFactory(
new OkHttpClient.Builder()
.connectTimeout(timeout)
.readTimeout(timeout)
.writeTimeout(timeout)
.build());
Request request =
new Request.Builder()
.url("http://localhost:" + metricsNode.getJsonRpcPort().get())
Expand All @@ -210,19 +231,22 @@ public void traceReportingWithTraceId() {
MediaType.get("application/json")))
.build();
Response response = client.newCall(request).execute();
assertThat(response.code()).isEqualTo(200);
response.close();
List<ResourceSpans> spans = new ArrayList<>(fakeTracesCollector.getReceivedSpans());
fakeTracesCollector.getReceivedSpans().clear();
assertThat(spans.isEmpty()).isFalse();
Span internalSpan = spans.get(0).getInstrumentationLibrarySpans(0).getSpans(0);
assertThat(internalSpan.getKind()).isEqualTo(Span.SpanKind.SPAN_KIND_INTERNAL);
ByteString parent = internalSpan.getParentSpanId();
assertThat(parent.isEmpty()).isFalse();
Span serverSpan = spans.get(0).getInstrumentationLibrarySpans(0).getSpans(1);
assertThat(serverSpan.getKind()).isEqualTo(Span.SpanKind.SPAN_KIND_SERVER);
ByteString rootSpanId = serverSpan.getParentSpanId();
assertThat(rootSpanId.isEmpty()).isFalse();
try {
assertThat(response.code()).isEqualTo(200);
List<ResourceSpans> spans = new ArrayList<>(fakeTracesCollector.getReceivedSpans());
assertThat(spans.isEmpty()).isFalse();
Span internalSpan = spans.get(0).getScopeSpans(0).getSpans(0);
assertThat(internalSpan.getKind()).isEqualTo(Span.SpanKind.SPAN_KIND_INTERNAL);
ByteString parent = internalSpan.getParentSpanId();
assertThat(parent.isEmpty()).isFalse();
Span serverSpan = spans.get(0).getScopeSpans(0).getSpans(1);
assertThat(serverSpan.getKind()).isEqualTo(Span.SpanKind.SPAN_KIND_SERVER);
ByteString rootSpanId = serverSpan.getParentSpanId();
assertThat(rootSpanId.isEmpty()).isFalse();
} finally {
response.close();
fakeTracesCollector.getReceivedSpans().clear();
}
});
}
}
1 change: 1 addition & 0 deletions besu/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ dependencies {
testImplementation 'com.google.auto.service:auto-service'
testImplementation 'com.squareup.okhttp3:okhttp'
testImplementation 'commons-io:commons-io'
testImplementation 'io.opentelemetry:opentelemetry-api'
testImplementation 'junit:junit'
testImplementation 'org.apache.commons:commons-text'
testImplementation 'org.apache.tuweni:tuweni-bytes'
Expand Down
Loading

0 comments on commit 5b462af

Please sign in to comment.