Add tracing support for internals and JSON-RPC #1557

atoulme · 2020-11-12T22:33:00Z

Signed-off-by: Antoine Toulme [email protected]

PR description

Adds tracing support for internals of Besu and the JSON-RPC HTTP service.

This was tested with soaking a Besu node for 48 hours against mainnet, with no sync issues oberved.

Changelog

I thought about the changelog and included a changelog update if required.

AbdelStark · 2020-11-19T09:10:05Z

Please can you do a full sync against Goerli and post the sync time here ? Just want to make sure this PR does not introduce performance issues.

atoulme · 2020-11-20T23:14:48Z

Sync time was 11 hours 32 minutes.

shemnon · 2020-11-21T02:51:24Z

Our 20.10.0-RC2 full sync was 12h9m. That was prior to the native libs improvements, so I'll run another full sync against 20.10.1.

shemnon · 2020-11-21T16:01:30Z

A 20.10.1 full sync clocked in at 11h6m, so the overhead of this change is under 4%. (20.10.1 had some significant crypto upgrades that significantly impact Goerli specifically) I understand this is "fully loaded" with OLTP spans being fully reported. What's the speed with the original prometheus metrics?

atoulme · 2020-11-21T21:25:11Z

There a number of things happening on the instance, as it also runs Splunk, the OpenTelemetry Collector and ethlogger.
Ethlogger in particular constantly polls Besu for all blocks, all transactions, all transaction receipts.
I haven't tried to sync without tracing on.

shemnon

A full sync with only Prometheus turned on and no open telemetry items resulted in a sync of 11h7h, only one minute longer than our 10.20.1 sync, well within noise levels of the network.

change io.hyperledger to org.hyperledger
remove usage of the shaded jar

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java

shemnon · 2020-11-24T12:44:33Z

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java

@@ -169,6 +198,7 @@ private JsonRpcHttpService(
    this.authenticationService = authenticationService;
    this.livenessService = livenessService;
    this.readinessService = readinessService;
+    this.tracer = OpenTelemetry.getGlobalTracer("io.hyperledger.besu.jsonrpc", "1.0.0");


org.hyperledger instead of io.hyperledger?

shemnon · 2020-11-24T12:47:31Z

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java

@@ -279,6 +310,32 @@ private Router buildRouter() {
    return router;
  }

+  private void createSpan(final RoutingContext routingContext) {


Is OpenTelemetry "always on"? Should there be a way for the end user to "opt out" like we do for other metrics collection?

Asking for the best way on this.

If it was just the one create span call it would be easy, but it looks like every request has to possibly add a span, so plumbing the metrics configs through seems a bit much... unless performance suffers. We have a benchmark where rpcs is the bottleneck, so if it's a problem I'd expect those benchmarks to fail. If you are willing to risk a call for a revert if performance takes a hit it can stay as-is.

So I'm asking, and so far if we don't use a metrics backend, the spans are created, but in the words of OpenTelemetry devs, they then "fall to the floor" as there is no export enabled.
If we want to even avoid the object allocation of creating span objects, we can add a check in the Besu code that disables creating span objects.

Object allocation is my worry, but I'm thinking if it matters it will only show up under load, which we have a test set up to detect (but only for committed code).

So if the test shows a regression the worst case scenario of a commit is a rollback and then a fix.

I am willing to try this out and see how the performance is impacted.
Initially I had a span collecting trie performance, but I reverted it, as it was too verbose. I believe the AbstractBlockProcessor might be too verbose as well.
We'll see - it would be great to understand better if there are specific use cases (block imports, mining) where tracing helps specifically.

gradle/versions.gradle

gradle/wrapper/gradle-wrapper.properties

metrics/core/build.gradle

services/pipeline/src/main/java/org/hyperledger/besu/services/pipeline/Pipeline.java

shemnon · 2020-12-08T00:45:45Z

Any progress on the requirement for non-shaded jars?

atoulme · 2020-12-08T05:46:49Z

Still working on it, slowly. I did a search to make sure I had all the libraries lined up, but since it tends to go wrong at runtime, I wanted to add an integration test to test this out.

Signed-off-by: Antoine Toulme <[email protected]>

shemnon · 2020-12-22T17:00:48Z

Our next release is 13 Jan. Reviews will be infrequent until the new year. Ping me on 1 Jan if this isn't approved.

atoulme · 2020-12-22T19:24:31Z

@shemnon I appreciate you chiming in. Happy holidays to you, your family and the contributors to the Besu project!
We can look at this together next year.

Signed-off-by: Antoine Toulme <[email protected]>

.../dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java

shemnon · 2021-01-04T16:44:55Z

build.gradle

-  // OpenTelemetry Wiring for windows scripts
-  def windowsRegex = $/"%JAVA_EXE%" %DEFAULT_JVM_OPTS%/$
-  def windowsReplacement = $/if Defined TRACING (TRACING_AGENT="-javaagent:" "%APP_HOME%\agent\/$ + agentFileName + '")\r\n"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %TRACING_AGENT%'
-  createScriptTask.windowsScript.text = createScriptTask.windowsScript.text.replace(windowsRegex, windowsReplacement)


What changed so that we no longer need the java agent for OTEL? I'm not seeing anything code-wise so I'm concerned this is an accidental deletion.

The agent inspects the code and injects changes at runtime to create traces from well-known libraries such as SQL querying or HTTP. We now do this manually and in a much more targeted fashion. We no longer need to use automatic instrumentation. Automatic instrumentation could also create extra traces that don't provide as much value.

metrics/core/src/main/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetrySystem.java

...src/test/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetryMetricsSystemTest.java

Signed-off-by: Antoine Toulme <[email protected]>

* Add tracing support for internals and JSON-RPC Signed-off-by: Antoine Toulme <[email protected]> * Remove rocksdb tracing as it slows down execution too much Signed-off-by: Antoine Toulme <[email protected]> * Add B3 headers extraction on JSON-RPC requests Signed-off-by: Antoine Toulme <[email protected]> * Remove traces around trie tree as they slow down syncing significantly Signed-off-by: Antoine Toulme <[email protected]> * Add tracing to fast sync pipeline Signed-off-by: Antoine Toulme <[email protected]> * Add tracing for all pipelines Signed-off-by: Antoine Toulme <[email protected]> * Address code review Signed-off-by: Antoine Toulme <[email protected]> * Add acceptance tests and break out the shaded dependency Signed-off-by: Antoine Toulme <[email protected]> * Fix tracer id Signed-off-by: Antoine Toulme <[email protected]> * Revert changes to trie Signed-off-by: Antoine Toulme <[email protected]> * Upgrade otel to latest, remove old tracing Signed-off-by: Antoine Toulme <[email protected]> * Code review comments Signed-off-by: Antoine Toulme <[email protected]>

atoulme force-pushed the tracing_with_otel branch 3 times, most recently from 9303e60 to be459ee Compare November 13, 2020 00:25

atoulme requested a review from AbdelStark November 17, 2020 21:04

atoulme force-pushed the tracing_with_otel branch from dc50467 to 00db98d Compare November 19, 2020 01:07

MadelineMurray mentioned this pull request Nov 23, 2020

Doc tracing hyperledger/besu-docs#551

Closed

shemnon reviewed Nov 24, 2020

View reviewed changes

atoulme added 8 commits December 21, 2020 18:13

Add tracing support for internals and JSON-RPC

017596e

Signed-off-by: Antoine Toulme <[email protected]>

Remove rocksdb tracing as it slows down execution too much

8ef03ea

Signed-off-by: Antoine Toulme <[email protected]>

Add B3 headers extraction on JSON-RPC requests

edea3bb

Signed-off-by: Antoine Toulme <[email protected]>

Remove traces around trie tree as they slow down syncing significantly

710bd02

Signed-off-by: Antoine Toulme <[email protected]>

Add tracing to fast sync pipeline

6efba27

Signed-off-by: Antoine Toulme <[email protected]>

Add tracing for all pipelines

22f2e97

Signed-off-by: Antoine Toulme <[email protected]>

Address code review

6fcc971

Signed-off-by: Antoine Toulme <[email protected]>

Add acceptance tests and break out the shaded dependency

38a2fe3

Signed-off-by: Antoine Toulme <[email protected]>

atoulme force-pushed the tracing_with_otel branch from 6ccdefa to 38a2fe3 Compare December 22, 2020 02:40

atoulme added 2 commits December 21, 2020 18:46

Fix tracer id

7568778

Signed-off-by: Antoine Toulme <[email protected]>

Revert changes to trie

f8afca1

Signed-off-by: Antoine Toulme <[email protected]>

atoulme force-pushed the tracing_with_otel branch from c891724 to 3fccb91 Compare December 23, 2020 22:25

Upgrade otel to latest, remove old tracing

c6f2ccd

Signed-off-by: Antoine Toulme <[email protected]>

atoulme force-pushed the tracing_with_otel branch from 3fccb91 to c6f2ccd Compare December 23, 2020 23:26

shemnon approved these changes Jan 4, 2021

View reviewed changes

Code review comments

80f7348

Signed-off-by: Antoine Toulme <[email protected]>

atoulme merged commit cd66968 into hyperledger:master Jan 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tracing support for internals and JSON-RPC #1557

Add tracing support for internals and JSON-RPC #1557

atoulme commented Nov 12, 2020

AbdelStark commented Nov 19, 2020

atoulme commented Nov 20, 2020

shemnon commented Nov 21, 2020

shemnon commented Nov 21, 2020

atoulme commented Nov 21, 2020

shemnon left a comment

shemnon Nov 24, 2020

shemnon Nov 24, 2020

atoulme Nov 24, 2020

shemnon Nov 24, 2020

atoulme Nov 24, 2020

shemnon Nov 24, 2020

atoulme Dec 22, 2020

shemnon commented Dec 8, 2020

atoulme commented Dec 8, 2020

shemnon commented Dec 22, 2020

atoulme commented Dec 22, 2020

shemnon Jan 4, 2021

atoulme Jan 4, 2021

Add tracing support for internals and JSON-RPC #1557

Add tracing support for internals and JSON-RPC #1557

Conversation

atoulme commented Nov 12, 2020

PR description

Changelog

AbdelStark commented Nov 19, 2020

atoulme commented Nov 20, 2020

shemnon commented Nov 21, 2020

shemnon commented Nov 21, 2020

atoulme commented Nov 21, 2020

shemnon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shemnon commented Dec 8, 2020

atoulme commented Dec 8, 2020

shemnon commented Dec 22, 2020

atoulme commented Dec 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment