Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Point out CDI restrictions with the mpTelemetry 2.0 WithSpan annotation #7568

Closed
yasmin-aumeeruddy opened this issue Sep 13, 2024 · 3 comments
Assignees
Labels
peer reviewed technical reviewed An SME reviewed and approved the documentation from a technical perspective.
Milestone

Comments

@yasmin-aumeeruddy
Copy link
Member

Please describe the problem you are having with the documentation. Is information missing, inaccurate, or unclear? Tell us about the context where you encountered the problem so we can understand how to address it.

There is missing information for MicroProfile Telemetry (versions 1.0, 1.1 and 2.0). Specifically, in the manual instrumentation section.

We currently show:

@ApplicationScoped
class SpanBean {

    @WithSpan("name")
    void spanName() {
       ...
    }

    @WithSpan
    void spanArgs(@SpanAttribute(value = "arg") String arg) {
       ...
    }
}

It would be helpful for users to know that@WithSpan adds a CDI interceptor and CDI interceptors are only called when you call the method directly on an object which was injected or looked up from CDI. They don't get called when you call a public method within the same class. Therefore in the above example, if there was code in spanName() that called the spanArgs(arg) method, a child span would not be created for spanArgs(arg). If a child span is desired when spanArgs(args) is called internally, the @WithSpan annotation should not be used. Instead, a Tracer and spanBuilder should be used like the folliowing example that is already in the docs:

@Inject private Tracer tracer;

@GET
public String myMethod() {
    ...
    Span newSpan = tracer.spanBuilder("QueryDatabase").startSpan();
    try (Scope s = newSpan.makeCurrent()) {
        queryDatabase();
    } finally {
        newSpan.end();
    }
    ...
}
@dmuelle
Copy link
Member

dmuelle commented Sep 16, 2024

Hi @yasmin-aumeeruddy - the updated draft (per our slack conv) is now ready for review:

https://docs-draft-openlibertyio.mqj6zf7jocq.us-south.codeengine.appdomain.cloud/docs/latest/telemetry-trace.html#manual

If any further edits are needed, just let me know. When you're satisfied with the update, you can add the technical reviewed label to this issue to sign off. Thanks!

@yasmin-aumeeruddy yasmin-aumeeruddy added the technical reviewed An SME reviewed and approved the documentation from a technical perspective. label Sep 17, 2024
@ramkumar-k-9286
Copy link
Contributor

ramkumar-k-9286 commented Sep 20, 2024

Peer Review (latest/telemetry-trace.html#manual)

After you complete those prerequisites, you’re ready to instrument your code.
->
instrument your code. -> Not clear to me - Not sure if it is used as terminology.


Any information that you add to a span is visible when you look at traces on your trace server.
->
Any information that you add to a span is visible when you look at the traces on your trace server.


Doubts regarding codeblock see additional spacing (Space after , Also placement of { )

private static final AttributeKey<String> USER_ID_ATTR = AttributeKey.stringKey("userId");
@Inject private Span currentSpan;
@GET
public String myMethod() {
    ...
    currentSpan.setAttribute(USER_ID_ATTR, getUserId());
    ...
}

->

private static final AttributeKey<String> USER_ID_ATTR = AttributeKey.stringKey("userId");
@Inject private Span currentSpan;
@GET
public String myMethod() 
{
    ...
    currentSpan.setAttribute(USER_ID_ATTR, getUserId());
    ...
}

This annotation creates a new Span and establishes any required relationships with the current trace context.
->
This annotation creates a new span and establishes any required relationships with the current trace context.


subspan -> sub-span ?


The following important considerations apply to manual instrumentation.
->
The following important considerations apply for manual instrumentation.


You must call the .end() method on any span you create, otherwise the span is not recorded.
->
Call the .end() method on any span you create, otherwise the span is not recorded.


Therefore, when you create a span, you usually also want to make it current.
->
Therefore, when you create a span, you also want to make it current.


dmuelle added a commit that referenced this issue Sep 24, 2024
@dmuelle
Copy link
Member

dmuelle commented Sep 25, 2024

Hi @ramkumar-k-9286 thanks for reviewing, all changes implemented except:

  • instrument your code-> "instrument" as verb is industry term in this context
  • Any information that you add to a span is visible when you look at traces on your trace server-> ok as is
  • Opening a curly bracket on the same line is valid Java syntax
  • subspan is ok in this context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
peer reviewed technical reviewed An SME reviewed and approved the documentation from a technical perspective.
Projects
None yet
Development

No branches or pull requests

3 participants