Implemented base zPages classes and TraceZ zPage #1380

wangty27 · 2020-06-29T15:47:57Z

zPages are a set of in-process dynamically generated HTML pages that collect and display data from the running process. In the set, TraceZ (/tracez) is the zPage that focuses on information collected about the trace spans. In particular, the TraceZ zPage displays information on sample running spans, sample latency, and sample error spans.

This PR implemented:

the base classes that are needed for creating a new zPage
the TraceZ zPage

linux-foundation-easycla · 2020-06-29T15:48:21Z

The committers are authorized under a signed CLA.

✅ William Hu (9ca81bc, 86c6db8, 66bafe2, 219d383)
✅ Terry (Tianyu) Wang (af80d27, d2eb7be)

codecov · 2020-06-29T15:58:14Z

Codecov Report

Merging #1380 into master will increase coverage by 0.06%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1380      +/-   ##
============================================
+ Coverage     91.94%   92.00%   +0.06%     
  Complexity      890      890              
============================================
  Files           114      114              
  Lines          3201     3201              
  Branches        262      262              
============================================
+ Hits           2943     2945       +2     
+ Misses          176      175       -1     
+ Partials         82       81       -1

Impacted Files	Coverage Δ	Complexity Δ
...telemetry/sdk/trace/export/BatchSpanProcessor.java	`95.94% <0.00%> (+1.35%)`	`8.00% <0.00%> (ø%)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0da293d...842629c. Read the comment docs.

summary table generation with running span logic

jkwatson · 2020-06-29T19:30:32Z

It looks like something went wrong with this branch, as it's got a TON of things that it shouldn't in it. Seems like you might need to rebase this and re-submit?

jkwatson · 2020-06-29T19:31:14Z

ooh. maybe you got it already. sorry for the noise!

williamhu99 · 2020-06-29T19:32:05Z

Yep, we merged our master branch with the OT master branch and cleaned up the old files.

...ensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezSpanProcessor.java

...tensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezZPageHandler.java

...extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageHttpHandler.java

...nsions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezDataAggregator.java

...ensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezSpanProcessor.java

carlosalberto · 2020-06-30T14:32:10Z

...ensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezSpanProcessor.java

+   *
+   * @return a Collection of {@link io.opentelemetry.sdk.trace.ReadableSpan}.
+   */
+  public Collection<ReadableSpan> getCompletedSpans() {


Return a copy perhaps?

...xtensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezSpanBuckets.java

sdk_extensions/zpages/README.md

sdk_extensions/zpages/build.gradle

anuraaga · 2020-07-11T01:48:35Z

...nsions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezDataAggregator.java

+    Map<String, TracezSpanBuckets> completedSpanCache = spanProcessor.getCompletedSpanCache();
+    TracezSpanBuckets buckets = completedSpanCache.get(spanName);
+    if (buckets == null) {
+      return new ArrayList<>();


Collections.emptyList

There's an error with immutability vs mutability if we revert back to Collections.emptyList. Should I change the function signature to return an immutable list?

If we're ok with guava, then yes. 👍

Oh, turns out there's another way to do it as well! You can resolve this comment along with the one below.

I'll wait to resolve until it's been changed. ;)

Sounds good. I already pushed the change, so I hope it updates in the repo soon.

...ensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezSpanProcessor.java

...sions/zpages/src/test/java/io/opentelemetry/sdk/extensions/zpages/TracezSpanBucketsTest.java

...ions/zpages/src/test/java/io/opentelemetry/sdk/extensions/zpages/TracezZPageHandlerTest.java

anuraaga · 2020-07-11T01:56:56Z

...nsions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezDataAggregator.java

+    Map<String, TracezSpanBuckets> completedSpanCache = spanProcessor.getCompletedSpanCache();
+    TracezSpanBuckets buckets = completedSpanCache.get(spanName);
+    if (buckets == null) {
+      return new ArrayList<>();


Collections.emptyList

Same response as above

sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageLogo.java

sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/SpanBucket.java

sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageLogo.java

…Trace

williamhu99 · 2020-07-13T18:26:14Z

@williamhu99 Can you go through all the conversations and resolve the ones that you think have been resolved? I'll give this one last review on Monday.

I don't think I have permission to resolve comments. Let me talk to Terry.

williamhu99 · 2020-07-13T20:07:32Z

It doesn't seem like Terry can make me an assignee or give me permission to resolve comments. Are code owners able to do this?

jkwatson · 2020-07-13T20:12:18Z

It doesn't seem like Terry can make me an assignee or give me permission to resolve comments. Are code owners able to do this?

I can resolve them for you. Just add a message to the end of each conversation you think should be resolved.

williamhu99 · 2020-07-13T20:22:51Z

Alright, sounds good. For the open comments right now, Terry and I plan to call and resolve them.

wangty27 · 2020-07-13T20:34:59Z

We've went through all the comments and resolved the ones that we think are resolved.

anuraaga

Minor points on the test, please take a look but otherwise looks good. Thanks a lot for the help!

...ions/zpages/src/test/java/io/opentelemetry/sdk/extensions/zpages/TracezZPageHandlerTest.java

anuraaga

Thanks a lot for all the hard work!

jkwatson · 2020-07-15T15:39:47Z

sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/SpanBucket.java

+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReferenceArray;
+
+final class SpanBucket {


I'd love to have unit tests on this class specifically, but that can be in a follow-up PR.

jkwatson

I have 3 follow-on requests for separate PRs after this one:

Make the ZPageServer not a singleton.
Add unit tests for the SpanBucket
Add an index page, so you don't have to remember the /tracez url to use this feature.

jkwatson · 2020-07-15T15:51:21Z

Thanks for all the hard work on this!

jkwatson · 2020-07-15T15:56:48Z

Note: follow on Issues: #1415 #1416 #1414

wangty27 requested review from arminru, bogdandrutu, carlosalberto, jkwatson, pavolloffay, thisthat and tylerbenson as code owners June 29, 2020 15:47

wangty27 force-pushed the master branch from 3c900bd to accdd4b Compare June 29, 2020 18:34

williamhu99 and others added 5 commits June 29, 2020 18:47

Implemented a span processor for the TraceZ zPage

9ca81bc

Implemented aggregation logic for running and latency based spans,

86c6db8

summary table generation with running span logic

Implemented aggregation logic for error based spans

66bafe2

Finished implementation of TraceZ zPage summary table

af80d27

Finished implementation of HttpHandler, HttpServer, and TraceZ zPage

d2eb7be

wangty27 force-pushed the master branch from accdd4b to d2eb7be Compare June 29, 2020 18:52

Refactored the sdk_contrib folder to follow updated syntax

219d383

williamhu99 force-pushed the master branch from a400dcc to 219d383 Compare June 29, 2020 19:18

Merge branch 'master' into master

febc453

Removed duplicate function

425e436

jkwatson reviewed Jun 29, 2020

View reviewed changes

...ensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezSpanProcessor.java Outdated Show resolved Hide resolved

jkwatson reviewed Jun 29, 2020

View reviewed changes

...ensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezSpanProcessor.java Outdated Show resolved Hide resolved

jkwatson reviewed Jun 29, 2020

View reviewed changes

...tensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezZPageHandler.java Outdated Show resolved Hide resolved

jkwatson reviewed Jun 29, 2020

View reviewed changes

...extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageHttpHandler.java Show resolved Hide resolved

thisthat reviewed Jun 30, 2020

View reviewed changes

...nsions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezDataAggregator.java Outdated Show resolved Hide resolved

thisthat reviewed Jun 30, 2020

View reviewed changes

...ensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TracezSpanProcessor.java Outdated Show resolved Hide resolved

carlosalberto reviewed Jun 30, 2020

View reviewed changes

anuraaga reviewed Jul 11, 2020

View reviewed changes

sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/SpanBucket.java Outdated Show resolved Hide resolved

Removed printStackTrace, added test for query parameter

2495cf9

jkwatson reviewed Jul 13, 2020

View reviewed changes

sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageLogo.java Outdated Show resolved Hide resolved

jkwatson reviewed Jul 13, 2020

View reviewed changes

sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageLogo.java Outdated Show resolved Hide resolved

jkwatson reviewed Jul 13, 2020

View reviewed changes

sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageLogo.java Show resolved Hide resolved

jkwatson reviewed Jul 13, 2020

View reviewed changes

sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageLogo.java Show resolved Hide resolved

Changed logger to static field, used log method to log detailed stack…

648a8b4

…Trace

Made minor fixes

d6113ea

Resolved unmodifiable list error with getOkSpans and getErrorSpans

23e1728

anuraaga approved these changes Jul 13, 2020

View reviewed changes

...ions/zpages/src/test/java/io/opentelemetry/sdk/extensions/zpages/TracezZPageHandlerTest.java Outdated Show resolved Hide resolved

...ions/zpages/src/test/java/io/opentelemetry/sdk/extensions/zpages/TracezZPageHandlerTest.java Show resolved Hide resolved

carlosalberto self-requested a review July 14, 2020 12:36

carlosalberto approved these changes Jul 14, 2020

View reviewed changes

wty27 added 2 commits July 14, 2020 20:32

Changed map @mock to empty map, added more test

317876f

Changed test runner

842629c

anuraaga approved these changes Jul 15, 2020

View reviewed changes

jkwatson reviewed Jul 15, 2020

View reviewed changes

jkwatson approved these changes Jul 15, 2020

View reviewed changes

jkwatson merged commit 373fbf4 into open-telemetry:master Jul 15, 2020

This was referenced Jul 17, 2020

TraceZ zpage is broken for spans names with characters that require url encoding #1425

Closed

Added preliminary documentation for the zPages contrib module #1455

Merged

This was referenced Aug 3, 2020

Allow users to create their own ZPageServer instance #1498

Closed

Wrote benchmark tests for the zPages module #1504

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented base zPages classes and TraceZ zPage #1380

Implemented base zPages classes and TraceZ zPage #1380

wangty27 commented Jun 29, 2020

linux-foundation-easycla bot commented Jun 29, 2020 •

edited

codecov bot commented Jun 29, 2020 •

edited

jkwatson commented Jun 29, 2020

jkwatson commented Jun 29, 2020

williamhu99 commented Jun 29, 2020

carlosalberto Jun 30, 2020

anuraaga Jul 11, 2020

williamhu99 Jul 13, 2020

jkwatson Jul 13, 2020

williamhu99 Jul 13, 2020

jkwatson Jul 13, 2020

williamhu99 Jul 13, 2020

anuraaga Jul 11, 2020

williamhu99 Jul 13, 2020

williamhu99 commented Jul 13, 2020

williamhu99 commented Jul 13, 2020

jkwatson commented Jul 13, 2020

williamhu99 commented Jul 13, 2020

wangty27 commented Jul 13, 2020

anuraaga left a comment

anuraaga left a comment

jkwatson Jul 15, 2020

jkwatson left a comment

jkwatson commented Jul 15, 2020

jkwatson commented Jul 15, 2020

Implemented base zPages classes and TraceZ zPage #1380

Implemented base zPages classes and TraceZ zPage #1380

Conversation

wangty27 commented Jun 29, 2020

linux-foundation-easycla bot commented Jun 29, 2020 • edited

codecov bot commented Jun 29, 2020 • edited

Codecov Report

jkwatson commented Jun 29, 2020

jkwatson commented Jun 29, 2020

williamhu99 commented Jun 29, 2020

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

Choose a reason for hiding this comment

williamhu99 commented Jul 13, 2020

williamhu99 commented Jul 13, 2020

jkwatson commented Jul 13, 2020

williamhu99 commented Jul 13, 2020

wangty27 commented Jul 13, 2020

anuraaga left a comment

Choose a reason for hiding this comment

anuraaga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkwatson left a comment

Choose a reason for hiding this comment

jkwatson commented Jul 15, 2020

jkwatson commented Jul 15, 2020

linux-foundation-easycla bot commented Jun 29, 2020 •

edited

codecov bot commented Jun 29, 2020 •

edited