[4.x][Doc] - Fix tracing documentation #7962

dalexandrov · 2023-11-07T11:33:33Z

Fix Traxing docs #7931

Signed-off-by: Dmitry Aleksandrov <[email protected]>

docs/se/guides/tracing.adoc

tjquinno · 2023-11-07T19:34:46Z

docs/se/guides/tracing.adoc

-This means a single trace can include spans from multiple services and hosts.  OpenTracing uses a `SpanContext` to
+Helidon automatically traces across services if the services use the same tracer, for example, the same instance of Jaeger.
+This means a single trace can include spans from multiple services and hosts.  Helidon uses a `Span.context()` to
 propagate tracing information across process boundaries.  When you make client API calls, Helidon will
-internally call OpenTracing APIs to propagate the `SpanContext`. There is nothing you need to do in your application to make this work.
+internally call OpenTracing APIs to propagate the Span Context. There is nothing you need to do in your application to make this work.


V4 still has a SpanContext interface io.helidon.tracing. I think we can continue to refer to it as in the original document. If you want to avoid referring to that type for some reason, "Helidon uses a Span.context()" is awkward wording; maybe just use "span context" here and a couple lines below (lower case with space between the words).

It does make good sense to change OpenTracing to Helidon as shown.

tjquinno · 2023-11-07T19:36:10Z

docs/se/guides/tracing.adoc

+     <artifactId>helidon-tracing-providers-jaeger</artifactId>  <3>
+ </dependency>
+----
+<1> Helidon Tracing dependencies.


All three are Helidon tracing dependencies so this description does not really distinguish this one from the others. Maybe change this first one to something like "Helidon tracing API" instead?

tjquinno · 2023-11-07T19:39:05Z

docs/se/guides/tracing.adoc

+  service: helidon-se-2
+  protocol: http
+  port: 14250
+  path: /api/traces
+  tags:
+    env: development
+  enabled: true
+  sampler-type: "const"
+  sampler-param: 1
+  log-spans: true
+  propagation: b3


It's very good to use config settings that will make tracing work easily as a user experiments with it. (The param = 1 for example, IIRC, makes sure traces are flushed right away so they appear in the back-end system with minimal buffering or delay.)

That said, maybe we add a brief sentence explaining that and advising users to use different settings for production servers for performance reasons.

tjquinno · 2023-11-07T19:45:15Z

docs/se/guides/tracing.adoc

      response.send(result);
    } finally {
-      span.finish();   // <5>
+      span.end();   // <5>
    }


This code should probably conform to the earlier example which uses span.end() in the try block and span.end(t) in a catch (Throwable t) block.

tjquinno · 2023-11-07T19:46:29Z

docs/se/tracing.adoc

+----
+<dependency>
+    <groupId>io.helidon.tracing.providers</groupId>
+    <artifactId>helidon-tracing-providers-jaeger</artifactId>


The OTel case should not refer to the Jaeger provider, should it?

tjquinno · 2023-11-07T19:47:00Z

docs/se/tracing.adoc

+----
+<dependency>
+    <groupId>io.helidon.tracing.providers</groupId>
+    <artifactId>helidon-tracing-providers-jaeger</artifactId>


Should the OpenTracing case refer to the Jaeger provider?

Signed-off-by: Dmitry Aleksandrov <[email protected]>

tjquinno

A few minor edits and one fix to an omission in an example.

tjquinno · 2023-11-08T16:19:48Z

docs/se/guides/tracing.adoc

 propagate tracing information across process boundaries.  When you make client API calls, Helidon will
-internally call OpenTracing APIs to propagate the `SpanContext`. There is nothing you need to do in your application to make this work.
+internally call OpenTelemetry APIs or OpenTracing APIs to propagate the Span Context. There is nothing you need to do in your application to make this work.


If you are going to make any other changes, please change Span Context to either

span context

or

SpanContext (with back ticks before and after)

tjquinno · 2023-11-08T16:21:17Z

docs/se/guides/tracing.adoc

+     <artifactId>helidon-tracing-providers-jaeger</artifactId>  <3>
+ </dependency>
+----
+<1> Helidon Tracing API dependencies.


If you are going to make any other changes, please remove the word "dependencies" because all the called-out elements are dependencies.

tjquinno · 2023-11-08T16:25:53Z

docs/se/guides/tracing.adoc


    try {
      requestBuilder.property(
-          ClientTracingFilter.CURRENT_SPAN_CONTEXT_PROPERTY_NAME, request.spanContext());  // <3>
+          ClientTracingFilter.CURRENT_SPAN_CONTEXT_PROPERTY_NAME, span.context());  // <3>

      String result = requestBuilder   // <4>
          .get(String.class);
      response.send(result);


After the response.send(result) line add span.end() so in the normal case the span is properly ended.

tjquinno · 2023-11-08T16:26:18Z

docs/se/guides/tracing.adoc


      String result = requestBuilder   // <4>
          .get(String.class);
      response.send(result);
-    } finally {
-      span.finish();   // <5>
+    } catch (Throwandle t) {


Change Throwandle to Throwable

New typo: line 480 should be span.end(); (not spa)

Looks like 481 still says Throwandle; it needs to be Throwable

Signed-off-by: Dmitry Aleksandrov <[email protected]>

tjquinno

One new typo; one left over from earlier.

tjquinno · 2023-11-13T17:19:36Z

docs/se/guides/tracing.adoc


      String result = requestBuilder   // <4>
          .get(String.class);
      response.send(result);
-    } finally {
-      span.finish();   // <5>
+    } catch (Throwandle t) {


New typo: line 480 should be span.end(); (not spa)

tjquinno · 2023-11-13T17:21:25Z

docs/se/guides/tracing.adoc


      String result = requestBuilder   // <4>
          .get(String.class);
      response.send(result);
-    } finally {
-      span.finish();   // <5>
+    } catch (Throwandle t) {


Looks like 481 still says Throwandle; it needs to be Throwable

Signed-off-by: Dmitry Aleksandrov <[email protected]>

Fix tracing documentation

b39282a

Signed-off-by: Dmitry Aleksandrov <[email protected]>

dalexandrov added docs 4.x Version 4.x labels Nov 7, 2023

dalexandrov requested a review from tjquinno November 7, 2023 11:33

dalexandrov self-assigned this Nov 7, 2023

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Nov 7, 2023

Minor style issues

666caae

Signed-off-by: Dmitry Aleksandrov <[email protected]>

tjquinno requested changes Nov 7, 2023

View reviewed changes

dalexandrov added 3 commits November 8, 2023 12:32

Additional info

62073cd

Signed-off-by: Dmitry Aleksandrov <[email protected]>

Address comments

6060272

Signed-off-by: Dmitry Aleksandrov <[email protected]>

Fix site build issue

88d53de

Signed-off-by: Dmitry Aleksandrov <[email protected]>

tjquinno requested changes Nov 8, 2023

View reviewed changes

Minor fixes

66e5a8f

Signed-off-by: Dmitry Aleksandrov <[email protected]>

dalexandrov marked this pull request as ready for review November 13, 2023 09:27

tjquinno requested changes Nov 13, 2023

View reviewed changes

Typo fixes

9a3b09b

Signed-off-by: Dmitry Aleksandrov <[email protected]>

dalexandrov requested a review from tjquinno November 14, 2023 09:17

tjquinno approved these changes Nov 14, 2023

View reviewed changes

dalexandrov merged commit a9b6444 into helidon-io:main Nov 14, 2023
12 checks passed

dalexandrov deleted the 7931_fix_tracing_docs branch November 14, 2023 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4.x][Doc] - Fix tracing documentation #7962

[4.x][Doc] - Fix tracing documentation #7962

dalexandrov commented Nov 7, 2023

tjquinno Nov 7, 2023

dalexandrov Nov 8, 2023

tjquinno Nov 7, 2023

dalexandrov Nov 8, 2023

tjquinno Nov 7, 2023

dalexandrov Nov 8, 2023

tjquinno Nov 7, 2023

dalexandrov Nov 8, 2023

tjquinno Nov 7, 2023

dalexandrov Nov 8, 2023

tjquinno Nov 7, 2023

dalexandrov Nov 8, 2023

tjquinno left a comment

tjquinno Nov 8, 2023

dalexandrov Nov 13, 2023

tjquinno Nov 8, 2023

dalexandrov Nov 13, 2023

tjquinno Nov 8, 2023

dalexandrov Nov 13, 2023

tjquinno Nov 8, 2023

dalexandrov Nov 13, 2023

tjquinno Nov 13, 2023

tjquinno Nov 13, 2023

tjquinno left a comment

tjquinno Nov 13, 2023

tjquinno Nov 13, 2023

[4.x][Doc] - Fix tracing documentation #7962

[4.x][Doc] - Fix tracing documentation #7962

Conversation

dalexandrov commented Nov 7, 2023

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjquinno 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjquinno left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment